| Message ID | 20241009190019.3222687-34-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Add EL2 support to FEAT_S1PIE/S1POE | expand |
On Wed, Oct 09, 2024 at 08:00:16PM +0100, Marc Zyngier wrote: > The hierarchical permissions must be disabled when POE is enabled > in the translation regime used for a given table walk. > > We store the two enable bits in the s1_walk_info structure so that > they can be retrieved down the line, as they will be useful. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c > index 4921284eeedff..301399f17983f 100644 > --- a/arch/arm64/kvm/at.c > +++ b/arch/arm64/kvm/at.c > @@ -24,6 +24,8 @@ struct s1_walk_info { > unsigned int txsz; > int sl; > bool hpd; > + bool e0poe; > + bool poe; > bool be; > bool s2; > }; > @@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) > } > } > > +static void compute_s1poe(struct kvm_vcpu *vcpu, struct s1_walk_info *wi) > +{ > + u64 val; > + > + if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) { nit: kvm_has_s1pie() > + wi->poe = wi->e0poe = false; > + return; > + } > + > + switch (wi->regime) { > + case TR_EL2: > + case TR_EL20: > + val = vcpu_read_sys_reg(vcpu, TCR2_EL2); > + wi->poe = val & TCR2_EL2_POE; > + wi->e0poe = val & TCR2_EL2_E0POE; Hmm... E0POE is always false in the EL2 translation regime. The RES0 mask does the heavy lifting here, but that only works if we force userspace to select an nVHE-only or VHE-only vCPU. It might make sense to have TR_EL2 force this to false to make it a bit more self-documenting, albeit not a functional issue. > + break; > + case TR_EL10: > + if (__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_TCR2En) { > + wi->poe = wi->e0poe = false; > + return; > + } > + > + val = __vcpu_sys_reg(vcpu, TCR2_EL1); > + wi->poe = val & TCR2_EL1x_POE; > + wi->e0poe = val & TCR2_EL1x_E0POE; > + } > +} > +
On Thu, 10 Oct 2024 09:08:13 +0100, Oliver Upton <oliver.upton@linux.dev> wrote: > > On Wed, Oct 09, 2024 at 08:00:16PM +0100, Marc Zyngier wrote: > > The hierarchical permissions must be disabled when POE is enabled > > in the translation regime used for a given table walk. > > > > We store the two enable bits in the s1_walk_info structure so that > > they can be retrieved down the line, as they will be useful. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > > > diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c > > index 4921284eeedff..301399f17983f 100644 > > --- a/arch/arm64/kvm/at.c > > +++ b/arch/arm64/kvm/at.c > > @@ -24,6 +24,8 @@ struct s1_walk_info { > > unsigned int txsz; > > int sl; > > bool hpd; > > + bool e0poe; > > + bool poe; > > bool be; > > bool s2; > > }; > > @@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) > > } > > } > > > > +static void compute_s1poe(struct kvm_vcpu *vcpu, struct s1_walk_info *wi) > > +{ > > + u64 val; > > + > > + if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) { > > nit: kvm_has_s1pie() Nah, that's a gold plated bug, and really should check for S1POE. I guess I'll add a helper (kvm_has_s1poe), and use that throughout. > > > + wi->poe = wi->e0poe = false; > > + return; > > + } > > + > > + switch (wi->regime) { > > + case TR_EL2: > > + case TR_EL20: > > + val = vcpu_read_sys_reg(vcpu, TCR2_EL2); > > + wi->poe = val & TCR2_EL2_POE; > > + wi->e0poe = val & TCR2_EL2_E0POE; > > Hmm... E0POE is always false in the EL2 translation regime. The RES0 > mask does the heavy lifting here, but that only works if we force > userspace to select an nVHE-only or VHE-only vCPU. Which is the plan of record, but yeah, you can't work that out from this snippet. > It might make sense to have TR_EL2 force this to false to make it a bit > more self-documenting, albeit not a functional issue. Sure, I'll add a mask if that helps making sense of the whole thing. Thanks, M.
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c index 4921284eeedff..301399f17983f 100644 --- a/arch/arm64/kvm/at.c +++ b/arch/arm64/kvm/at.c @@ -24,6 +24,8 @@ struct s1_walk_info { unsigned int txsz; int sl; bool hpd; + bool e0poe; + bool poe; bool be; bool s2; }; @@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) } } +static void compute_s1poe(struct kvm_vcpu *vcpu, struct s1_walk_info *wi) +{ + u64 val; + + if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) { + wi->poe = wi->e0poe = false; + return; + } + + switch (wi->regime) { + case TR_EL2: + case TR_EL20: + val = vcpu_read_sys_reg(vcpu, TCR2_EL2); + wi->poe = val & TCR2_EL2_POE; + wi->e0poe = val & TCR2_EL2_E0POE; + break; + case TR_EL10: + if (__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_TCR2En) { + wi->poe = wi->e0poe = false; + return; + } + + val = __vcpu_sys_reg(vcpu, TCR2_EL1); + wi->poe = val & TCR2_EL1x_POE; + wi->e0poe = val & TCR2_EL1x_E0POE; + } +} + static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi, struct s1_walk_result *wr, u64 va) { @@ -206,6 +236,12 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi, /* R_JHSVW */ wi->hpd |= s1pie_enabled(vcpu, wi->regime); + /* Do we have POE? */ + compute_s1poe(vcpu, wi); + + /* R_BVXDG */ + wi->hpd |= (wi->poe || wi->e0poe); + /* Someone was silly enough to encode TG0/TG1 differently */ if (va55) { wi->txsz = FIELD_GET(TCR_T1SZ_MASK, tcr);
The hierarchical permissions must be disabled when POE is enabled in the translation regime used for a given table walk. We store the two enable bits in the s1_walk_info structure so that they can be retrieved down the line, as they will be useful. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)