All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oupton@google.com>
To: Ricardo Koller <ricarkol@google.com>
Cc: kvm@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	Peter Shier <pshier@google.com>, Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 11/25] KVM: arm64: Add remaining ID registers to id_reg_desc_table
Date: Thu, 24 Mar 2022 02:26:30 +0000	[thread overview]
Message-ID: <YjvW1lLT1sVZf0jK@google.com> (raw)
In-Reply-To: <YjueX2DOxjoc/d4j@google.com>

On Wed, Mar 23, 2022 at 10:25:35PM +0000, Oliver Upton wrote:
> On Wed, Mar 23, 2022 at 03:22:43PM -0700, Ricardo Koller wrote:
> > On Wed, Mar 23, 2022 at 08:44:26PM +0000, Oliver Upton wrote:
> > > On Wed, Mar 23, 2022 at 01:13:32PM -0700, Ricardo Koller wrote:
> > > > On Wed, Mar 23, 2022 at 07:53:14PM +0000, Oliver Upton wrote:
> > > > > Hi Reiji,
> > > > > 
> > > > > On Thu, Mar 10, 2022 at 08:47:57PM -0800, Reiji Watanabe wrote:
> > > > > > Add hidden or reserved ID registers, and remaining ID registers,
> > > > > > which don't require special handling, to id_reg_desc_table.
> > > > > > Add 'flags' field to id_reg_desc, which is used to indicates hiddden
> > > > > > or reserved registers. Since now id_reg_desc_init() is called even
> > > > > > for hidden/reserved registers, change it to not do anything for them.
> > > > > > 
> > > > > > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > > > > 
> > > > > I think there is a very important detail of the series that probably
> > > > > should be highlighted. We are only allowing AArch64 feature registers to
> > > > > be configurable, right? AArch32 feature registers remain visible with
> > > > > their default values passed through to the guest. If you've already
> > > > > stated this as a precondition elsewhere then my apologies for the noise.
> > > > 
> > > > Aren't AArch64 ID regs architecturally mapped to their AArch32
> > > > counterparts?  They should show the same values.  I'm not sure if it's a
> > > > problem (and if KVM is faithful to that rule),
> > > 
> > > I believe it's a bit more subtle than that. The AArch32 feature registers
> > > are architecturally mapped to certain encodings accessible from AArch64.
> > > For example, ID_PFR0_EL1 is actually a 64 bit register where bits [31:0]
> > > map to the ID_PFR0 AArch32 register. ID_PFR0_EL1 is only accessible from
> > > AArch64 with the MRS instruction, and ID_PFR0 is only accessible from
> > > AArch32 with the MRC instruction. KVM just so happens to handle both of
> > > these reads from the same sys_reg_desc.

Ughhhhh.

We actually clear HCR_EL2.TID3 for AArch32 guests, so AArch32 EL1 reads
straight from hardware. Considering the work we put in to make sure
feature registers are consistent system-wide and the limitations on
certain features, this is plain wrong.

I have a series that addresses this but need to go find some 32 bit
hardware to test with :)

--
Thanks,
Oliver
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Ricardo Koller <ricarkol@google.com>
Cc: Reiji Watanabe <reijiw@google.com>, Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Will Deacon <will@kernel.org>, Andrew Jones <drjones@redhat.com>,
	Fuad Tabba <tabba@google.com>,
	Peng Liang <liangpeng10@huawei.com>,
	Peter Shier <pshier@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v6 11/25] KVM: arm64: Add remaining ID registers to id_reg_desc_table
Date: Thu, 24 Mar 2022 02:26:30 +0000	[thread overview]
Message-ID: <YjvW1lLT1sVZf0jK@google.com> (raw)
In-Reply-To: <YjueX2DOxjoc/d4j@google.com>

