On 2019-11-25 16:21, Peter Maydell wrote:
On Sat, 23 Nov 2019 at 11:56, Marc Zyngier <maz@xxxxxxxxxx> wrote:
HCR_EL2.TID3 mandates that access from EL1 to a long list of id
registers traps to EL2, and QEMU has so far ignored this
requirement.
This breaks (among other things) KVM guests that have PtrAuth
enabled,
while the hypervisor doesn't want to expose the feature to its
guest.
To achieve this, KVM traps the ID registers (ID_AA64ISAR1_EL1 in
this
case), and masks out the unsupported feature.
QEMU not honoring the trap request means that the guest observes
that the feature is present in the HW, starts using it, and dies
a horrible death when KVM injects an UNDEF, because the feature
*really* isn't supported.
Do the right thing by trapping to EL2 if HCR_EL2.TID3 is set.
Reported-by: Will Deacon <will@xxxxxxxxxx>
Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
There is a number of other trap bits missing (TID[0-2], for
example),
but this at least gets a mainline Linux going with cpu=max.
I guess this ought to go into 4.2, given that it gets recent
Linux guest kernels to work.
@@ -6185,11 +6221,13 @@ void register_cp_regs_for_features(ARMCPU
*cpu)
{ .name = "ID_AA64PFR0_EL1", .state =
ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
.access = PL1_R, .type = ARM_CP_NO_RAW,
+ .accessfn = access_aa64idreg,
.readfn = id_aa64pfr0_read,
.writefn = arm_cp_write_ignore },
{ .name = "ID_AA64PFR1_EL1", .state =
ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 1,
.access = PL1_R, .type = ARM_CP_CONST,
+ .accessfn = access_aa64idreg,
.resetvalue = cpu->isar.id_aa64pfr1},
{ .name = "ID_AA64PFR2_EL1_RESERVED", .state =
ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 2,
@@ -6198,151 +6236,188 @@ void register_cp_regs_for_features(ARMCPU
*cpu)
{ .name = "ID_AA64PFR3_EL1_RESERVED", .state =
ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 3,
.access = PL1_R, .type = ARM_CP_CONST,
+ .accessfn = access_aa64idreg,
.resetvalue = 0 },
Missing .accessfn for ID_AA4PFR2_EL1_RESERVED ?
Indeed, I oversaw that one. I'll fix it and repost it together with
the extra patches to handle TID1 and TID2.
{ .name = "ID_AA64ZFR0_EL1", .state =
ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 4,
.access = PL1_R, .type = ARM_CP_CONST,
+ .accessfn = access_aa64idreg,
/* At present, only SVEver == 0 is defined anyway.
*/
.resetvalue = 0 },
{ .name = "MVFR0_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 0,
.access = PL1_R, .type = ARM_CP_CONST,
+ .accessfn = access_aa64idreg,
.resetvalue = cpu->isar.mvfr0 },
These are the AArch64 accessors for the aarch32 MVFR registers,
but this trap bit is also supposed to trap aarch32 accesses to
the MVFR registers, which are via the vmrs insn. That needs
to be done in trans_VMSR_VMRS(); we can do that with a
followup patch, since the mechanism there is completely different.
Holy cow! I'm afraid that my newly acquired QEMU-foo is missing
a few key options. Does it mean we need to generate a trapping
instruction, as opposed to take the exception on access?
I'd very much appreciate some guidance here.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
