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@xxxxxxx> 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.
