diff mbox

[v11,0/7] Support x2APIC mode with TCG accelerator

Message ID 20231225164101.105958-1-minhquangbui99@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bui Quang Minh Dec. 25, 2023, 4:40 p.m. UTC
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


~ env QEMU=/home/minh/Desktop/oss/qemu/build/qemu-system-x86_64 ACCEL=tcg \
./run_tests.sh -v -g apic

TESTNAME=apic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/apic.flat -smp 2
-cpu qemu64,+x2apic,+tsc-deadline -machine kernel_irqchip=split FAIL
apic-split (54 tests, 8 unexpected failures, 1 skipped)
TESTNAME=ioapic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/ioapic.flat -smp
1 -cpu qemu64 -machine kernel_irqchip=split PASS ioapic-split (19 tests)
TESTNAME=x2apic TIMEOUT=30 ACCEL=tcg ./x86/run x86/apic.flat -smp 2 -cpu
qemu64,+x2apic,+tsc-deadline FAIL x2apic (54 tests, 8 unexpected failures,
1 skipped) TESTNAME=xapic TIMEOUT=60 ACCEL=tcg ./x86/run x86/apic.flat -smp
2 -cpu qemu64,-x2apic,+tsc-deadline -machine pit=off FAIL xapic (43 tests,
6 unexpected failures, 2 skipped)

  FAIL: apic_disable: *0xfee00030: 50014
  FAIL: apic_disable: *0xfee00080: f0
  FAIL: apic_disable: *0xfee00030: 50014
  FAIL: apic_disable: *0xfee00080: f0
  FAIL: apicbase: relocate apic

These errors are because we don't disable MMIO region when switching to
x2APIC and don't support relocate MMIO region yet. This is a problem
because, MMIO region is the same for all CPUs, in order to support these we
need to figure out how to allocate and manage different MMIO regions for
each CPUs. This can be an improvement in the future.

  FAIL: nmi-after-sti
  FAIL: multiple nmi

These errors are in the way we handle CPU_INTERRUPT_NMI in core TCG.

  FAIL: TMCCT should stay at zero

This error is related to APIC timer which should be addressed in separate
patch.

Version 11 changes,
- Patch 2:
  + Rebase to master and fix conflict with commit c04cfb4596 (hw/i386: fix
    short-circuit logic with non-optimizing builds)

Version 10 changes,
- Patch 2:
  + Fix null pointer dereference due to uninitialized local_apics when using
  machine none
- Patch 5, 7:
  + These patches are added to follow the bios-tables-test instructions to
  commit the new changed IVRS.ivrs binary file

Version 9 changes,
- Patch 1:
  + Create apic_msr_read/write which is a small wrapper around
  apic_register_read/write that have additional x2apic mode check
- Patch 2:
  + Remove raise_exception_ra which is is TCG specific. Instead, return -1
  and let the accelerator raise the appropriate exception
  + Refactor apic_get_delivery_bitmask a little bit to reduce line length
  + Move cpu_has_x2apic_feature and cpu_set_apic_feature from patch 3 to
  patch 2 so that patch 2 can be compiled without patch 3
- Patch 3:
  + set_base in APICCommonClass now returns an int to indicate error
  + Remove raise_exception_ra in apic_set base which is is TCG specific.
  Instead, return -1 and let the accelerator raise the appropriate
  exception

Version 8 changes,
- Patch 2, 4:
  + Rebase to master and resolve conflicts in these 2 patches

Version 7 changes,
- Patch 4:
  + If eim=on, keep checking if kvm x2APIC is enabled when kernel-irqchip
  is split

Version 6 changes,
- Patch 5:
  + Make all places use the amdvi_extended_feature_register to get extended
  feature register

Version 5 changes,
- Patch 3:
  + Rebase to master and fix conflict
- Patch 5:
  + Create a helper function to get amdvi extended feature register instead
  of storing it in AMDVIState

Version 4 changes,
- Patch 5:
  + Instead of replacing IVHD type 0x10 with type 0x11, export both types
  for backward compatibility with old guest operating system
  + Flip the xtsup feature check condition in amdvi_int_remap_ga for
  readability

Version 3 changes,
- Patch 2:
  + Allow APIC ID > 255 only when x2APIC feature is supported on CPU
  + Make physical destination mode IPI which has destination id 0xffffffff
  a broadcast to xAPIC CPUs
  + Make cluster address 0xf in cluster model of xAPIC logical destination
  mode a broadcast to all clusters
  + Create new extended_log_dest to store APIC_LDR information in x2APIC
  instead of extending log_dest for backward compatibility in vmstate