On Wed, Mar 23, 2022 at 10:25:35PM +0000, Oliver Upton wrote:
> On Wed, Mar 23, 2022 at 03:22:43PM -0700, Ricardo Koller wrote:
> > On Wed, Mar 23, 2022 at 08:44:26PM +0000, Oliver Upton wrote:
> > > On Wed, Mar 23, 2022 at 01:13:32PM -0700, Ricardo Koller wrote:
> > > > On Wed, Mar 23, 2022 at 07:53:14PM +0000, Oliver Upton wrote:
> > > > > Hi Reiji,
> > > > > 
> > > > > On Thu, Mar 10, 2022 at 08:47:57PM -0800, Reiji Watanabe wrote:
> > > > > > Add hidden or reserved ID registers, and remaining ID registers,
> > > > > > which don't require special handling, to id_reg_desc_table.
> > > > > > Add 'flags' field to id_reg_desc, which is used to indicates hiddden
> > > > > > or reserved registers. Since now id_reg_desc_init() is called even
> > > > > > for hidden/reserved registers, change it to not do anything for them.
> > > > > > 
> > > > > > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > > > > 
> > > > > I think there is a very important detail of the series that probably
> > > > > should be highlighted. We are only allowing AArch64 feature registers to
> > > > > be configurable, right? AArch32 feature registers remain visible with
> > > > > their default values passed through to the guest. If you've already
> > > > > stated this as a precondition elsewhere then my apologies for the noise.
> > > > 
> > > > Aren't AArch64 ID regs architecturally mapped to their AArch32
> > > > counterparts?  They should show the same values.  I'm not sure if it's a
> > > > problem (and if KVM is faithful to that rule),
> > > 
> > > I believe it's a bit more subtle than that. The AArch32 feature registers
> > > are architecturally mapped to certain encodings accessible from AArch64.
> > > For example, ID_PFR0_EL1 is actually a 64 bit register where bits [31:0]
> > > map to the ID_PFR0 AArch32 register. ID_PFR0_EL1 is only accessible from
> > > AArch64 with the MRS instruction, and ID_PFR0 is only accessible from
> > > AArch32 with the MRC instruction. KVM just so happens to handle both of
> > > these reads from the same sys_reg_desc.

Ughhhhh.

We actually clear HCR_EL2.TID3 for AArch32 guests, so AArch32 EL1 reads
straight from hardware. Considering the work we put in to make sure
feature registers are consistent system-wide and the limitations on
certain features, this is plain wrong.

I have a series that addresses this but need to go find some 32 bit
hardware to test with :)

--
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com>
To: Ricardo Koller <ricarkol@google.com>
Cc: Reiji Watanabe <reijiw@google.com>, Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Will Deacon <will@kernel.org>, Andrew Jones <drjones@redhat.com>,
	Fuad Tabba <tabba@google.com>,
	Peng Liang <liangpeng10@huawei.com>,
	Peter Shier <pshier@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v6 11/25] KVM: arm64: Add remaining ID registers to id_reg_desc_table
Date: Thu, 24 Mar 2022 02:26:30 +0000	[thread overview]
Message-ID: <YjvW1lLT1sVZf0jK@google.com> (raw)
In-Reply-To: <YjueX2DOxjoc/d4j@google.com>

