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

[Marc Zyngier <maz@kernel.org>]

On Wed, 04 Oct 2023 17:52:08 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Wed, Oct 04, 2023 at 10:36:50AM +0100, Marc Zyngier wrote:
> > On Wed, 04 Oct 2023 00:04:06 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> [...]
> 
> > > +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.
> 
> Oh, very good point.
> 
> > 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.
> 
> Agreed, I'm happy to duck behind some vague language here :)
> 
> > > +
> > > +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...
> 
> Going to fix both of these with the following diff:
> 
> diff --git a/Documentation/virt/kvm/arm/vcpu-features.rst b/Documentation/virt/kvm/arm/vcpu-features.rst
> index 2d2f89c5781f..f7cc6d8d8b74 100644
> --- a/Documentation/virt/kvm/arm/vcpu-features.rst
> +++ b/Documentation/virt/kvm/arm/vcpu-features.rst
> @@ -24,8 +24,8 @@ 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.
> +system. The ID register values may be VM-scoped in KVM, meaning that the
> +values could be shared for all vCPUs in a VM.
>  
>  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
> @@ -36,9 +36,9 @@ 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
> -for fields in ID register'. KVM does not allow ID register values that exceed
> -the capabilities of the system.
> +outlined by the architecture in DDI0487J.a D19.1.3 'Principles of the ID
> +scheme 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
> 

Yup, looks good.

	M.

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