Hi Phil,
On 11/10/2024 4:36 PM, Phil Dennis-Jordan wrote:
> Hi,
>
> This commit seems to be causing link errors, likely on all platforms
> where KVM is not available, but at minimum that's what I'm seeing when
> trying to build the staging branch on macOS.
>
> ld: Undefined symbols:
> _kvm_enable_x2apic, referenced from:
> _amdvi_sysbus_realize in hw_i386_amd_iommu.c.o
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
>
Hmm,.
Thank you for reporting.
> On Mon, 4 Nov 2024 at 22:10, Michael S. Tsirkin <m...@redhat.com> wrote:
>>
>> From: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
>>
>> The XTSup mode enables x2APIC support for AMD IOMMU, which is needed
>> to support vcpu w/ APIC ID > 255.
>>
>> Reviewed-by: Alejandro Jimenez <alejandro.j.jime...@oracle.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
>> Signed-off-by: Santosh Shukla <santosh.shu...@amd.com>
>> Message-Id: <20240927172913.121477-6-santosh.shu...@amd.com>
>> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
>> ---
>> hw/i386/amd_iommu.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 38297376e7..13af7211e1 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -32,6 +32,7 @@
>> #include "trace.h"
>> #include "hw/i386/apic-msidef.h"
>> #include "hw/qdev-properties.h"
>> +#include "kvm/kvm_i386.h"
>>
>> /* used AMD-Vi MMIO registers */
>> const char *amdvi_mmio_low[] = {
>> @@ -1651,6 +1652,16 @@ static void amdvi_sysbus_realize(DeviceState *dev,
>> Error **errp)
>> memory_region_add_subregion_overlap(&s->mr_sys, AMDVI_INT_ADDR_FIRST,
>> &s->mr_ir, 1);
>>
>> + /* AMD IOMMU with x2APIC mode requires xtsup=on */
>> + if (x86ms->apic_id_limit > 255 && !s->xtsup) {
>> + 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);
>> + }
>
> I suspect this last if() { … } block should be wrapped in an #ifdef
> CONFIG_KVM block? I don't know anything about this code though, so if
> this whole code path is generally a KVM-only feature, perhaps the KVM
> check should be implemented at the build system dependency level?
>
Could you please try below in your target system w/ clang and confirm back?
-----
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 13af7211e1..7081d448e4 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1657,9 +1657,12 @@ 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);
-----
Thank you!
Santosh
