Hi Alex,
On Fri, 16 Aug 2024 10:22:43 +0100,
Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>
> Hi Marc,
>
> On Thu, Aug 15, 2024 at 07:28:41PM +0100, Marc Zyngier wrote:
> >
> > Hi Alex,
> >
> > On Thu, 15 Aug 2024 17:44:02 +0100,
> > Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
[...]
> > > > +static bool par_check_s1_perm_fault(u64 par)
> > > > +{
> > > > + u8 fst = FIELD_GET(SYS_PAR_EL1_FST, par);
> > > > +
> > > > + return ((fst & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_PERM &&
> > > > + !(par & SYS_PAR_EL1_S));
> > >
> > > ESR_ELx_FSC_PERM = 0x0c is a permission fault, level 0, which Arm ARM says can
> > > only happen when FEAT_LPA2. I think the code should check that the value for
> > > PAR_EL1.FST is in the interval (ESR_ELx_FSC_PERM_L(0), ESR_ELx_FSC_PERM_L(3)].
> >
> > I honestly don't want to second-guess the HW. If it reports something
> > that is the wrong level, why should we trust the FSC at all?
>
> Sorry, I should have been clearer.
>
> It's not about the hardware reporting a fault on level 0 of the translation
> tables, it's about the function returning false if the hardware reports a
> permission fault on levels 1, 2 or 3 of the translation tables.
>
> For example, on a permssion fault on level 3, PAR_EL1. FST = 0b001111 = 0x0F,
> which means that the condition:
>
> (fst & ESR_ELx_FSC_TYPE) == ESR_ELx_FSC_PERM (which is 0x0C) is false and KVM
> will fall back to the software walker.
>
> Does that make sense to you?
I'm afraid I still don't get it.
