Re: [PATCH v8 03/28] arm64: mte: CPU feature detection and initial sysreg configuration

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Aug 25, 2020 at 02:53:47PM +0100, Marc Zyngier wrote:
> On 2020-08-25 11:54, Catalin Marinas wrote:
> > On Tue, Aug 25, 2020 at 09:53:16AM +0100, Marc Zyngier wrote:
> > > On 2020-08-24 19:27, Catalin Marinas wrote:
> > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > > index 077293b5115f..59b91f58efec 100644
> > > > --- a/arch/arm64/kvm/sys_regs.c
> > > > +++ b/arch/arm64/kvm/sys_regs.c
> > > > @@ -1131,6 +1131,8 @@ static u64 read_id_reg(const struct kvm_vcpu
> > > > *vcpu,
> > > >  		if (!vcpu_has_sve(vcpu))
> > > >  			val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
> > > >  		val &= ~(0xfUL << ID_AA64PFR0_AMU_SHIFT);
> > > > +	} else if (id == SYS_ID_AA64PFR1_EL1) {
> > > > +		val &= ~(0xfUL << ID_AA64PFR1_MTE_SHIFT);
> > > 
> > > Hiding the capability is fine, but where is the handling of trapping
> > > instructions done? They should result in an UNDEF being injected.
> > 
> > They are a few new MTE-specific MSR/MRS which are trapped at EL2 but
> > since KVM doesn't understand them yet, shouldn't it already inject
> > undef back at EL1? That would be safer regardless of MTE support.
> 
> An UNDEF will be injected, but not without spitting a nastygram in
> the kernel log (look at emulate_sys_reg()).
> 
> The best course of action is to have an entry in the sysreg table
> that would explicitly do the handling.

Something like below. I'll put them in a separate patch, to be reverted
when we get proper MTE support in KVM.

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 59b91f58efec..c7d5d1bae044 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1384,6 +1384,13 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	return true;
 }
 
+static bool access_mte_regs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	kvm_inject_undefined(vcpu);
+	return false;
+}
+
 /* sys_reg_desc initialiser for known cpufeature ID registers */
 #define ID_SANITISED(name) {			\
 	SYS_DESC(SYS_##name),			\
@@ -1549,6 +1556,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_SCTLR_EL1), access_vm_reg, reset_val, SCTLR_EL1, 0x00C50078 },
 	{ SYS_DESC(SYS_ACTLR_EL1), access_actlr, reset_actlr, ACTLR_EL1 },
 	{ SYS_DESC(SYS_CPACR_EL1), NULL, reset_val, CPACR_EL1, 0 },
+
+	{ SYS_DESC(SYS_RGSR_EL1), access_mte_regs },
+	{ SYS_DESC(SYS_GCR_EL1), access_mte_regs },
+
 	{ SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility },
 	{ SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
 	{ SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
@@ -1573,6 +1584,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
 	{ SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
 
+	{ SYS_DESC(SYS_TFSR_EL1), access_mte_regs },
+	{ SYS_DESC(SYS_TFSRE0_EL1), access_mte_regs },
+
 	{ SYS_DESC(SYS_FAR_EL1), access_vm_reg, reset_unknown, FAR_EL1 },
 	{ SYS_DESC(SYS_PAR_EL1), NULL, reset_unknown, PAR_EL1 },
 

(still testing, it takes ages to boot a VM inside inside FVP)

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel