Re: [PATCH v11 10/12] KVM: arm64: Document vCPU feature selection UAPIs

[Marc Zyngier <maz@kernel.org>]

On Wed, 04 Oct 2023 00:04:06 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> KVM/arm64 has a couple schemes for handling vCPU feature selection now,
> which is a lot to put on userspace. Add some documentation about how
> these interact and provide some recommendations for how to use the
> writable ID register scheme.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Documentation/virt/kvm/api.rst               |  4 ++
>  Documentation/virt/kvm/arm/index.rst         |  1 +
>  Documentation/virt/kvm/arm/vcpu-features.rst | 48 ++++++++++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 Documentation/virt/kvm/arm/vcpu-features.rst
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index d55c2b68c0a9..8d4050eedb26 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -3370,6 +3370,8 @@ return indicates the attribute is implemented.  It does not necessarily
>  indicate that the attribute can be read or written in the device's
>  current state.  "addr" is ignored.
>  
> +.. _KVM_ARM_VCPU_INIT:
> +
>  4.82 KVM_ARM_VCPU_INIT
>  ----------------------
>  
> @@ -6070,6 +6072,8 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG
>  interface. No error will be returned, but the resulting offset will not be
>  applied.
>  
> +.. _KVM_ARM_GET_REG_WRITABLE_MASKS:
> +
>  4.139 KVM_ARM_GET_REG_WRITABLE_MASKS
>  -------------------------------------------
>  
> diff --git a/Documentation/virt/kvm/arm/index.rst b/Documentation/virt/kvm/arm/index.rst
> index e84848432158..7f231c724e16 100644
> --- a/Documentation/virt/kvm/arm/index.rst
> +++ b/Documentation/virt/kvm/arm/index.rst
> @@ -11,3 +11,4 @@ ARM
>     hypercalls
>     pvtime
>     ptp_kvm
> +   vcpu-features
> diff --git a/Documentation/virt/kvm/arm/vcpu-features.rst b/Documentation/virt/kvm/arm/vcpu-features.rst
> new file mode 100644
> index 000000000000..2d2f89c5781f
> --- /dev/null
> +++ b/Documentation/virt/kvm/arm/vcpu-features.rst
> @@ -0,0 +1,48 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===============================
> +vCPU feature selection on arm64
> +===============================
> +
> +KVM/arm64 provides two mechanisms that allow userspace to configure
> +the CPU features presented to the guest.
> +
> +KVM_ARM_VCPU_INIT
> +=================
> +
> +The ``KVM_ARM_VCPU_INIT`` ioctl accepts a bitmap of feature flags
> +(``struct kvm_vcpu_init::features``). Features enabled by this interface are
> +*opt-in* and may change/extend UAPI. See :ref:`KVM_ARM_VCPU_INIT` for complete
> +documentation of the features controlled by the ioctl.
> +
> +Otherwise, all CPU features supported by KVM are described by the architected
> +ID registers.
> +
> +The ID Registers
> +================
> +
> +The Arm architecture specifies a range of *ID Registers* that describe the set
> +of architectural features supported by the CPU implementation. KVM initializes
> +the guest's ID registers to the maximum set of CPU features supported by the
> +system. The ID register values are VM-scoped in KVM, meaning that the values
> +are identical for all vCPUs in a VM.

I'm a bit reluctant to give this guarantee. Case in point: MPIDR_EL1
is part of the Feature ID space, and is definitely *not* a register
that we can make global, even on a fully homogeneous system.

I'd also like to give us more flexibility to change the implementation
in the future without having to change the API again. IMO, the fact
that we make our life simpler by only tracking a single copy is an
implementation detail, not something that userspace should rely on.

I would simply turn the "The ID register values are VM-scoped" into
"The ID register values may be VM-scoped", which gives us that
flexibility.

> +
> +KVM allows userspace to *opt-out* of certain CPU features described by the ID
> +registers by writing values to them via the ``KVM_SET_ONE_REG`` ioctl. The ID
> +registers are mutable until the VM has started, i.e. userspace has called
> +``KVM_RUN`` on at least one vCPU in the VM. Userspace can discover what fields
> +are mutable in the ID registers using the ``KVM_ARM_GET_REG_WRITABLE_MASKS``.
> +See the :ref:`ioctl documentation <KVM_ARM_GET_REG_WRITABLE_MASKS>` for more
> +details.
> +
> +Userspace is allowed to *limit* or *mask* CPU features according to the rules
> +outlined by the architecture in DDI0487J 'D19.1.3 Principles of the ID scheme

nit: consider spelling out the *full* version of the ARM ARM (DDI
0487J.a), just in case we get a J.b this side of Xmas and that this
reference is renumbered...

> +for fields in ID register'. KVM does not allow ID register values that exceed
> +the capabilities of the system.
> +
> +.. warning::
> +   It is **strongly recommended** that userspace modify the ID register values
> +   before accessing the rest of the vCPU's CPU register state. KVM may use the
> +   ID register values to control feature emulation. Interleaving ID register
> +   modification with other system register accesses may lead to unpredictable
> +   behavior.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.