Re: [PATCH v9 14/22] KVM: arm64: pkvm: Make the ERR/ERX*_EL1 registers RAZ/WI

From: Marc Zyngier <maz@kernel.org>
To: Andrew Jones <drjones@redhat.com>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, will@kernel.org,
	james.morse@arm.com, alexandru.elisei@arm.com,
	suzuki.poulose@arm.com, mark.rutland@arm.com,
	pbonzini@redhat.com, oupton@google.com, qperret@google.com,
	kernel-team@android.com, tabba@google.com
Subject: Re: [PATCH v9 14/22] KVM: arm64: pkvm: Make the ERR/ERX*_EL1 registers RAZ/WI
Date: Thu, 14 Oct 2021 17:09:13 +0100	[thread overview]
Message-ID: <878ryvpp06.wl-maz@kernel.org> (raw)
In-Reply-To: <20211014093220.ylnqb3ohuh5qayps@gator.home>

On Thu, 14 Oct 2021 10:32:20 +0100,
Andrew Jones <drjones@redhat.com> wrote:
> 
> On Wed, Oct 13, 2021 at 01:03:38PM +0100, Marc Zyngier wrote:
> > The ERR*/ERX* registers should be handled as RAZ/WI, and there
> > should be no need to involve EL1 for that.
> > 
> > Add a helper that handles such registers, and repaint the sysreg
> > table to declare these registers as RAZ/WI.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/hyp/nvhe/sys_regs.c | 33 ++++++++++++++++++++----------
> >  1 file changed, 22 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> > index f125d6a52880..042a1c0be7e0 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> > @@ -248,6 +248,16 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> >  	return pvm_read_id_reg(vcpu, reg_to_encoding(r));
> >  }
> >  
> > +/* Handler to RAZ/WI sysregs */
> > +static bool pvm_access_raz_wi(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> > +			      const struct sys_reg_desc *r)
> > +{
> > +	if (!p->is_write)
> > +		p->regval = 0;
> > +
> > +	return true;
> > +}
> > +
> >  /*
> >   * Accessor for AArch32 feature id registers.
> >   *
> > @@ -270,9 +280,7 @@ static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,
> >  	BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1),
> >  		     PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_ELx_64BIT_ONLY);
> >  
> > -	/* Use 0 for architecturally "unknown" values. */
> > -	p->regval = 0;
> > -	return true;
> > +	return pvm_access_raz_wi(vcpu, p, r);
> >  }
> >  
> >  /*
> > @@ -301,6 +309,9 @@ static bool pvm_access_id_aarch64(struct kvm_vcpu *vcpu,
> >  /* Mark the specified system register as an AArch64 feature id register. */
> >  #define AARCH64(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch64 }
> >  
> > +/* Mark the specified system register as Read-As-Zero/Write-Ignored */
> > +#define RAZ_WI(REG) { SYS_DESC(REG), .access = pvm_access_raz_wi }
> > +
> >  /* Mark the specified system register as not being handled in hyp. */
> >  #define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }
> >  
> > @@ -388,14 +399,14 @@ static const struct sys_reg_desc pvm_sys_reg_descs[] = {
> >  	HOST_HANDLED(SYS_AFSR1_EL1),
> >  	HOST_HANDLED(SYS_ESR_EL1),
> >  
> > -	HOST_HANDLED(SYS_ERRIDR_EL1),
> > -	HOST_HANDLED(SYS_ERRSELR_EL1),
> > -	HOST_HANDLED(SYS_ERXFR_EL1),
> > -	HOST_HANDLED(SYS_ERXCTLR_EL1),
> > -	HOST_HANDLED(SYS_ERXSTATUS_EL1),
> > -	HOST_HANDLED(SYS_ERXADDR_EL1),
> > -	HOST_HANDLED(SYS_ERXMISC0_EL1),
> > -	HOST_HANDLED(SYS_ERXMISC1_EL1),
> > +	RAZ_WI(SYS_ERRIDR_EL1),
> 
> This is a read-only register. Is write-ignore correct? I'd expect we to
> inject an exception.

The HW will do it for us, as the MSR instruction doesn't exist for
this register.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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