Re: [RFC/PATCH v0 03/12] gunyah: Basic support

[Srivatsa Vaddagiri <quic_svaddagi@quicinc.com>]

* Alex Benn?e <alex.bennee@linaro.org> [2023-11-29 16:56:38]:

> Srivatsa Vaddagiri <quic_svaddagi@quicinc.com> writes:
> 
> > Add a new accelerator, gunyah, with basic functionality of creating a
> > VM. Subsequent patches will add support for other functions required to
> > run a VM.
> >
> > Signed-off-by: Srivatsa Vaddagiri <quic_svaddagi@quicinc.com>
> 
> Hmm this failed to build:
> 
>   FAILED: libqemu-aarch64-softmmu.fa.p/accel_gunyah_gunyah-all.c.o 
>   cc -m64 -mcx16 -Ilibqemu-aarch64-softmmu.fa.p -I. -I../.. -Itarget/arm -I../../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -Wshadow=local -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/host/include/x86_64 -iquote /home/alex/lsrc/qemu.git/host/include/generic -iquote /home/alex/lsrc/qemu.git/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/accel_gunyah_gunyah-all.c.o -MF libqemu-aarch64-softmmu.fa.p/accel_gunyah_gunyah-all.c.o.d -o libqemu-aarch64-softmmu.fa.p/accel_gunyah_gunyah-all.c.o -c ../../accel/gunyah/gunyah-all.c
>   ../../accel/gunyah/gunyah-all.c:21:10: fatal error: linux-headers/linux/gunyah.h: No such file or directory
>      21 | #include "linux-headers/linux/gunyah.h"
>         |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   compilation terminated.
> 
> Easiest solution:
> 
>   add PATCH after 02 importing headers
> 
> I usually add a !MERGE to the summary just to remind myself that we'll
> need to properly update the headers before we merge. e.g: linux-headers: update to v6.6-with-gunyah (!MERGE)

I had run update-linux-headers.sh after Patch 02 before trying to compile rest of
the patch series. It was run against a Linux kernel tree having the Gunyah
patches. If it helps, I can include the actual header itself as a Patch (with
!MERGE noted), so that it saves some trouble.

> There are some other merge failures bellow so perhaps its time to do a
> re-base and send v1?

Yes working on v1. I have tested virtio-pci working I think, testing SMP now after
which will send out v1 (hopefully early next week).

> Whats the current state of the kernel merge?

The kernel patches are being reworked to avoid pinning of memory and to allow
on-demand allocation of VM memory. That should address the major feedback
received. We expect the next version of kernel patches to be published later
this month, and a potential merge in kernel.org hopefully before April (6.9)?!

It will also require some rework in Qemu support to use the new guest
memfd APIs for allocating VM memory (for confidential VMs). I will publish
support for it in v2 (could be a month away I think).

> <snip>
> > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> > index 9da3fe299b..0c95f79645 100644
> > --- a/scripts/meson-buildoptions.sh
> > +++ b/scripts/meson-buildoptions.sh
> > @@ -112,6 +112,7 @@ meson_options_help() {
> >    printf "%s\n" '  guest-agent-msi Build MSI package for the QEMU Guest Agent'
> >    printf "%s\n" '  hax             HAX acceleration support'
> >    printf "%s\n" '  hvf             HVF acceleration support'
> > +  printf "%s\n" '  gunyah          Gunyah acceleration support'
> 
> need rebasing due to hax drop
> 
> >    printf "%s\n" '  iconv           Font glyph conversion support'
> >    printf "%s\n" '  jack            JACK sound support'
> >    printf "%s\n" '  keyring         Linux keyring support'
> > @@ -312,6 +313,8 @@ _meson_option_parse() {
> >      --disable-guest-agent) printf "%s" -Dguest_agent=disabled ;;
> >      --enable-guest-agent-msi) printf "%s" -Dguest_agent_msi=enabled ;;
> >      --disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;;
> > +    --enable-gunyah) printf "%s" -Dgunyah=enabled ;;
> > +    --disable-gunyah) printf "%s" -Dgunyah=disabled ;;
> 
> ditto.
> 
> >      --enable-hax) printf "%s" -Dhax=enabled ;;
> >      --disable-hax) printf "%s" -Dhax=disabled ;;
> >      --enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;;
> > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> > index 96158093cc..a712f1a3b3 100644
> > --- a/target/arm/cpu64.c
> > +++ b/target/arm/cpu64.c
> > @@ -33,6 +33,7 @@
> >  #include "hw/qdev-properties.h"
> >  #include "internals.h"
> >  #include "cpregs.h"
> > +#include "sysemu/gunyah.h"
> 
> This should be moved up next to the other accelerator includes.

Sure thanks for this feedback. Will incorporate them in v1.

- vatsa