Version 2 changes,
- Add support for APIC ID larger than 255
- Adjust AMD iommu for x2APIC support
- Reorganize and split patch 1,2 into patch 1,2,3 in version 2

Thanks,
Quang Minh.

Bui Quang Minh (7):
  i386/tcg: implement x2APIC registers MSR access
  apic: add support for x2APIC mode
  apic, i386/tcg: add x2apic transitions
  intel_iommu: allow Extended Interrupt Mode when using userspace APIC
  test: bios-tables-test: prepare IVRS change in ACPI table
  amd_iommu: report x2APIC support to the operating system
  test: bios-tables-test: add IVRS changed binary

 hw/i386/acpi-build.c                 | 129 +++++---
 hw/i386/amd_iommu.c                  |  29 +-
 hw/i386/amd_iommu.h                  |  16 +-
 hw/i386/intel_iommu.c                |   6 +-
 hw/i386/kvm/apic.c                   |   3 +-
 hw/i386/x86.c                        |   6 +-
 hw/i386/xen/xen_apic.c               |   3 +-
 hw/intc/apic.c                       | 473 +++++++++++++++++++++------
 hw/intc/apic_common.c                |  22 +-
 hw/intc/trace-events                 |   4 +-
 include/hw/i386/apic.h               |   8 +-
 include/hw/i386/apic_internal.h      |   9 +-
 target/i386/cpu-sysemu.c             |  18 +-
 target/i386/cpu.c                    |   9 +-
 target/i386/cpu.h                    |   9 +
 target/i386/tcg/sysemu/misc_helper.c |  41 ++-
 target/i386/whpx/whpx-apic.c         |   3 +-
 tests/data/acpi/q35/IVRS.ivrs        | Bin 104 -> 176 bytes
 18 files changed, 600 insertions(+), 188 deletions(-)

Comments

Michael S. Tsirkin Dec. 26, 2023, 9:21 a.m. UTC | #1
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





> diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
> index 1734afb..f56fe1c 100644
> --- a/lib/x86/fwcfg.c
> +++ b/lib/x86/fwcfg.c
> @@ -27,6 +27,7 @@ static void read_cfg_override(void)
> 
>         if ((str = getenv("TEST_DEVICE")))
>                 no_test_device = !atol(str);
> +       no_test_device = true;
> 
>         if ((str = getenv("MEMLIMIT")))
>                 fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024;
> 
> ~ env QEMU=/home/minh/Desktop/oss/qemu/build/qemu-system-x86_64 ACCEL=tcg \
> ./run_tests.sh -v -g apic
> 
> TESTNAME=apic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/apic.flat -smp 2
> -cpu qemu64,+x2apic,+tsc-deadline -machine kernel_irqchip=split FAIL
> apic-split (54 tests, 8 unexpected failures, 1 skipped)
> TESTNAME=ioapic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/ioapic.flat -smp
> 1 -cpu qemu64 -machine kernel_irqchip=split PASS ioapic-split (19 tests)
> TESTNAME=x2apic TIMEOUT=30 ACCEL=tcg ./x86/run x86/apic.flat -smp 2 -cpu
> qemu64,+x2apic,+tsc-deadline FAIL x2apic (54 tests, 8 unexpected failures,
> 1 skipped) TESTNAME=xapic TIMEOUT=60 ACCEL=tcg ./x86/run x86/apic.flat -smp
> 2 -cpu qemu64,-x2apic,+tsc-deadline -machine pit=off FAIL xapic (43 tests,
> 6 unexpected failures, 2 skipped)
> 
>   FAIL: apic_disable: *0xfee00030: 50014
>   FAIL: apic_disable: *0xfee00080: f0
>   FAIL: apic_disable: *0xfee00030: 50014
>   FAIL: apic_disable: *0xfee00080: f0
>   FAIL: apicbase: relocate apic
> 
> These errors are because we don't disable MMIO region when switching to
> x2APIC and don't support relocate MMIO region yet. This is a problem
> because, MMIO region is the same for all CPUs, in order to support these we
> need to figure out how to allocate and manage different MMIO regions for
> each CPUs. This can be an improvement in the future.
> 
>   FAIL: nmi-after-sti
>   FAIL: multiple nmi
> 
> These errors are in the way we handle CPU_INTERRUPT_NMI in core TCG.
> 
>   FAIL: TMCCT should stay at zero
> 
> This error is related to APIC timer which should be addressed in separate
> patch.
> 
> Version 11 changes,
> - Patch 2:
>   + Rebase to master and fix conflict with commit c04cfb4596 (hw/i386: fix
>     short-circuit logic with non-optimizing builds)
> 
> Version 10 changes,
> - Patch 2:
>   + Fix null pointer dereference due to uninitialized local_apics when using
>   machine none
> - Patch 5, 7:
>   + These patches are added to follow the bios-tables-test instructions to
>   commit the new changed IVRS.ivrs binary file
> 
> Version 9 changes,
> - Patch 1:
>   + Create apic_msr_read/write which is a small wrapper around
>   apic_register_read/write that have additional x2apic mode check
> - Patch 2:
>   + Remove raise_exception_ra which is is TCG specific. Instead, return -1
>   and let the accelerator raise the appropriate exception
>   + Refactor apic_get_delivery_bitmask a little bit to reduce line length
>   + Move cpu_has_x2apic_feature and cpu_set_apic_feature from patch 3 to
>   patch 2 so that patch 2 can be compiled without patch 3
> - Patch 3:
>   + set_base in APICCommonClass now returns an int to indicate error
>   + Remove raise_exception_ra in apic_set base which is is TCG specific.
>   Instead, return -1 and let the accelerator raise the appropriate
>   exception
> 
> Version 8 changes,
> - Patch 2, 4:
>   + Rebase to master and resolve conflicts in these 2 patches
> 
> Version 7 changes,
> - Patch 4:
>   + If eim=on, keep checking if kvm x2APIC is enabled when kernel-irqchip
>   is split
> 
> Version 6 changes,
> - Patch 5:
>   + Make all places use the amdvi_extended_feature_register to get extended
>   feature register
> 
> Version 5 changes,
> - Patch 3:
>   + Rebase to master and fix conflict
> - Patch 5:
>   + Create a helper function to get amdvi extended feature register instead
>   of storing it in AMDVIState
> 
> Version 4 changes,
> - Patch 5:
>   + Instead of replacing IVHD type 0x10 with type 0x11, export both types
>   for backward compatibility with old guest operating system
>   + Flip the xtsup feature check condition in amdvi_int_remap_ga for
>   readability
> 
> Version 3 changes,
> - Patch 2:
>   + Allow APIC ID > 255 only when x2APIC feature is supported on CPU
>   + Make physical destination mode IPI which has destination id 0xffffffff
>   a broadcast to xAPIC CPUs
>   + Make cluster address 0xf in cluster model of xAPIC logical destination
>   mode a broadcast to all clusters
>   + Create new extended_log_dest to store APIC_LDR information in x2APIC
>   instead of extending log_dest for backward compatibility in vmstate
> 
> Version 2 changes,
> - Add support for APIC ID larger than 255
> - Adjust AMD iommu for x2APIC support
> - Reorganize and split patch 1,2 into patch 1,2,3 in version 2
> 
> Thanks,
> Quang Minh.
> 
> Bui Quang Minh (7):
>   i386/tcg: implement x2APIC registers MSR access
>   apic: add support for x2APIC mode
>   apic, i386/tcg: add x2apic transitions
>   intel_iommu: allow Extended Interrupt Mode when using userspace APIC
>   test: bios-tables-test: prepare IVRS change in ACPI table
>   amd_iommu: report x2APIC support to the operating system
>   test: bios-tables-test: add IVRS changed binary
> 
>  hw/i386/acpi-build.c                 | 129 +++++---
>  hw/i386/amd_iommu.c                  |  29 +-
>  hw/i386/amd_iommu.h                  |  16 +-
>  hw/i386/intel_iommu.c                |   6 +-
>  hw/i386/kvm/apic.c                   |   3 +-
>  hw/i386/x86.c                        |   6 +-
>  hw/i386/xen/xen_apic.c               |   3 +-
>  hw/intc/apic.c                       | 473 +++++++++++++++++++++------
>  hw/intc/apic_common.c                |  22 +-
>  hw/intc/trace-events                 |   4 +-
>  include/hw/i386/apic.h               |   8 +-
>  include/hw/i386/apic_internal.h      |   9 +-
>  target/i386/cpu-sysemu.c             |  18 +-
>  target/i386/cpu.c                    |   9 +-
>  target/i386/cpu.h                    |   9 +
>  target/i386/tcg/sysemu/misc_helper.c |  41 ++-
>  target/i386/whpx/whpx-apic.c         |   3 +-
>  tests/data/acpi/q35/IVRS.ivrs        | Bin 104 -> 176 bytes
>  18 files changed, 600 insertions(+), 188 deletions(-)
> 
> -- 
> 2.25.1
Bui Quang Minh Dec. 27, 2023, 11:03 a.m. UTC | #2
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.
Michael S. Tsirkin Dec. 27, 2023, 3:35 p.m. UTC | #3
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.
Bui Quang Minh Dec. 28, 2023, 3:44 p.m. UTC | #4
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

