[PATCH 16/27] KVM: arm64: nv: Add trap forwarding for HCR_EL2
Marc Zyngier
maz at kernel.org
Thu Jul 13 08:53:40 PDT 2023
Hey Eric,
Thanks for looking into this, much appreciated given how tedious it
is.
On Thu, 13 Jul 2023 15:05:33 +0100,
Eric Auger <eric.auger at redhat.com> wrote:
>
> Hi Marc,
>
> On 7/12/23 16:57, Marc Zyngier wrote:
> > Describe the HCR_EL2 register, and associate it with all the sysregs
> > it allows to trap.
> >
> > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > ---
> > arch/arm64/kvm/emulate-nested.c | 475 ++++++++++++++++++++++++++++++++
> > 1 file changed, 475 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index 5bab2e85d70c..51901e85e43d 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
[...]
> > + [CGT_HCR_TPC] = {
> modern revisions now refer to TPCP, maybe worth a comment?
Absolutely.
[...]
> > + SR_RANGE_TRAP(SYS_ID_PFR0_EL1,
> > + sys_reg(3, 0, 0, 7, 7), CGT_HCR_TID3),
> in the spec I see this upper limit in the FEAT_FGT section. Out of
> curiosity how were you able to convert the sys reg names into this Op0,
> Op1, CRn, CRm, Op2. Is there any ordering logic documented somewhere for
> those group3 regs?
If you look at the sysreg encoding described on page D18-6308 if
version J.a of the ARM ARM, you will find a block of 56 contiguous
encodings ranging from (3, 0, 0, 1, 0), which happens to be
ID_PFR0_EL1, all the way to a reserved range ending in (3, 0, 0, 7,
7).
This is the block of register that is controlled by TID3.
> I checked Table D18-2 and this looks good but I wonder if there isn't
> any more efficient way to review this.
Not that I know of, unfortunately. Even the pseudocode isn't enough
for this as it doesn't described the trapping of unallocated regions.
> > + SR_TRAP(SYS_ICC_SGI0R_EL1, CGT_HCR_IMO_FMO),
> > + SR_TRAP(SYS_ICC_ASGI1R_EL1, CGT_HCR_IMO_FMO),
> > + SR_TRAP(SYS_ICC_SGI1R_EL1, CGT_HCR_IMO_FMO),
> > + SR_RANGE_TRAP(sys_reg(3, 0, 11, 0, 0),
> > + sys_reg(3, 0, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 1, 11, 0, 0),
> > + sys_reg(3, 1, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 2, 11, 0, 0),
> > + sys_reg(3, 2, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 3, 11, 0, 0),
> > + sys_reg(3, 3, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 4, 11, 0, 0),
> > + sys_reg(3, 4, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 5, 11, 0, 0),
> > + sys_reg(3, 5, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 6, 11, 0, 0),
> > + sys_reg(3, 6, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 7, 11, 0, 0),
> > + sys_reg(3, 7, 11, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 0, 15, 0, 0),
> > + sys_reg(3, 0, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 1, 15, 0, 0),
> > + sys_reg(3, 1, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 2, 15, 0, 0),
> > + sys_reg(3, 2, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 3, 15, 0, 0),
> > + sys_reg(3, 3, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 4, 15, 0, 0),
> > + sys_reg(3, 4, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 5, 15, 0, 0),
> > + sys_reg(3, 5, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 6, 15, 0, 0),
> > + sys_reg(3, 6, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_RANGE_TRAP(sys_reg(3, 7, 15, 0, 0),
> > + sys_reg(3, 7, 15, 15, 7), CGT_HCR_TIDCP),
> > + SR_TRAP(SYS_ACTLR_EL1, CGT_HCR_TACR),
> > + SR_TRAP(SYS_DC_ISW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CISW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_IGSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_IGDSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CGSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CGDSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CIGSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CIGDSW, CGT_HCR_TSW),
> > + SR_TRAP(SYS_DC_CIVAC, CGT_HCR_TPC),
> I don't see CVADP?
Me neither! Good catch!
[...]
> > + SR_TRAP(SYS_SCTLR_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_TTBR0_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_TTBR1_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_TCR_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_ESR_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_FAR_EL1, CGT_HCR_TVM_TRVM),
> > + SR_TRAP(SYS_AFSR0_EL1, CGT_HCR_TVM_TRVM),*
> Looking at the SFSR0_EL1 MRS/MSR pseudo code I understand TRVM is tested
> on read and
> TVM is tested on write. However CGT_HCR_TVM has FORWARD_ANY behaviour
> while TRVM looks good as FORWARD_READ? Do I miss something.
You're not missing anything. For some reason, I had in my head that
TVM was trapping both reads and writes, while the spec is clear that
it only traps writes.
>
> > + SR_TRAP(SYS_AFSR1_EL1, CGT_HCR_TVM_TRVM),*
> same here and below
Yup, I need to fix the TVM encoding like this:
@@ -176,7 +176,7 @@ static const struct trap_bits coarse_trap_bits[] = {
.index = HCR_EL2,
.value = HCR_TVM,
.mask = HCR_TVM,
- .behaviour = BEHAVE_FORWARD_ANY,
+ .behaviour = BEHAVE_FORWARD_WRITE,
},
[CGT_HCR_TDZ] = {
.index = HCR_EL2,
[...]
> > + /* All _EL2 registers */
> > + SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
> > + sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
> > + SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
> > + sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
> > + /* All _EL02, _EL12 registers */
> > + SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
> > + sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
> > + SR_RANGE_TRAP(sys_reg(3, 5, 12, 0, 0),
> > + sys_reg(3, 5, 14, 15, 7), CGT_HCR_NV),
> same question as bove, where in the ARM ARM do you find those
> ranges?
I went over the encoding with a fine comb, and realised that all the
(3, 4, ...) encodings are EL2, and all the (3, 5, ...) ones are EL02
and EL12.
I appreciate that this is taking a massive bet on the future, but
there is no such rule in the ARM ARM as such...
> > + SR_TRAP(SYS_SP_EL1, CGT_HCR_NV),*
> > + SR_TRAP(OP_AT_S1E2R, CGT_HCR_NV),*
> > + SR_TRAP(OP_AT_S1E2W, CGT_HCR_NV),*
> > + SR_TRAP(OP_AT_S12E1R, CGT_HCR_NV),*
> > + SR_TRAP(OP_AT_S12E1W, CGT_HCR_NV),*
> > + SR_TRAP(OP_AT_S12E0R, CGT_HCR_NV),*
> according to the pseudo code NV2 is not checked
> shouldn't we have a separate CGT? Question also valid for a bunch of ops
> below
Hmmm. Yes, this is wrong. Well spotted. I guess I need a
CGT_HCR_NV_nNV2 for the cases that want that particular condition (NV1
probably needs a similar fix).
[...]
> CIGDPAE?
> CIPAE?
These two are part of RME (well, technically MEC, but that an RME
thing), and I have no plan to support this with NV -- yet.
> CFP/CPP/DVP RCTX?
These are definitely missing. I'll add them.
Thanks again for going through this list, this is awesome work!
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list