| Message ID | 20240122201852.262057-9-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM/arm64: VM configuration enforcement | expand |
Hi, On Mon, Jan 22, 2024 at 08:18:35PM +0000, Marc Zyngier wrote: > There is no reason to have separate FGT group identifiers for > the debug fine grain trapping. The sole requirement is to provide > the *names* so that the SR_FGF() macro can do its magic of picking > the correct bit definition. > > So let's alias HDFGWTR_GROUP and HDFGRTR_GROUP. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/emulate-nested.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > index 7a4a886adb9d..8a1cfcf553a2 100644 > --- a/arch/arm64/kvm/emulate-nested.c > +++ b/arch/arm64/kvm/emulate-nested.c > @@ -1010,7 +1010,7 @@ enum fgt_group_id { > __NO_FGT_GROUP__, > HFGxTR_GROUP, > HDFGRTR_GROUP, > - HDFGWTR_GROUP, > + HDFGWTR_GROUP = HDFGRTR_GROUP, > HFGITR_GROUP, > HAFGRTR_GROUP, > > @@ -1938,7 +1938,6 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu) > break; > > case HDFGRTR_GROUP: > - case HDFGWTR_GROUP: > if (is_read) > val = __vcpu_sys_reg(vcpu, HDFGRTR_EL2); > else I guess you could rename it to HDFGxTR_GROUP like for HFGxTR_GROUP but that means changing all those tables, so I think it's fine. Reviewed-by: Joey Gouly <joey.gouly@arm.com> Thanks, Joey
On Tue, 23 Jan 2024 14:14:33 +0000, Joey Gouly <joey.gouly@arm.com> wrote: > > Hi, > > On Mon, Jan 22, 2024 at 08:18:35PM +0000, Marc Zyngier wrote: > > There is no reason to have separate FGT group identifiers for > > the debug fine grain trapping. The sole requirement is to provide > > the *names* so that the SR_FGF() macro can do its magic of picking > > the correct bit definition. > > > > So let's alias HDFGWTR_GROUP and HDFGRTR_GROUP. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kvm/emulate-nested.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > > index 7a4a886adb9d..8a1cfcf553a2 100644 > > --- a/arch/arm64/kvm/emulate-nested.c > > +++ b/arch/arm64/kvm/emulate-nested.c > > @@ -1010,7 +1010,7 @@ enum fgt_group_id { > > __NO_FGT_GROUP__, > > HFGxTR_GROUP, > > HDFGRTR_GROUP, > > - HDFGWTR_GROUP, > > + HDFGWTR_GROUP = HDFGRTR_GROUP, > > HFGITR_GROUP, > > HAFGRTR_GROUP, > > > > @@ -1938,7 +1938,6 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu) > > break; > > > > case HDFGRTR_GROUP: > > - case HDFGWTR_GROUP: > > if (is_read) > > val = __vcpu_sys_reg(vcpu, HDFGRTR_EL2); > > else > > I guess you could rename it to HDFGxTR_GROUP like for HFGxTR_GROUP but that > means changing all those tables, so I think it's fine. Not just the tables, but also the arch/arm64/tools/sysreg, which distinguishes between HDFGRTR and HDFGWTR. I'm pretty sure it isn't worth the hassle... > Reviewed-by: Joey Gouly <joey.gouly@arm.com> Thanks! M.
On Tue, Jan 23, 2024 at 03:03:03PM +0000, Marc Zyngier wrote: > Joey Gouly <joey.gouly@arm.com> wrote: > > > case HDFGRTR_GROUP: > > > - case HDFGWTR_GROUP: > > > if (is_read) > > > val = __vcpu_sys_reg(vcpu, HDFGRTR_EL2); > > > else > > I guess you could rename it to HDFGxTR_GROUP like for HFGxTR_GROUP but that > > means changing all those tables, so I think it's fine. > Not just the tables, but also the arch/arm64/tools/sysreg, which > distinguishes between HDFGRTR and HDFGWTR. I'm pretty sure it isn't > worth the hassle... Quickly checking the current definitions in the kernel there's differences between the traps (eg, HDFGRTR_EL2 has bit 63 PMBIDR_EL1 but that's RES0 for the write registers and there's others). Some of them are just no write traps for read only registers but some it's not so immediately obvious.
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c index 7a4a886adb9d..8a1cfcf553a2 100644 --- a/arch/arm64/kvm/emulate-nested.c +++ b/arch/arm64/kvm/emulate-nested.c @@ -1010,7 +1010,7 @@ enum fgt_group_id { __NO_FGT_GROUP__, HFGxTR_GROUP, HDFGRTR_GROUP, - HDFGWTR_GROUP, + HDFGWTR_GROUP = HDFGRTR_GROUP, HFGITR_GROUP, HAFGRTR_GROUP, @@ -1938,7 +1938,6 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu) break; case HDFGRTR_GROUP: - case HDFGWTR_GROUP: if (is_read) val = __vcpu_sys_reg(vcpu, HDFGRTR_EL2); else
There is no reason to have separate FGT group identifiers for the debug fine grain trapping. The sole requirement is to provide the *names* so that the SR_FGF() macro can do its magic of picking the correct bit definition. So let's alias HDFGWTR_GROUP and HDFGRTR_GROUP. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/emulate-nested.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)