diff mbox series

[PATCH-for-10.0,v2] hw/i386/amd_iommu: Simplify non-KVM checks on XTSup feature

Message ID 20241129155802.35534-1-philmd@linaro.org
State Superseded
Headers show
Series [PATCH-for-10.0,v2] hw/i386/amd_iommu: Simplify non-KVM checks on XTSup feature | expand

Commit Message

Philippe Mathieu-Daudé Nov. 29, 2024, 3:58 p.m. UTC 
Generic code wanting to access KVM specific methods should
do so being protected by the 'kvm_enabled()' helper.

Doing so avoid link failures when optimization is disabled
(using --enable-debug), see for example commits c04cfb4596a
("hw/i386: fix short-circuit logic with non-optimizing builds")
and 0266aef8cd6 ("amd_iommu: Fix kvm_enable_x2apic link error
with clang in non-KVM builds").

XTSup feature depends on KVM, so protect the whole block
checking the XTSup feature with a check on whether KVM is
enabled.

Since x86_cpus_init() already checks APIC ID > 255 imply
kernel support for irqchip and X2APIC, remove the confuse
and unlikely reachable "AMD IOMMU xtsup=on requires support
on the KVM side" message.

Fix a type in "configuration" in error message.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v2:
Rebased on merged 0266aef8cd6 ("amd_iommu: Fix kvm_enable_x2apic
link error with clang in non-KVM builds").
Supersedes: <20241129114113.33215-1-philmd@linaro.org>
---
 hw/i386/amd_iommu.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Vasant Hegde Dec. 18, 2024, 9:16 a.m. UTC | #1 
On 11/29/2024 9:28 PM, Philippe Mathieu-Daudé wrote:
> Generic code wanting to access KVM specific methods should
> do so being protected by the 'kvm_enabled()' helper.
> 
> Doing so avoid link failures when optimization is disabled
> (using --enable-debug), see for example commits c04cfb4596a
> ("hw/i386: fix short-circuit logic with non-optimizing builds")
> and 0266aef8cd6 ("amd_iommu: Fix kvm_enable_x2apic link error
> with clang in non-KVM builds").
> 
> XTSup feature depends on KVM, so protect the whole block
> checking the XTSup feature with a check on whether KVM is
> enabled.
> 
> Since x86_cpus_init() already checks APIC ID > 255 imply
> kernel support for irqchip and X2APIC, remove the confuse
> and unlikely reachable "AMD IOMMU xtsup=on requires support
> on the KVM side" message.
> 
> Fix a type in "configuration" in error message.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks for the fix. Looks good to me.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant
Philippe Mathieu-Daudé Dec. 31, 2024, 5:36 p.m. UTC | #2 
On 29/11/24 16:58, Philippe Mathieu-Daudé wrote:
> Generic code wanting to access KVM specific methods should
> do so being protected by the 'kvm_enabled()' helper.
> 
> Doing so avoid link failures when optimization is disabled
> (using --enable-debug), see for example commits c04cfb4596a
> ("hw/i386: fix short-circuit logic with non-optimizing builds")
> and 0266aef8cd6 ("amd_iommu: Fix kvm_enable_x2apic link error
> with clang in non-KVM builds").
> 
> XTSup feature depends on KVM, so protect the whole block
> checking the XTSup feature with a check on whether KVM is
> enabled.
> 
> Since x86_cpus_init() already checks APIC ID > 255 imply
> kernel support for irqchip and X2APIC, remove the confuse
> and unlikely reachable "AMD IOMMU xtsup=on requires support
> on the KVM side" message.
> 
> Fix a type in "configuration" in error message.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v2:
> Rebased on merged 0266aef8cd6 ("amd_iommu: Fix kvm_enable_x2apic
> link error with clang in non-KVM builds").
> Supersedes: <20241129114113.33215-1-philmd@linaro.org>
> ---
>   hw/i386/amd_iommu.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)

Patch queued, thanks.
diff mbox series

Patch

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index af0f4da1f69..39b6d6ef295 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1652,17 +1652,10 @@  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");
+    if (kvm_enabled() && x86ms->apic_id_limit > 255 && !s->xtsup) {
+        error_report("AMD IOMMU with x2APIC configuration requires xtsup=on");
         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);
     amdvi_init(s);