Re: [PATCH v8 02/11] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 17, 2023 at 7:00 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Thu, 17 Aug 2023 09:16:56 +0100,
> Cornelia Huck <cohuck@xxxxxxxxxx> wrote:
> >
> > On Mon, Aug 14 2023, Jing Zhang <jingzhangos@xxxxxxxxxx> wrote:
> >
> > > Maybe it'd be better to leave this to whenever we do need to add other
> > > range support?
> >
> > My point is: How does userspace figure out if the kernel that is running
> > supports ranges other than id regs? If this is just an insurance against
> > changes that might arrive or not, we can live with the awkward "just try
> > it out" approach; if we think it's likely that we'll need to extend it,
> > we need to add the mechanism for userspace to find out about it now, or
> > it would need to probe for presence of the mechanism...
>
> Agreed. Nothing like the present to address this sort of things. it
> really doesn't cost much, and I'd rather have it right now.
>
> Here's a vague attempt at an advertising mechanism. If people are OK
> with it, I can stash that on top of Jing's series.
>
> Thanks,
>
>         M.
>
> From bcfd87e85954e24ac4a905a3486c9040cdfc6d0b Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@xxxxxxxxxx>
> Date: Thu, 17 Aug 2023 14:48:16 +0100
> Subject: [PATCH] KVM: arm64: Add KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
>
> While the Feature ID range is well defined and pretty large, it
> isn't inconceivable that the architecture will eventually grow
> some other ranges that will need to similarly be described to
> userspace.
>
> Add a new capability (KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES)
> that returns a bitmap of the valid ranges, which can subsequently
> be retrieved, one at a time by setting the index of the set bit
> as the range identifier.
>
> Obviously, we only support a value of 0 for now.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> ---
>  Documentation/virt/kvm/api.rst    | 37 ++++++++++++++++++++-----------
>  arch/arm64/include/uapi/asm/kvm.h | 13 +++++++----
>  arch/arm64/kvm/arm.c              |  3 +++
>  arch/arm64/kvm/sys_regs.c         |  5 +++--
>  include/uapi/linux/kvm.h          |  1 +
>  5 files changed, 40 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 92a9b20f970e..0e6ce02cac3b 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6071,7 +6071,7 @@ applied.
>  4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
>  -------------------------------------------
>
> -:Capability: none
> +:Capability: KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
>  :Architectures: arm64
>  :Type: vm ioctl
>  :Parameters: struct reg_mask_range (in/out)
> @@ -6082,20 +6082,31 @@ applied.
>
>          #define ARM64_FEATURE_ID_SPACE_SIZE    (3 * 8 * 8)
>
> -        struct reg_mask_range {
> -                __u64 addr;             /* Pointer to mask array */
> -                __u64 reserved[7];
> -        };
> +       struct reg_mask_range {
> +               __u64 addr;             /* Pointer to mask array */
> +               __u32 range;            /* Requested range */
> +               __u32 reserved[13];
> +       };
>
> -This ioctl would copy the writable masks for feature ID registers to userspace.
> -The Feature ID space is defined as the System register space in AArch64 with
> +This ioctl copies the writable masks for Feature ID registers to userspace.
> +The Feature ID space is defined as the AArch64 System Register space with
>  op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
> -To get the index in the mask array pointed by ``addr`` for a specified feature
> -ID register, use the macro ``ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2)``.
> -This allows the userspace to know upfront whether it can actually tweak the
> -contents of a feature ID register or not.
> -The ``reserved[7]`` is reserved for future use to add other register space. For
> -feature ID registers, it should be 0, otherwise, KVM may return error.
> +
> +The mask array pointed to by ``addr`` is indexed by the macro
> +``ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2)``, allowing userspace
> +to know what bits can be changed for the system register described by ``op0,
> +op1, crn, crm, op2``.
> +
> +The ``range`` field describes the requested range of registers. The valid
> +ranges can be retrieved by checking the return value of
> +KVM_CAP_CHECK_EXTENSION_VM for the KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES
> +capability, which will return a bitmask of the supported ranges. Each bit
> +set in the return value represents a possible value for the ``range``
> +field.  At the time of writing, only bit 0 is returned set by the
> +capability, meaning that only the value 0 is value for ``range``.
> +
> +The ``reserved[13]`` array is reserved for future use and should be 0, or
> +KVM may return an error.
>
>  5. The kvm_run structure
>  ========================
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 7a21bbb8a0f7..5148b4c22549 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -505,8 +505,9 @@ struct kvm_smccc_filter {
>  #define KVM_HYPERCALL_EXIT_SMC         (1U << 0)
>  #define KVM_HYPERCALL_EXIT_16BIT       (1U << 1)
>
> -/* Get feature ID registers userspace writable mask. */
>  /*
> + * Get feature ID registers userspace writable mask.
> + *
>   * From DDI0487J.a, D19.2.66 ("ID_AA64MMFR2_EL1, AArch64 Memory Model
>   * Feature Register 2"):
>   *
> @@ -514,8 +515,11 @@ struct kvm_smccc_filter {
>   * AArch64 with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7},
>   * op2=={0-7}."
>   *
> - * This covers all R/O registers that indicate anything useful feature
> - * wise, including the ID registers.
> + * This covers all currently known R/O registers that indicate
> + * anything useful feature wise, including the ID registers.
> + *
> + * If we ever need to introduce a new range, it will be described as
> + * such in the range field.
>   */
>  #define ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2)            \
>         ({                                                              \
> @@ -528,7 +532,8 @@ struct kvm_smccc_filter {
>
>  struct reg_mask_range {
>         __u64 addr;             /* Pointer to mask array */
> -       __u64 reserved[7];
> +       __u32 range;            /* Requested range */
> +       __u32 reserved[13];
>  };
>
>  #endif
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index e08894692829..6ea4d8b0e744 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -316,6 +316,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>         case KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES:
>                 r = kvm_supported_block_sizes();
>                 break;
> +       case KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES:
> +               r = BIT(0);
> +               break;
>         default:
>                 r = 0;
>         }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 59c590fff4f2..6eadd0fa2c53 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -3600,8 +3600,9 @@ int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm, struct reg_mask_range *
>         const void *zero_page = page_to_virt(ZERO_PAGE(0));
>         u64 __user *masks = (u64 __user *)range->addr;
>
> -       /* Only feature id range is supported, reserved[7] must be zero. */
> -       if (memcmp(range->reserved, zero_page, sizeof(range->reserved)))
> +       /* Only feature id range is supported, reserved[13] must be zero. */
> +       if (range->range ||
> +           memcmp(range->reserved, zero_page, sizeof(range->reserved)))
>                 return -EINVAL;
>
>         /* Wipe the whole thing first */
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 424b6d00440b..f5100055a1a6 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1192,6 +1192,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_COUNTER_OFFSET 227
>  #define KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE 228
>  #define KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES 229
> +#define KVM_CAP_ARM_SUPPORTED_FEATURE_ID_RANGES 230
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
>
> --
> Without deviation from the norm, progress is not possible.

Looks good to me.

Thanks,
Jing




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux