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. -- PMM
