Re: [kvm-unit-tests PATCH] lib/devicetree: Support 64 bit addresses for the initrd

Alexandru Elisei
2022-02-15T16:14:55Z

Hi Drew,

On Tue, Feb 15, 2022 at 04:50:37PM +0100, Andrew Jones wrote:
> On Tue, Feb 15, 2022 at 02:16:32PM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> >
> > On Tue, Feb 15, 2022 at 01:53:00PM +0100, Andrew Jones wrote:
> > > On Tue, Feb 15, 2022 at 10:07:16AM +0000, Alexandru Elisei wrote:
> > > >
> > > > I've started working on the next iteration of the kvmtool test
> > > > runner support series, I'll do my best to make sure kvmtool wll be able to run
> > > > the tests when kvm-unit-tests has been configured with --arch=arm.
> > > >
> > >
> > > Excellent!
> > >
> > > BTW, I went ahead an pushed a patch to misc/queue to improve the initrd
> > > address stuff
> > >
> > > https://gitlab.com/rhdrjones/kvm-unit-tests/-/commit/6f8f74ed2d9953830a3c74669f25439d9ad68dec
> > >
> > > It may be necessary for you if kvmtool shares its fdt creation between
> > > aarch64 and aarch32 guests, emitting 8 byte initrd addresses for both,
> > > even though the aarch32 guest puts the fdt below 4G.
> >
> > While trying your patch (it works, but with the caveat below), I remembered that
> > kvmtool is not able to run kvm-unit-tests for arm. That's because the code is
> > not relocatable (like it is for arm64) and the text address is hardcoded in the
> > makefile.
> >
> > In past, to run the arm tests with kvmtool, I was doing this change:
> >
> > diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> > index 3a4cc6b26234..6c580b067413 100644
> > --- a/arm/Makefile.arm
> > +++ b/arm/Makefile.arm
> > @@ -14,7 +14,7 @@ CFLAGS += $(machine)
> > CFLAGS += -mcpu=$(PROCESSOR)
> > CFLAGS += -mno-unaligned-access
> >
> > -arch_LDFLAGS = -Ttext=40010000
> > +arch_LDFLAGS = -Ttext=80008000
> >
> > define arch_elf_check =
> > endef
> >
> > Any suggestions how to fix that? One way would be to change the LDFLAGS based on
> > $TARGET. Another way would be to make the arm tests relocatable, I tried to do
> > that in the past but I couldn't make any progress.
>
> Ideally we'd eventually make it relocatable, but it's not worth moving
> mountains, so I'm OK with the LDFLAGS approach.
>
> >
> > Separate from that, I also tried to run the 32 bit arm tests with run_tests.sh.
> > The runner uses qemu-system-arm (because $ARCH_NAME is arm in
> > scripts/arch-run.bash::search_qemu_binary()), but uses kvm as the accelerator
> > (because /dev/kvm exists in scrips/arch-run.bash::kvm_available()). This fails
> > with the error:
> >
> > qemu-system-arm: -accel kvm: invalid accelerator kvm
> >
> > I don't think that's supposed to happen, as kvm_available() specifically returns
> > true if $HOST = aarch64 and $ARCH = arm. Any suggestions?
> >
>
> Ah, that's a kvm-unit-tests bug. Typically qemu-system-$ARCH_NAME is
> correct and, with TCG, qemu-system-arm runs the arm built unit tests fine.
> But, when KVM is enabled, the QEMU used should be qemu-system-$HOST.
>
> I don't think we want to change search_qemu_binary(), though, as that
> would require ACCEL being set first and may not apply to all
> architectures. Probably the best thing to do is fix this up in arm/run

Definitely, this should go in arm/run.

>
> diff --git a/arm/run b/arm/run
> index 2153bd320751..ff18def43403 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -13,6 +13,10 @@ processor="$PROCESSOR"
> ACCEL=$(get_qemu_accelerator) ||
> exit $?
>
> +if [ "$ACCEL" = "kvm" ] && [ -z "$QEMU" ]; then
> + QEMU=qemu-system-$HOST
> +fi

Looking at get_qemu_accelerator, I think it returns true when /dev/kvm exists,
which means that get_qemu_accelerator can return "kvm" when trying to run the
arm64 tests on an arm machine with KVM. But in this case, it's impossible to run
the tests using KVM.

And I also think there should be some checking that aarch64=off works, as there
are arm64 CPUs which don't support AArch32 (like Cortex-A510).

Thanks,
Alex

> +
> qemu=$(search_qemu_binary) ||
> exit $?
>
>
> Thanks,
> drew
>