All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oliver.upton@linux.dev>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Jing Zhang <jingzhangos@google.com>,
	Cornelia Huck <cohuck@redhat.com>
Subject: Re: [PATCH v11 10/12] KVM: arm64: Document vCPU feature selection UAPIs
Date: Wed, 4 Oct 2023 16:52:08 +0000	[thread overview]
Message-ID: <ZR2YOF1lMGLraR1Q@linux.dev> (raw)
In-Reply-To: <86o7heohjh.wl-maz@kernel.org>

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

-- 
Thanks,
Oliver

  reply	other threads:[~2023-10-04 16:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-03 23:03 [PATCH v11 00/12] KVM: arm64: Enable 'writable' ID registers Oliver Upton
2023-10-03 23:03 ` [PATCH v11 01/12] KVM: arm64: Allow userspace to get the writable masks for feature " Oliver Upton
2023-10-03 23:03 ` [PATCH v11 02/12] KVM: arm64: Document KVM_ARM_GET_REG_WRITABLE_MASKS Oliver Upton
2023-10-03 23:03 ` [PATCH v11 03/12] KVM: arm64: Use guest ID register values for the sake of emulation Oliver Upton
2023-10-03 23:04 ` [PATCH v11 04/12] KVM: arm64: Reject attempts to set invalid debug arch version Oliver Upton
2023-10-03 23:04 ` [PATCH v11 05/12] KVM: arm64: Bump up the default KVM sanitised debug version to v8p8 Oliver Upton
2023-10-04  8:57   ` Marc Zyngier
2023-10-04 17:08     ` Oliver Upton
2023-10-04 17:46       ` Marc Zyngier
2023-10-03 23:04 ` [PATCH v11 06/12] KVM: arm64: Allow userspace to change ID_AA64ISAR{0-2}_EL1 Oliver Upton
2023-10-03 23:04 ` [PATCH v11 07/12] KVM: arm64: Allow userspace to change ID_AA64MMFR{0-2}_EL1 Oliver Upton
2023-10-03 23:04 ` [PATCH v11 08/12] KVM: arm64: Allow userspace to change ID_AA64PFR0_EL1 Oliver Upton
2023-10-03 23:04 ` [PATCH v11 09/12] KVM: arm64: Allow userspace to change ID_AA64ZFR0_EL1 Oliver Upton
2023-10-03 23:04 ` [PATCH v11 10/12] KVM: arm64: Document vCPU feature selection UAPIs Oliver Upton
2023-10-04  9:36   ` Marc Zyngier
2023-10-04 16:52     ` Oliver Upton [this message]
2023-10-04 17:48       ` Marc Zyngier
2023-10-03 23:04 ` [PATCH v11 11/12] KVM: arm64: selftests: Import automatic generation of sysreg defs Oliver Upton
2023-10-03 23:04 ` [PATCH v11 12/12] KVM: arm64: selftests: Test for setting ID register from usersapce Oliver Upton
2023-10-04  9:40 ` [PATCH v11 00/12] KVM: arm64: Enable 'writable' ID registers Marc Zyngier
2023-10-04 16:53   ` Oliver Upton
2023-10-04 17:46 ` Oliver Upton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZR2YOF1lMGLraR1Q@linux.dev \
    --to=oliver.upton@linux.dev \
    --cc=cohuck@redhat.com \
    --cc=james.morse@arm.com \
    --cc=jingzhangos@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.