On Wed, Mar 23, 2022 at 10:25:35PM +0000, Oliver Upton wrote:
> On Wed, Mar 23, 2022 at 03:22:43PM -0700, Ricardo Koller wrote:
> > On Wed, Mar 23, 2022 at 08:44:26PM +0000, Oliver Upton wrote:
> > > On Wed, Mar 23, 2022 at 01:13:32PM -0700, Ricardo Koller wrote:
> > > > On Wed, Mar 23, 2022 at 07:53:14PM +0000, Oliver Upton wrote:
> > > > > Hi Reiji,
> > > > > 
> > > > > On Thu, Mar 10, 2022 at 08:47:57PM -0800, Reiji Watanabe wrote:
> > > > > > Add hidden or reserved ID registers, and remaining ID registers,
> > > > > > which don't require special handling, to id_reg_desc_table.
> > > > > > Add 'flags' field to id_reg_desc, which is used to indicates hiddden
> > > > > > or reserved registers. Since now id_reg_desc_init() is called even
> > > > > > for hidden/reserved registers, change it to not do anything for them.
> > > > > > 
> > > > > > Signed-off-by: Reiji Watanabe <reijiw@google.com>
> > > > > 
> > > > > I think there is a very important detail of the series that probably
> > > > > should be highlighted. We are only allowing AArch64 feature registers to
> > > > > be configurable, right? AArch32 feature registers remain visible with
> > > > > their default values passed through to the guest. If you've already
> > > > > stated this as a precondition elsewhere then my apologies for the noise.
> > > > 
> > > > Aren't AArch64 ID regs architecturally mapped to their AArch32
> > > > counterparts?  They should show the same values.  I'm not sure if it's a
> > > > problem (and if KVM is faithful to that rule),
> > > 
> > > I believe it's a bit more subtle than that. The AArch32 feature registers
> > > are architecturally mapped to certain encodings accessible from AArch64.
> > > For example, ID_PFR0_EL1 is actually a 64 bit register where bits [31:0]
> > > map to the ID_PFR0 AArch32 register. ID_PFR0_EL1 is only accessible from
> > > AArch64 with the MRS instruction, and ID_PFR0 is only accessible from
> > > AArch32 with the MRC instruction. KVM just so happens to handle both of
> > > these reads from the same sys_reg_desc.

Ughhhhh.

We actually clear HCR_EL2.TID3 for AArch32 guests, so AArch32 EL1 reads
straight from hardware. Considering the work we put in to make sure
feature registers are consistent system-wide and the limitations on
certain features, this is plain wrong.

I have a series that addresses this but need to go find some 32 bit
hardware to test with :)

--
Thanks,
Oliver

  reply	other threads:[~2022-03-24  2:26 UTC|newest]

