Re: [PATCH v3 09/10] KVM: s390: Update api documentation for memop ioctl

Janosch Frank
2022-02-11T09:17:23Z

On 2/9/22 18:04, Janis Schoetterl-Glausch wrote:
> Document all currently existing operations, flags and explain under
> which circumstances they are available. Document the recently
> introduced absolute operations and the storage key protection flag,
> as well as the existing SIDA operations.
>
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Minor nits below

> ---
> Documentation/virt/kvm/api.rst | 112 ++++++++++++++++++++++++++-------
> include/uapi/linux/kvm.h | 2 +-
> 2 files changed, 91 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index a4267104db50..2d131af44576 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -3683,15 +3683,17 @@ The fields in each entry are defined as follows:
> 4.89 KVM_S390_MEM_OP
> --------------------
>
> -:Capability: KVM_CAP_S390_MEM_OP
> +:Capability: KVM_CAP_S390_MEM_OP, KVM_CAP_S390_PROTECTED, KVM_CAP_S390_MEM_OP_EXTENSION
> :Architectures: s390
> -:Type: vcpu ioctl
> +:Type: vm ioctl, vcpu ioctl
> :Parameters: struct kvm_s390_mem_op (in)
> :Returns: = 0 on success,
> < 0 on generic error (e.g. -EFAULT or -ENOMEM),
> > 0 if an exception occurred while walking the page tables
>
> -Read or write data from/to the logical (virtual) memory of a VCPU.
> +Read or write data from/to the VM's memory.
> +The KVM_CAP_S390_MEM_OP_EXTENSION capability specifies what functionality is
> +supported.
>
> Parameters are specified via the following structure::
>
> @@ -3701,33 +3703,99 @@ Parameters are specified via the following structure::
> __u32 size; /* amount of bytes */
> __u32 op; /* type of operation */
> __u64 buf; /* buffer in userspace */
> - __u8 ar; /* the access register number */
> - __u8 reserved[31]; /* should be set to 0 */
> + union {
> + struct {
> + __u8 ar; /* the access register number */
> + __u8 key; /* access key, ignored if flag unset */
> + };
> + __u32 sida_offset; /* offset into the sida */
> + __u8 reserved[32]; /* ignored */
> + };
> };
>
> -The type of operation is specified in the "op" field. It is either
> -KVM_S390_MEMOP_LOGICAL_READ for reading from logical memory space or
> -KVM_S390_MEMOP_LOGICAL_WRITE for writing to logical memory space. The
> -KVM_S390_MEMOP_F_CHECK_ONLY flag can be set in the "flags" field to check
> -whether the corresponding memory access would create an access exception
> -(without touching the data in the memory at the destination). In case an
> -access exception occurred while walking the MMU tables of the guest, the
> -ioctl returns a positive error number to indicate the type of exception.
> -This exception is also raised directly at the corresponding VCPU if the
> -flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field.
> -
> The start address of the memory region has to be specified in the "gaddr"
> field, and the length of the region in the "size" field (which must not
> be 0). The maximum value for "size" can be obtained by checking the
> KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the
> userspace application where the read data should be written to for
> -KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written is
> -stored for a KVM_S390_MEMOP_LOGICAL_WRITE. When KVM_S390_MEMOP_F_CHECK_ONLY
> -is specified, "buf" is unused and can be NULL. "ar" designates the access
> -register number to be used; the valid range is 0..15.
> +a read access, or where the data that should be written is stored for
> +a write access. The "reserved" field is meant for future extensions.
> +Reserved and unused values are ignored. Future extension that add members must
> +introduce new flags.
> +
> +The type of operation is specified in the "op" field. Flags modifying
> +their behavior can be set in the "flags" field. Undefined flag bits must
> +be set to 0.
> +
> +Possible operations are:
> + * ``KVM_S390_MEMOP_LOGICAL_READ``
> + * ``KVM_S390_MEMOP_LOGICAL_WRITE``
> + * ``KVM_S390_MEMOP_ABSOLUTE_READ``
> + * ``KVM_S390_MEMOP_ABSOLUTE_WRITE``
> + * ``KVM_S390_MEMOP_SIDA_READ``
> + * ``KVM_S390_MEMOP_SIDA_WRITE``
> +
> +Logical read/write:
> +^^^^^^^^^^^^^^^^^^^
> +
> +Access logical memory, i.e. translate the given guest address to an absolute
> +address given the state of the VCPU and use the absolute address as target of
> +the access. "ar" designates the access register number to be used; the valid
> +range is 0..15.
> +Logical accesses are permitted for the VCPU ioctl only.
> +Logical accesses are permitted for non secure guests only.

s/secure/protected/

> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_INJECT_EXCEPTION``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The KVM_S390_MEMOP_F_CHECK_ONLY flag can be set to check whether the
> +corresponding memory access would cause an access exception, without touching

I think the comma needs to be removed.

> +the data in memory at the destination.
> +In this case, "buf" is unused and can be NULL.
> +
> +In case an access exception occurred during the access (or would occur
> +in case of KVM_S390_MEMOP_F_CHECK_ONLY), the ioctl returns a positive
> +error number indicating the type of exception. This exception is also
> +raised directly at the corresponding VCPU if the flag
> +KVM_S390_MEMOP_F_INJECT_EXCEPTION is set.
> +
> +If the KVM_S390_MEMOP_F_SKEY_PROTECTION flag is set, storage key
> +protection is also in effect and may cause exceptions if accesses are
> +prohibited given the access key passed in "key".
> +KVM_S390_MEMOP_F_SKEY_PROTECTION is available if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +
> +Absolute read/write:
> +^^^^^^^^^^^^^^^^^^^^
> +
> +Access absolute memory. This operation is intended to be used with the
> +KVM_S390_MEMOP_F_SKEY_PROTECTION flag, to allow accessing memory and performing
> +the checks required for storage key protection as one operation (as opposed to
> +user space getting the storage keys, performing the checks, and accessing
> +memory thereafter, which could lead to a delay between check and access).
> +Absolute accesses are permitted for the VM ioctl if KVM_CAP_S390_MEM_OP_EXTENSION
> +is > 0.
> +Currently absolute accesses are not permitted for VCPU ioctls.
> +Absolute accesses are permitted for non secure guests only.

s/secure/protected/

> +
> +Supported flags:
> + * ``KVM_S390_MEMOP_F_CHECK_ONLY``
> + * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
> +
> +The semantics of the flags are as for logical accesses.
> +
> +SIDA read/write:
> +^^^^^^^^^^^^^^^^
> +
> +Access the secure instruction data area which contains memory operands necessary
> +for instruction emulation for secure guests.
> +SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
> +SIDA accesses are permitted for the VCPU ioctl only.
> +SIDA accesses are permitted for secure guests only.

s/secure/protected/

>
> -The "reserved" field is meant for future extensions. It is not used by
> -KVM with the currently defined set of flags.
> +No flags are supported.
>
> 4.90 KVM_S390_GET_SKEYS
> -----------------------
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 26bff414f1a0..fd01fe04a183 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -567,7 +567,7 @@ struct kvm_s390_mem_op {
> __u8 key; /* access key, ignored if flag unset */
> };
> __u32 sida_offset; /* offset into the sida */
> - __u8 reserved[32]; /* should be set to 0 */
> + __u8 reserved[32]; /* ignored */
> };
> };
> /* types for kvm_s390_mem_op->op */