On Fri, 1 Mar 2024 10:41:09 -1000 Richard Henderson <richard.hender...@linaro.org> wrote:
> If translation is disabled, the default memory type is Device, which > requires alignment checking. This is more optimally done early via > the MemOp given to the TCG memory operation. > > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > Reported-by: Idan Horowitz <idan.horow...@gmail.com> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204 > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> Hi Richard. I noticed some tests I was running stopped booting with master. (it's a fun and complex stack of QEMU + kvm on QEMU for vCPU Hotplug kernel work, but this is the host booting) EDK2 build from upstream as of somepoint last week. Bisects to this patch. qemu-system-aarch64 -M virt,gic-version=3,virtualization=true -m 4g,maxmem=8G,slots=8 -cpu cortex-a76 -smp cpus=4,threads=2,clusters=2,sockets=1 \ -kernel Image \ -drive if=none,file=full.qcow2,format=qcow2,id=hd \ -device ioh3420,id=root_port1 -device virtio-blk-pci,drive=hd \ -netdev user,id=mynet,hostfwd=tcp::5555-:22 -device virtio-net-pci,netdev=mynet,id=bob \ -nographic -no-reboot -append 'earlycon root=/dev/vda2 fsck.mode=skip tp_printk' \ -monitor telnet:127.0.0.1:1235,server,nowait -bios QEMU_EFI.fd \ -object memory-backend-ram,size=4G,id=mem0 \ -numa node,nodeid=0,cpus=0-3,memdev=mem0 Symptoms: Nothing on console from edk2 which is built in debug mode so is normally very noisy. No sign of anything much happening at all :( Jonathan > --- > target/arm/tcg/hflags.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c > index 8e5d35d922..5da1b0fc1d 100644 > --- a/target/arm/tcg/hflags.c > +++ b/target/arm/tcg/hflags.c > @@ -26,6 +26,35 @@ static inline bool fgt_svc(CPUARMState *env, int el) > FIELD_EX64(env->cp15.fgt_exec[FGTREG_HFGITR], HFGITR_EL2, SVC_EL1); > } > > +/* Return true if memory alignment should be enforced. */ > +static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t > sctlr) > +{ > +#ifdef CONFIG_USER_ONLY > + return false; > +#else > + /* Check the alignment enable bit. */ > + if (sctlr & SCTLR_A) { > + return true; > + } > + > + /* > + * If translation is disabled, then the default memory type is > + * Device(-nGnRnE) instead of Normal, which requires that alignment > + * be enforced. Since this affects all ram, it is most efficient > + * to handle this during translation. > + */ > + if (sctlr & SCTLR_M) { > + /* Translation enabled: memory type in PTE via MAIR_ELx. */ > + return false; > + } > + if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) { > + /* Stage 2 translation enabled: memory type in PTE. */ > + return false; > + } > + return true; > +#endif > +} > + > static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el, > ARMMMUIdx mmu_idx, > CPUARMTBFlags flags) > @@ -121,8 +150,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, > int fp_el, > { > CPUARMTBFlags flags = {}; > int el = arm_current_el(env); > + uint64_t sctlr = arm_sctlr(env, el); > > - if (arm_sctlr(env, el) & SCTLR_A) { > + if (aprofile_require_alignment(env, el, sctlr)) { > DP_TBFLAG_ANY(flags, ALIGN_MEM, 1); > } > > @@ -223,7 +253,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, > int el, int fp_el, > > sctlr = regime_sctlr(env, stage1); > > - if (sctlr & SCTLR_A) { > + if (aprofile_require_alignment(env, el, sctlr)) { > DP_TBFLAG_ANY(flags, ALIGN_MEM, 1); > } >