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
