From: Marc Zyngier <maz@kernel.org> To: Peter Maydell <peter.maydell@linaro.org> Cc: Will Deacon <will@kernel.org>, QEMU Developers <qemu-devel@nongnu.org>, kvmarm@lists.cs.columbia.edu Subject: Re: [PATCH] target/arm: Honor HCR_EL2.TID3 trapping requirements Date: Mon, 25 Nov 2019 17:08:15 +0000 [thread overview] Message-ID: <4d8c4763da39d5bfb1800735f90d82d1@www.loen.fr> (raw) In-Reply-To: <CAFEAcA_MQpJ=8UFnP=Qnt+4GWMUO_AtJBBNz-bM2L2wf5htuaQ@mail.gmail.com> On 2019-11-25 16:21, Peter Maydell wrote: > On Sat, 23 Nov 2019 at 11:56, Marc Zyngier <maz@kernel.org> 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@kernel.org> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> --- >> 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...
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org> To: Peter Maydell <peter.maydell@linaro.org> Cc: Will Deacon <will@kernel.org>, QEMU Developers <qemu-devel@nongnu.org>, kvmarm@lists.cs.columbia.edu Subject: Re: [PATCH] target/arm: Honor HCR_EL2.TID3 trapping requirements Date: Mon, 25 Nov 2019 17:08:15 +0000 [thread overview] Message-ID: <4d8c4763da39d5bfb1800735f90d82d1@www.loen.fr> (raw) In-Reply-To: <CAFEAcA_MQpJ=8UFnP=Qnt+4GWMUO_AtJBBNz-bM2L2wf5htuaQ@mail.gmail.com> On 2019-11-25 16:21, Peter Maydell wrote: > On Sat, 23 Nov 2019 at 11:56, Marc Zyngier <maz@kernel.org> 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@kernel.org> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> --- >> 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@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply other threads:[~2019-11-25 17:16 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-23 11:56 [PATCH] target/arm: Honor HCR_EL2.TID3 trapping requirements Marc Zyngier 2019-11-23 11:56 ` Marc Zyngier 2019-11-25 10:40 ` Will Deacon 2019-11-25 10:40 ` Will Deacon 2019-11-25 10:59 ` Marc Zyngier 2019-11-25 10:59 ` Marc Zyngier 2019-11-25 16:21 ` Peter Maydell 2019-11-25 16:21 ` Peter Maydell 2019-11-25 17:08 ` Marc Zyngier [this message] 2019-11-25 17:08 ` Marc Zyngier 2019-11-25 17:27 ` Peter Maydell 2019-11-25 17:27 ` Peter Maydell 2019-11-25 17:49 ` Marc Zyngier 2019-11-25 17:49 ` Marc Zyngier 2019-11-26 12:46 ` Peter Maydell 2019-11-26 12:46 ` Peter Maydell 2019-11-26 10:12 ` Richard Henderson 2019-11-26 10:12 ` Richard Henderson 2019-11-26 13:19 ` Peter Maydell 2019-11-26 13:19 ` Peter Maydell 2019-11-26 21:04 ` Richard Henderson 2019-11-26 21:04 ` Richard Henderson 2019-11-27 9:13 ` Marc Zyngier 2019-11-27 9:13 ` Marc Zyngier
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=4d8c4763da39d5bfb1800735f90d82d1@www.loen.fr \ --to=maz@kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=peter.maydell@linaro.org \ --cc=qemu-devel@nongnu.org \ --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: linkBe 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.