On Wed, 22 Sept 2021 at 14:45, Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Wed, 22 Sept 2021 at 12:41, Ard Biesheuvel <a...@kernel.org> wrote: > > > > On Thu, 16 Sept 2021 at 18:17, Peter Maydell <peter.mayd...@linaro.org> > > wrote: > > > > > > On Thu, 16 Sept 2021 at 17:05, Ard Biesheuvel <a...@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.
