Re: [PATCH v11 06/10] hvf: arm: Implement -cpu host

[Ard Biesheuvel]

On Wed, 22 Sept 2021 at 14:45, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 22 Sept 2021 at 12:41, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 16 Sept 2021 at 18:17, Peter Maydell <peter.maydell@linaro.org> 
> > wrote:
> > >
> > > On Thu, 16 Sept 2021 at 17:05, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > I'd argue that compliance with the architecture means that the
> > > > software should not clear RES1 bits
> > >
> > > Architecturally, RES1 means that "software
> > >  * Must not rely on the bit reading as 1.
> > >  * Must use an SBOP policy to write to the bit."
> > > (SBOP=="should be 1 or preserved", ie you can preserve the existing value,
> > > as in "read register, change some bits, write back", or you can write a 
> > > 1.)
> > >
> >
> > OVMF preserves the bit, and does not reason or care about its value.
> > So in this sense, it is compliant.
>
> Hmm. Alex, can you give more details about what fails here ?
>

It seems that EDK2 ends up setting EL0 r/o or r/w permissions on some
mappings, even though it never runs anything at EL0. So any execution
that gets initiated via the timer interrupt causing a EL1->EL1 IRQ
exception will run with PAN enabled and lose access to those mappings.

So it seems like a definite bug that is unrelated to reset state of
the registers and assumptions related to that.



> > > > but I don't think we can blame it
> > > > for not touching bits that were in in invalid state upon entry.
> > >
> > > SCTLR_EL1.SPAN == 0 is perfectly valid for a CPU that supports the
> > > PAN feature. It's just not the value OVMF wants, so OVMF should
> > > be setting it to what it does want. Also, as the first thing to
> > > run after reset (ie firmware) OVMF absolutely is responsible for
> > > dealing with system registers which have UNKNOWN values out of
> > > reset.
> > >
> >
> > Fair enough. But I'd still suggest fixing this at both ends.
>
> Yes, the version of this code that we committed sets SPAN to 1.
> (This argument is mostly about what the comment justifying that
> value should say :-))
>

OK, that makes sense. But I'd like to get EDK2 fixed as well, obviously.