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

[Bui Quang Minh <minhquangbui99@gmail.com>]

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