Re: [PATCH v2 12/16] KVM: arm64: Implement AT S1PIE support

[Joey Gouly <joey.gouly@arm.com>]

On Thu, Sep 05, 2024 at 03:57:13PM +0100, Marc Zyngier wrote:
> Hi Joey,
> 
> Thanks for having a look.
> 
> On Thu, 05 Sep 2024 14:58:20 +0100,
> Joey Gouly <joey.gouly@arm.com> wrote:
> > 
> > Hello Marc!
> > 
> > >  static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
> > >  				   struct s1_walk_info *wi,
> > >  				   struct s1_walk_result *wr,
> > >  				   struct s1_perms *s1p)
> > >  {
> > > -	compute_s1_direct_permissions(vcpu, wi, wr, s1p);
> > > +	if (!s1pie_enabled(vcpu, wi->regime))
> > > +		compute_s1_direct_permissions(vcpu, wi, wr, s1p);
> > > +	else
> > > +		compute_s1_indirect_permissions(vcpu, wi, wr, s1p);
> > > +
> > >  	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
> > 
> > Is this (and the previous patch to split this up) right?
> > 
> > Looking at this from the ARM ARM (ARM DDI 0487K.a):
> > 
> > 	R JHSVW If Indirect permissions are used, then hierarchical
> > 	permissions are disabled and TCR_ELx.HPDn are RES 1.
> 
> Odd. I was convinced that it was when S1POE is enabled that HPs were
> disabled. But you are absolutely right, and it is once more proven
> that I can't read. Oh well.

For POE there is:

	RBVXDG Hierarchical Permissions are disabled and the TCR_ELx.{HPD0, HPD1} bits are RES1 for stage 1 of a translation
	regime using VMSAv8-64 if one or more of POE and E0POE (for EL1&0, EL2&0) is enabled for that translation
	regime.

> 
> Not to worry, I've since found other issues with this series. I have
> forgotten the patch dealing with the fast path on another branch, and
> since decided that TCR2_EL2 needed extra care to cope with individual
> features being disabled.
> 
> The rework is still useful, as I'm looking at POE as well, but I need
> to hoist the HP stuff up a notch.
> 
> I'll repost things once I've sorted these things up.

I think the rest of this patch looked fine though.

> 
> Thanks again,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.