Re: [PATCH v11 0/7] Support x2APIC mode with TCG accelerator

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Dec 27, 2023 at 06:03:46PM +0700, Bui Quang Minh wrote:
> On 12/26/23 16:21, Michael S. Tsirkin wrote:
> > On Mon, Dec 25, 2023 at 11:40:54PM +0700, Bui Quang Minh wrote:
> > > Hi everyone,
> > > 
> > > This series implements x2APIC mode in userspace local APIC and the
> > > RDMSR/WRMSR helper to access x2APIC registers in x2APIC mode. Intel iommu
> > > and AMD iommu are adjusted to support x2APIC interrupt remapping. With this
> > > series, we can now boot Linux kernel into x2APIC mode with TCG accelerator
> > > using either Intel or AMD iommu.
> > > 
> > > Testing to boot my own built Linux 6.3.0-rc2, the kernel successfully boot
> > > with enabled x2APIC and can enumerate CPU with APIC ID 257
> > > 
> > > Using Intel IOMMU
> > > 
> > > qemu/build/qemu-system-x86_64 \
> > >    -smp 2,maxcpus=260 \
> > >    -cpu qemu64,x2apic=on \
> > >    -machine q35 \
> > >    -device intel-iommu,intremap=on,eim=on \
> > >    -device qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0 \
> > >    -m 2G \
> > >    -kernel $KERNEL_DIR \
> > >    -append "nokaslr console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
> > >    -drive file=$IMAGE_DIR,format=raw \
> > >    -nographic \
> > >    -s
> > > 
> > > Using AMD IOMMU
> > > 
> > > qemu/build/qemu-system-x86_64 \
> > >    -smp 2,maxcpus=260 \
> > >    -cpu qemu64,x2apic=on \
> > >    -machine q35 \
> > >    -device amd-iommu,intremap=on,xtsup=on \
> > >    -device qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0 \
> > >    -m 2G \
> > >    -kernel $KERNEL_DIR \
> > >    -append "nokaslr console=ttyS0 root=/dev/sda earlyprintk=serial net.ifnames=0" \
> > >    -drive file=$IMAGE_DIR,format=raw \
> > >    -nographic \
> > >    -s
> > > 
> > > Testing the emulated userspace APIC with kvm-unit-tests, disable test
> > > device with this patch
> > 
> > Seems to break build for windows/amd64
> > https://gitlab.com/mstredhat/qemu/-/pipelines/1118886361/failures
> 
> The failure is because when CONFIG_AMD_IOMMU=n, amd_iommu.c is not built so
> the linker cannot find the definition of amdvi_extended_feature_register
> (amdvi_extended_feature_register is used in acpi-build.c). I create a stub
> to solve this problem and it passes all CI tests. I will squash the
> following changes into patch 6. What do you think about this?
> 
> diff --git a/hw/i386/amd_iommu_stub.c b/hw/i386/amd_iommu_stub.c

I'd probably call it amd_iommu-stub.c even if the mix of _ and - looks
weird, but this is how all other stubs are called.
Document this in the commit log too please.

> new file mode 100644
> index 0000000000..d62a3732e6
> --- /dev/null
> +++ b/hw/i386/amd_iommu_stub.c
> @@ -0,0 +1,26 @@
> +/*
> + * Stubs for AMD IOMMU emulation
> + *
> + * Copyright (C) 2023 Bui Quang Minh <minhquangbui99@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "amd_iommu.h"
> +
> +uint64_t amdvi_extended_feature_register(AMDVIState *s)
> +{
> +    return AMDVI_DEFAULT_EXT_FEATURES;
> +}
> diff --git a/hw/i386/meson.build b/hw/i386/meson.build
> index 369c6bf823..d38637b046 100644
> --- a/hw/i386/meson.build
> +++ b/hw/i386/meson.build
> @@ -9,7 +9,8 @@ i386_ss.add(files(
> 
>  i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
>                                        if_false: files('x86-iommu-stub.c'))
> -i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'))
> +i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'),
> +                                      if_false: files('amd_iommu_stub.c'))
>  i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
>  i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('microvm.c',
> 'acpi-microvm.c', 'microvm-dt.c'))
>  i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
> 
> 
> Thanks,
> Quang Minh.