I saw the CI test "pages" failed too. On my CI, most of the time, it 
failed with

$ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU 
sourcecode"
00:24
htags: Negative exec line limit = -8099

It only succeeded once. I could not reproduce locally. Do you have any 
ideas what the problem is?

Thanks,
Quang Minh.
Bui Quang Minh Jan. 6, 2024, 4:33 p.m. UTC | #5
On 12/28/23 22:44, 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
> 
> I saw the CI test "pages" failed too. On my CI, most of the time, it 
> failed with
> 
> $ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU 
> sourcecode"
> 00:24
> htags: Negative exec line limit = -8099
> 
> It only succeeded once. I could not reproduce locally. Do you have any 
> ideas what the problem is?

I think I briefly understand why pages test fails. Internally, htags 
call global to parse the output of gtags. When executing command, it 
expects the size of argv and env to 20*1024 
(https://github.com/harai/gnu-global/blob/f86ba74d867385353815f8656c4a6cf4029c1f0b/libutil/xargs.c#L92-L105). 
The failed test case only happens when the last commit is patch 7 (test: 
bios-tables-test: add IVRS changed binary) which has a very long commit 
message (around 9000 bytes). By default, Gitlab appends some environment 
variables to the runner and one of them is CI_COMMIT_MESSAGE which 
contains the long commit message. So it exceeds the limit of 20*1024 
bytes and fails.

In my opinion, this failed test is not so critical and seems unrelated 
to the series so I skip this failed test. I will post the new version to 
fix the windows/amd64 build soon.

Thanks,
Quang Minh.
Michael S. Tsirkin Jan. 7, 2024, 1:38 p.m. UTC | #6
On Sat, Jan 06, 2024 at 11:33:29PM +0700, Bui Quang Minh wrote:
> On 12/28/23 22:44, 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
> > 
> > I saw the CI test "pages" failed too. On my CI, most of the time, it
> > failed with
> > 
> > $ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU
> > sourcecode"
> > 00:24
> > htags: Negative exec line limit = -8099
> > 
> > It only succeeded once. I could not reproduce locally. Do you have any
> > ideas what the problem is?
> 
> I think I briefly understand why pages test fails. Internally, htags call
> global to parse the output of gtags. When executing command, it expects the
> size of argv and env to 20*1024 (https://github.com/harai/gnu-global/blob/f86ba74d867385353815f8656c4a6cf4029c1f0b/libutil/xargs.c#L92-L105).
> The failed test case only happens when the last commit is patch 7 (test:
> bios-tables-test: add IVRS changed binary) which has a very long commit
> message (around 9000 bytes). By default, Gitlab appends some environment
> variables to the runner and one of them is CI_COMMIT_MESSAGE which contains
> the long commit message. So it exceeds the limit of 20*1024 bytes and fails.


Thanks for debugging this!
So I think we should clear CI_COMMIT_MESSAGE when invoking htags, right?


> In my opinion, this failed test is not so critical and seems unrelated to
> the series so I skip this failed test.

Yes ok to ignore. But even better if we also add a workaround.

> I will post the new version to fix
> the windows/amd64 build soon.
> 
> Thanks,
> Quang Minh.
diff mbox

Patch

diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
index 1734afb..f56fe1c 100644
--- a/lib/x86/fwcfg.c
+++ b/lib/x86/fwcfg.c
@@ -27,6 +27,7 @@  static void read_cfg_override(void)

        if ((str = getenv("TEST_DEVICE")))
                no_test_device = !atol(str);
+       no_test_device = true;

        if ((str = getenv("MEMLIMIT")))
                fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024;