Re: [PATCH v11 05/12] KVM: arm64: Bump up the default KVM sanitised debug version to v8p8

[Oliver Upton <oliver.upton@linux.dev>]

On Wed, Oct 04, 2023 at 09:57:44AM +0100, Marc Zyngier wrote:
> On Wed, 04 Oct 2023 00:04:01 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > Since ID_AA64DFR0_EL1 and ID_DFR0_EL1 are now writable from userspace,
> > it is safe to bump up the default KVM sanitised debug version to v8p8.
> > 
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> 
> The SoB sequence looks odd. Either you're the author, and Jing's SoB
> shouldn't be there without a Co-DB tag, or you've lost Jing's
> attribution (which sometimes happens when rebasing and squashing
> patches together).

This is an artifact of me applying Jing's version of the series and
hacking on top of it.

> > ---
> >  arch/arm64/kvm/sys_regs.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 8fbfe61fe7bc..b342c96e08f4 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1496,8 +1496,7 @@ static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> >  {
> >  	u64 val = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> >  
> > -	/* Limit debug to ARMv8.0 */
> > -	val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64DFR0_EL1, DebugVer, IMP);
> > +	val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64DFR0_EL1, DebugVer, V8P8);
> >  
> >  	/*
> >  	 * Only initialize the PMU version if the vCPU was configured with one.
> > @@ -1557,6 +1556,8 @@ static u64 read_sanitised_id_dfr0_el1(struct kvm_vcpu *vcpu,
> >  	if (kvm_vcpu_has_pmu(vcpu))
> >  		val |= SYS_FIELD_PREP(ID_DFR0_EL1, PerfMon, perfmon);
> >  
> > +	val = ID_REG_LIMIT_FIELD_ENUM(val, ID_DFR0_EL1, CopDbg, Debugv8p8);
> > +
> 
> For consistency, you should also repaint DBGDIDR, which has a
> hardcoded '6' (ARMv8) as the supported debug version.

Ah, good point. I'll extract the value from the ID register much like we
do for other fields in DBGDIDR:

commit b92565ca433f611ea0901a6098d72f91be84cdb0
Author: Oliver Upton <oliver.upton@linux.dev>
Date:   Wed Oct 4 17:03:17 2023 +0000

    KVM: arm64: Advertise selected DebugVer in DBGDIDR.Version
    
    Much like we do for other fields, extract the Debug architecture version
    from the ID register to populate the corresponding field in DBGDIDR.
    Rewrite the existing sysreg field extractors to use SYS_FIELD_GET() for
    consistency.
    
    Suggested-by: Marc Zyngier <maz@kernel.org>
    Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index cf1b2def53db..57c8190d5438 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2510,12 +2510,13 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
 	} else {
 		u64 dfr = IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
 		u64 pfr = IDREG(vcpu->kvm, SYS_ID_AA64PFR0_EL1);
-		u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, ID_AA64PFR0_EL1_EL3_SHIFT);
+		u32 el3 = !!SYS_FIELD_GET(ID_AA64PFR0_EL1, EL3, pfr);
 
-		p->regval = ((((dfr >> ID_AA64DFR0_EL1_WRPs_SHIFT) & 0xf) << 28) |
-			     (((dfr >> ID_AA64DFR0_EL1_BRPs_SHIFT) & 0xf) << 24) |
-			     (((dfr >> ID_AA64DFR0_EL1_CTX_CMPs_SHIFT) & 0xf) << 20)
-			     | (6 << 16) | (1 << 15) | (el3 << 14) | (el3 << 12));
+		p->regval = ((SYS_FIELD_GET(ID_AA64DFR0_EL1, WRPs, dfr) << 28) |
+			     (SYS_FIELD_GET(ID_AA64DFR0_EL1, BRPs, dfr) << 24) |
+			     (SYS_FIELD_GET(ID_AA64DFR0_EL1, CTX_CMPs, dfr) << 20) |
+			     (SYS_FIELD_GET(ID_AA64DFR0_EL1, DebugVer, dfr) << 16) |
+			     (1 << 15) | (el3 << 14) | (el3 << 12));
 		return true;
 	}
 }

-- 
Thanks,
Oliver