Thread overview: 144+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-11  4:47 [PATCH v6 00/25] KVM: arm64: Make CPU ID registers writable by userspace Reiji Watanabe
2022-03-11  4:47 ` Reiji Watanabe
2022-03-11  4:47 ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 01/25] KVM: arm64: Introduce a validation function for an ID register Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-22  7:42   ` Oliver Upton
2022-03-22  7:42     ` Oliver Upton
2022-03-22  7:42     ` Oliver Upton
2022-03-23  6:06     ` Reiji Watanabe
2022-03-23  6:06       ` Reiji Watanabe
2022-03-23  6:06       ` Reiji Watanabe
2022-03-23  7:05       ` Oliver Upton
2022-03-23  7:05         ` Oliver Upton
2022-03-23  7:05         ` Oliver Upton
2022-03-24  6:00         ` Reiji Watanabe
2022-03-24  6:00           ` Reiji Watanabe
2022-03-24  6:00           ` Reiji Watanabe
2022-03-24  7:37           ` Oliver Upton
2022-03-24  7:37             ` Oliver Upton
2022-03-24  7:37             ` Oliver Upton
2022-03-29  1:57             ` Reiji Watanabe
2022-03-29  1:57               ` Reiji Watanabe
2022-03-29  1:57               ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 02/25] KVM: arm64: Save ID registers' sanitized value per guest Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-23 19:22   ` Oliver Upton
2022-03-23 19:22     ` Oliver Upton
2022-03-23 19:22     ` Oliver Upton
2022-03-24 16:23     ` Reiji Watanabe
2022-03-24 16:23       ` Reiji Watanabe
2022-03-24 16:23       ` Reiji Watanabe
2022-03-24 17:54       ` Oliver Upton
2022-03-24 17:54         ` Oliver Upton
2022-03-24 17:54         ` Oliver Upton
2022-03-26  2:35         ` Reiji Watanabe
2022-03-26  2:35           ` Reiji Watanabe
2022-03-26  2:35           ` Reiji Watanabe
2022-03-27 22:57           ` Oliver Upton
2022-03-27 22:57             ` Oliver Upton
2022-03-27 22:57             ` Oliver Upton
2022-03-28  0:04             ` Reiji Watanabe
2022-03-28  0:04               ` Reiji Watanabe
2022-03-28  0:04               ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 03/25] KVM: arm64: Introduce struct id_reg_desc Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 04/25] KVM: arm64: Make ID_AA64PFR0_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 05/25] KVM: arm64: Make ID_AA64PFR1_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 06/25] KVM: arm64: Make ID_AA64ISAR0_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 07/25] KVM: arm64: Make ID_AA64ISAR1_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 08/25] KVM: arm64: Make ID_AA64MMFR0_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 09/25] KVM: arm64: Make ID_AA64DFR0_EL1/ID_DFR0_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 10/25] KVM: arm64: Make MVFR1_EL1 writable Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 11/25] KVM: arm64: Add remaining ID registers to id_reg_desc_table Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-23 19:53   ` Oliver Upton
2022-03-23 19:53     ` Oliver Upton
2022-03-23 19:53     ` Oliver Upton
2022-03-23 20:13     ` Ricardo Koller
2022-03-23 20:13       ` Ricardo Koller
2022-03-23 20:13       ` Ricardo Koller
2022-03-23 20:44       ` Oliver Upton
2022-03-23 20:44         ` Oliver Upton
2022-03-23 20:44         ` Oliver Upton
2022-03-23 22:22         ` Ricardo Koller
2022-03-23 22:22           ` Ricardo Koller
2022-03-23 22:22           ` Ricardo Koller
2022-03-23 22:25           ` Oliver Upton
2022-03-23 22:25             ` Oliver Upton
2022-03-23 22:25             ` Oliver Upton
2022-03-24  2:26             ` Oliver Upton [this message]
2022-03-24  2:26               ` Oliver Upton
2022-03-24  2:26               ` Oliver Upton
2022-03-24 20:23     ` Reiji Watanabe
2022-03-24 20:23       ` Reiji Watanabe
2022-03-24 20:23       ` Reiji Watanabe
2022-03-24 23:01       ` Oliver Upton
2022-03-24 23:01         ` Oliver Upton
2022-03-24 23:01         ` Oliver Upton
2022-03-25  5:15         ` Reiji Watanabe
2022-03-25  5:15           ` Reiji Watanabe
2022-03-25  5:15           ` Reiji Watanabe
2022-03-25  8:51           ` Oliver Upton
2022-03-25  8:51             ` Oliver Upton
2022-03-25  8:51             ` Oliver Upton
2022-03-11  4:47 ` [PATCH v6 12/25] KVM: arm64: Use id_reg_desc_table for ID registers Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47 ` [PATCH v6 13/25] KVM: arm64: Add consistency checking for frac fields of " Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:47   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 14/25] KVM: arm64: Introduce KVM_CAP_ARM_ID_REG_CONFIGURABLE capability Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 15/25] KVM: arm64: Add kunit test for ID register validation Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 16/25] KVM: arm64: Use vcpu->arch cptr_el2 to track value of cptr_el2 for VHE Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 17/25] KVM: arm64: Use vcpu->arch.mdcr_el2 to track value of mdcr_el2 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 18/25] KVM: arm64: Introduce framework to trap disabled features Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 19/25] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 20/25] KVM: arm64: Trap disabled features of ID_AA64PFR1_EL1 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 21/25] KVM: arm64: Trap disabled features of ID_AA64DFR0_EL1 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 22/25] KVM: arm64: Trap disabled features of ID_AA64MMFR1_EL1 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 23/25] KVM: arm64: Trap disabled features of ID_AA64ISAR1_EL1 Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 24/25] KVM: arm64: Add kunit test for trap initialization Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48 ` [PATCH v6 25/25] KVM: arm64: selftests: Introduce id_reg_test Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe
2022-03-11  4:48   ` Reiji Watanabe

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=YjvW1lLT1sVZf0jK@google.com \
    --to=oupton@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=pshier@google.com \
    --cc=ricarkol@google.com \
    --cc=will@kernel.org \
    /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.