[PATCH 14/25] KVM: arm64: Register AArch64 system register entries with the sysreg xarray

[Joey Gouly]

On Wed, Jan 24, 2024 at 04:37:23PM +0000, Marc Zyngier wrote:
> On Wed, 24 Jan 2024 16:34:25 +0000,
> Joey Gouly <joey.gouly at arm.com> wrote:
> > 
> > On Mon, Jan 22, 2024 at 08:18:41PM +0000, Marc Zyngier wrote:
> > > In order to reduce the number of lookups that we have to perform
> > > when handling a sysreg, register each AArch64 sysreg descriptor
> > > with the global xarray. The index of the descriptor is stored
> > > as a 10 bit field in the data word.
> > > 
> > > Subsequent patches will retrieve and use the stored index.
> > > 
> > > Signed-off-by: Marc Zyngier <maz at kernel.org>
> > > ---
> > >  arch/arm64/include/asm/kvm_host.h |  3 +++
> > >  arch/arm64/kvm/emulate-nested.c   | 39 +++++++++++++++++++++++++++++--
> > >  arch/arm64/kvm/sys_regs.c         | 11 ++++++++-
> > >  3 files changed, 50 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index fe35c59214ad..e7a6219f2929 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -1083,6 +1083,9 @@ int kvm_handle_cp10_id(struct kvm_vcpu *vcpu);
> > >  void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
> > >  
> > >  int __init kvm_sys_reg_table_init(void);
> > > +struct sys_reg_desc;
> > > +int __init populate_sysreg_config(const struct sys_reg_desc *sr,
> > > +				  unsigned int idx);
> > >  int __init populate_nv_trap_config(void);
> > >  
> > >  bool lock_all_vcpus(struct kvm *kvm);
> > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > > index 59622636b723..342d43b66fda 100644
> > > --- a/arch/arm64/kvm/emulate-nested.c
> > > +++ b/arch/arm64/kvm/emulate-nested.c
> > > @@ -427,12 +427,14 @@ static const complex_condition_check ccc[] = {
> > >   * [19:14]	bit number in the FGT register (6 bits)
> > >   * [20]		trap polarity (1 bit)
> > >   * [25:21]	FG filter (5 bits)
> > > - * [62:26]	Unused (37 bits)
> > > + * [35:26]	Main SysReg table index (10 bits)
> > > + * [62:36]	Unused (27 bits)
> > >   * [63]		RES0 - Must be zero, as lost on insertion in the xarray
> > >   */
> > >  #define TC_CGT_BITS	10
> > >  #define TC_FGT_BITS	4
> > >  #define TC_FGF_BITS	5
> > > +#define TC_MSR_BITS	10
> > >  
> > >  union trap_config {
> > >  	u64	val;
> > > @@ -442,7 +444,8 @@ union trap_config {
> > >  		unsigned long	bit:6;		 /* Bit number */
> > >  		unsigned long	pol:1;		 /* Polarity */
> > >  		unsigned long	fgf:TC_FGF_BITS; /* Fine Grained Filter */
> > > -		unsigned long	unused:37;	 /* Unused, should be zero */
> > > +		unsigned long	msr:TC_MSR_BITS; /* Main SysReg index */
> > > +		unsigned long	unused:27;	 /* Unused, should be zero */
> > >  		unsigned long	mbz:1;		 /* Must Be Zero */
> > >  	};
> > >  };
> > > @@ -1862,6 +1865,38 @@ int __init populate_nv_trap_config(void)
> > >  	return ret;
> > >  }
> > >  
> > > +int __init populate_sysreg_config(const struct sys_reg_desc *sr,
> > > +				  unsigned int idx)
> > > +{
> > > +	union trap_config tc;
> > > +	u32 encoding;
> > > +	void *ret;
> > > +
> > > +	/*
> > > +	 * 0 is a valid value for the index, but not for the storage.
> > > +	 * We'll store (idx+1), so check against an offset'd limit.
> > > +	 */
> > > +	if (idx >= (BIT(TC_MSR_BITS) - 1)) {
> > > +		kvm_err("sysreg %s (%d) out of range\n", sr->name, idx);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	encoding = sys_reg(sr->Op0, sr->Op1, sr->CRn, sr->CRm, sr->Op2);
> > > +	tc = get_trap_config(encoding);
> > > +
> > > +	if (tc.msr) {
> > > +		kvm_err("sysreg %s (%d) duplicate entry (%d)\n",
> > > +			sr->name, idx - 1, tc.msr);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	tc.msr = idx + 1;
> > > +	ret = xa_store(&sr_forward_xa, encoding,
> > > +		       xa_mk_value(tc.val), GFP_KERNEL);
> > > +
> > > +	return xa_err(ret);
> > > +}
> > > +
> > >  static enum trap_behaviour get_behaviour(struct kvm_vcpu *vcpu,
> > >  					 const struct trap_bits *tb)
> > >  {
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 77cd818c23b0..65319193e443 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -3974,6 +3974,7 @@ int __init kvm_sys_reg_table_init(void)
> > >  	struct sys_reg_params params;
> > >  	bool valid = true;
> > >  	unsigned int i;
> > > +	int ret = 0;
> > >  
> > >  	/* Make sure tables are unique and in order. */
> > >  	valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false);
> > > @@ -3997,5 +3998,13 @@ int __init kvm_sys_reg_table_init(void)
> > >  	if (!first_idreg)
> > >  		return -EINVAL;
> > >  
> > > -	return populate_nv_trap_config();
> > > +	ret = populate_nv_trap_config();
> > > +
> > > +	for (i = 0; !ret && i < ARRAY_SIZE(sys_reg_descs); i++)
> > > +		ret = populate_sysreg_config(sys_reg_descs + i, i);
> > > +
> > > +	for (i = 0; !ret && i < ARRAY_SIZE(sys_insn_descs); i++)
> > > +		ret = populate_sysreg_config(sys_insn_descs + i, i);
> > > +
> > > +	return ret;
> > >  }
> > 
> > The choice of `msr` was a tiny bit confusing due to the conflict with the asm
> > instruction `msr`, but not enough to warrant renaming.
> 
> No, that's actually a very good point. How about SRI (Sys Reg Index)?

LGTM

Thanks,
Joey