Hi Eric, On Mon, 07 Aug 2023 18:19:16 +0100, Eric Auger <eric.auger@xxxxxxxxxx> wrote: > > Hi Marc, > On 7/28/23 10:29, Marc Zyngier wrote: > > ... and finally, the Debug version of FGT, with its *enormous* > > list of trapped registers. > > > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx> > > Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx> > Hi Marc, I think you mixed up with the R-b's sent on > [PATCH v2 06/26] arm64: Add debug registers affected by HDFGxTR_EL2 Most probably. Really sorry about that. I'll fix that right away. > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > > --- > > arch/arm64/include/asm/kvm_arm.h | 11 + > > arch/arm64/kvm/emulate-nested.c | 460 +++++++++++++++++++++++++++++++ [...] > > + SR_FGT(SYS_TRCIMSPEC0, HDFGRTR, TRCIMSPECn, 1), > why not SYS_TRCIMSPEC(0)? Yup, it's been mentioned a couple of time already, now fixed. > > + SR_FGT(SYS_TRCIMSPEC(1), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(2), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(3), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(4), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(5), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(6), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCIMSPEC(7), HDFGRTR, TRCIMSPECn, 1), > > + SR_FGT(SYS_TRCDEVARCH, HDFGRTR, TRCID, 1), > > + SR_FGT(SYS_TRCDEVID, HDFGRTR, TRCID, 1), > what about all of the TRCIDR regs refered to in the spec? Ah, nothing escapes you! Yup totally missed it, now added. [...] > > + SR_FGT(SYS_TRCRSCTLR(2), HDFGRTR, TRC, 1),y > > + SR_FGT(SYS_TRCRSCTLR(3), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(4), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(5), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(6), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(7), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(8), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(9), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(10), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(11), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(12), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(13), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(14), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(15), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(16), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(17), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(18), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(19), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(20), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(21), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(22), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(23), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(24), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(25), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(26), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(27), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(28), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(29), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(30), HDFGRTR, TRC, 1), > > + SR_FGT(SYS_TRCRSCTLR(31), HDFGRTR, TRC, 1),y > > + SR_FGT(SYS_TRCQCTLR, HDFGRTR, TRC, 1), > nit: maybe put this one before the > > SYS_TRCRSCTLR(n) series to follow the spec order Done. [...] > > + /* > > + * HDFGWTR_EL2 > > + * > > + * Although HDFGRTR_EL2 and HDFGWTR_EL2 registers largely > > + * overlap in their bit assignment, there are a number of bits > > + * that are RES0 on one side, and an actual trap bit on the > > + * other. The policy chosen here is to describe all the > > + * read-side mappings, and only the write-side mappings that > > + * differ from the write side, and the trap handler will pick > differ from the read side? Well spotted! Now fixed. Thanks again! M. -- Without deviation from the norm, progress is not possible.
