On Thu, 16 Sept 2021 at 17:56, Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Thu, 16 Sept 2021 at 16:30, Alexander Graf <ag...@csgraf.de> wrote: > > > > > > On 16.09.21 14:24, Peter Maydell wrote: > > > On Wed, 15 Sept 2021 at 19:10, Alexander Graf <ag...@csgraf.de> wrote: > > >> Now that we have working system register sync, we push more target CPU > > >> properties into the virtual machine. That might be useful in some > > >> situations, but is not the typical case that users want. > > >> > > >> So let's add a -cpu host option that allows them to explicitly pass all > > >> CPU capabilities of their host CPU into the guest. > > >> > > >> Signed-off-by: Alexander Graf <ag...@csgraf.de> > > >> Acked-by: Roman Bolshakov <r.bolsha...@yadro.com> > > >> Reviewed-by: Sergio Lopez <s...@redhat.com> > > >> > > >> + /* > > >> + * A scratch vCPU returns SCTLR 0, so let's fill our default with > > >> the M1 > > >> + * boot SCTLR from https://github.com/AsahiLinux/m1n1/issues/97 > > Side note: SCTLR_EL1 is a 64-bit register, do you have anything that > prints the full 64-bits to confirm that [63:32] are indeed all 0? > > > >> + */ > > >> + ahcf->reset_sctlr = 0x30100180; > > >> + /* OVMF chokes on boot if SPAN is not set, so default it to on */ > > >> + ahcf->reset_sctlr |= 0x00800000; > > > Isn't that just an OVMF bug ? If you want this then you need to > > > convince me why this isn't just a workaround for a buggy guest. > > > > > > I couldn't find anything in the ARMv8 spec that explicitly says "If you > > support PAN, SCTLR.SPAN should be 1 by default". It is RES1 for CPUs > > that do not implement PAN. Beware that for SPAN, "1" means disabled and > > "0" means enabled. > > It's UNKNOWN on reset. So unless OVMF is relying on whatever > is launching it to set SCTLR correctly (ie there is some part of > the "firmware-to-OVMF" contract it is relying on) then it seems to > me that it's OVMF's job to initialize it to what it needs. (Lots of > SCTLR is like that.) > > Linux does this here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/head.S?h=v5.15-rc1#n485 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/sysreg.h?h=v5.15-rc1#n695 > because the INIT_SCTLR_EL1_MMU_OFF constant includes forcing > all "this kernel expects these to be RES0/RES1 because that's all > the architectural features we know about at this time" bits to > their RESn values. > > But we can probably construct an argument for why having it set > makes sense, yes. >
I'd argue that compliance with the architecture means that the software should not clear RES1 bits, but I don't think we can blame it for not touching bits that were in in invalid state upon entry.
