On 11/13/24 15:49, Phil Dennis-Jordan wrote:
It appears that existing call sites for the kvm_enable_x2apic()
function rely on the compiler eliding the calls during optimisation
when building with KVM disabled, or on platforms other than Linux,
where that function is declared but not defined.
This fragile reliance recently broke down when commit b12cb38 added
a new call site which apparently failed to be optimised away when
building QEMU on macOS with clang, resulting in a link error.
This change moves the function declaration into the existing
#if CONFIG_KVM
block in the same header file, while the corresponding
#else
block now #defines the symbol as 0, same as for various other
KVM-specific query functions.
Signed-off-by: Phil Dennis-Jordan <phil@xxxxxxxxxxxxx>
Nevermind, this actually rung a bell and seems to be the same as
this commit from last year:
commit c04cfb4596ad5032a9869a8f77fe9114ca8af9e0
Author: Daniel Hoffman <dhoff749@xxxxxxxxx>
Date: Sun Nov 19 12:31:16 2023 -0800
hw/i386: fix short-circuit logic with non-optimizing builds
`kvm_enabled()` is compiled down to `0` and short-circuit logic is
used to remove references to undefined symbols at the compile stage.
Some build configurations with some compilers don't attempt to
simplify this logic down in some cases (the pattern appears to be
that the literal false must be the first term) and this was causing
some builds to emit references to undefined symbols.
An example of such a configuration is clang 16.0.6 with the following
configure: ./configure --enable-debug --without-default-features
--target-list=x86_64-softmmu --enable-tcg-interpreter
Signed-off-by: Daniel Hoffman <dhoff749@xxxxxxxxx>
Message-Id: <20231119203116.3027230-1-dhoff749@xxxxxxxxx>
Reviewed-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
So, this should work:
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 13af7211e11..af0f4da1f69 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1657,9 +1657,11 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
exit(EXIT_FAILURE);
}
- if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
- error_report("AMD IOMMU xtsup=on requires support on the KVM side");
- exit(EXIT_FAILURE);
+ if (s->xtsup) {
+ if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
+ error_report("AMD IOMMU xtsup=on requires support on the KVM side");
+ exit(EXIT_FAILURE);
+ }
}
pci_setup_iommu(bus, &amdvi_iommu_ops, s);
It's admittedly a bit brittle, but it's already done in the neighboring
hw/i386/intel_iommu.c so I guess it's okay.
Paolo
