linux-kernel - Re: [RFC 5/8] KVM: arm64: Explicitly handle MDSELR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5a2a74b3-f6cd-4cb6-8ee8-5dd7dc2bd686@arm.com>
Date: Fri, 12 Apr 2024 08:11:23 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Marc Zyngier <maz@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, Jonathan Corbet <corbet@....net>,
 Oliver Upton <oliver.upton@...ux.dev>, James Morse <james.morse@....com>,
 Suzuki K Poulose <suzuki.poulose@....com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Mark Brown <broonie@...nel.org>, Mark Rutland <mark.rutland@....com>,
 kvmarm@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [RFC 5/8] KVM: arm64: Explicitly handle MDSELR_EL1 traps as
 UNDEFINED



On 4/5/24 15:45, Marc Zyngier wrote:
> On Fri, 05 Apr 2024 09:00:05 +0100,
> Anshuman Khandual <anshuman.khandual@....com> wrote:
>>
>> Currently read_sanitised_id_aa64dfr0_el1() caps the ID_AA64DFR0.DebugVer to
>> ID_AA64DFR0_DebugVer_V8P8, resulting in FEAT_Debugv8p9 not being exposed to
>> the guest. MDSELR_EL1 register access in the guest, is currently trapped by
>> the existing configuration of the fine-grained traps.
> 
> Please add support for the HDFGxTR2_EL2 registers in the trap routing
> arrays, add support for the corresponding FGUs in the corresponding

Afraid that I might not have enough background here to sufficiently understand
your suggestion above, but nonetheless here is an attempt in this regard.

- Add HDFGRTR2_EL2/HDFGWTR2_EL2 to enum vcpu_sysreg
	enum vcpu_sysreg {
		..........
		VNCR(HDFGRTR2_EL2),
		VNCR(HDFGWTR2_EL2),
		..........
	}

- Add their VNCR mappings addresses

	#define VNCR_HDFGRTR2_EL2      0x1A0
	#define VNCR_HDFGWTR2_EL2      0x1B0

- Add HDFGRTR2_EL2/HDFGWTR2_EL2 to sys_reg_descs[]

static const struct sys_reg_desc sys_reg_descs[] = {
	..........
	EL2_REG_VNCR(HDFGRTR2_EL2, reset_val, 0),
	EL2_REG_VNCR(HDFGWTR2_EL2, reset_val, 0),
	..........
}

- Add HDFGRTR2_GROUP to enum fgt_group_id
- Add HDFGRTR2_GROUP to reg_to_fgt_group_id()
- Update triage_sysreg_trap() for HDFGRTR2_GROUP
- Update __activate_traps_hfgxtr() both for HDFGRTR2_EL2 and HDFGWTR2_EL2
- Updated __deactivate_traps_hfgxtr() both for HDFGRTR2_EL2 and HDFGWTR2_EL2

> structure, and condition the UNDEF on the lack of *guest* support for
> the feature.

Does something like the following looks OK for preventing guest access into
MDSELR_EL1 instead ?

--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1711,6 +1711,19 @@ static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
        return val;
 }
 
+static bool trap_mdselr_el1(struct kvm_vcpu *vcpu,
+                          struct sys_reg_params *p,
+                          const struct sys_reg_desc *r)
+{
+       u64 dfr0 = read_sanitised_id_aa64dfr0_el1(vcpu, r);
+       int dver = cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_EL1_DebugVer_SHIFT);
+
+       if (dver != ID_AA64DFR0_EL1_DebugVer_V8P9)
+               return undef_access(vcpu, p, r);
+
+       return true;
+}
+
 static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
                               const struct sys_reg_desc *rd,
                               u64 val)
@@ -2203,7 +2216,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
        { SYS_DESC(SYS_MDSCR_EL1), trap_debug_regs, reset_val, MDSCR_EL1, 0 },
        DBG_BCR_BVR_WCR_WVR_EL1(2),
        DBG_BCR_BVR_WCR_WVR_EL1(3),
-       { SYS_DESC(SYS_MDSELR_EL1), undef_access },
+       { SYS_DESC(SYS_MDSELR_EL1), trap_mdselr_el1 },
        DBG_BCR_BVR_WCR_WVR_EL1(4),
        DBG_BCR_BVR_WCR_WVR_EL1(5),
        DBG_BCR_BVR_WCR_WVR_EL1(6),

I am sure this is rather incomplete, but will really appreciate if you could
provide some details and pointers.

> 
> In short, implement the architecture as described in the pseudocode,
> and not a cheap shortcut.
> 
> Thanks,
> 
> 	M.
>