diff mbox series

x86/x2apic: remove usage of ACPI_FADT_APIC_CLUSTER

Message ID 20231024102630.47691-1-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/x2apic: remove usage of ACPI_FADT_APIC_CLUSTER | expand

Commit Message

Roger Pau Monné Oct. 24, 2023, 10:26 a.m. UTC
The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
Logical mode APIC must be configured for Cluster destination model.  However in
apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
can be used.

Since Xen when in x2APIC mode only uses Logical mode together with Cluster
model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
fulfills the requirement signaled by the flag.

Fixes: eb40ae41b658 ('x86/Kconfig: add option for default x2APIC destination mode')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/genapic/x2apic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jan Beulich Oct. 30, 2023, 2:12 p.m. UTC | #1
On 24.10.2023 12:26, Roger Pau Monne wrote:
> The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
> Logical mode APIC must be configured for Cluster destination model.  However in
> apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
> can be used.
> 
> Since Xen when in x2APIC mode only uses Logical mode together with Cluster
> model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
> fulfills the requirement signaled by the flag.
> 
> Fixes: eb40ae41b658 ('x86/Kconfig: add option for default x2APIC destination mode')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich Oct. 30, 2023, 2:13 p.m. UTC | #2
On 24.10.2023 12:26, Roger Pau Monne wrote:
> The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
> Logical mode APIC must be configured for Cluster destination model.  However in
> apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
> can be used.
> 
> Since Xen when in x2APIC mode only uses Logical mode together with Cluster
> model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
> fulfills the requirement signaled by the flag.

Actually, one remark: The text in the 6.5 spec really only mentions xAPIC
mode, so it's not entirely clear whether the two flags actually have any
meaning for x2APIC mode.

Jan
Roger Pau Monné Oct. 30, 2023, 4:19 p.m. UTC | #3
On Mon, Oct 30, 2023 at 03:13:50PM +0100, Jan Beulich wrote:
> On 24.10.2023 12:26, Roger Pau Monne wrote:
> > The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
> > Logical mode APIC must be configured for Cluster destination model.  However in
> > apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
> > can be used.
> > 
> > Since Xen when in x2APIC mode only uses Logical mode together with Cluster
> > model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
> > fulfills the requirement signaled by the flag.
> 
> Actually, one remark: The text in the 6.5 spec really only mentions xAPIC
> mode, so it's not entirely clear whether the two flags actually have any
> meaning for x2APIC mode.

Hm, indeed.  That wants to be in a different fix however.

Thanks, Roger.
Jan Beulich Oct. 30, 2023, 4:32 p.m. UTC | #4
On 30.10.2023 17:19, Roger Pau Monné wrote:
> On Mon, Oct 30, 2023 at 03:13:50PM +0100, Jan Beulich wrote:
>> On 24.10.2023 12:26, Roger Pau Monne wrote:
>>> The ACPI FADT APIC_CLUSTER flag mandates that when the interrupt delivery is
>>> Logical mode APIC must be configured for Cluster destination model.  However in
>>> apic_x2apic_probe() such flag is incorrectly used to gate whether Physical mode
>>> can be used.
>>>
>>> Since Xen when in x2APIC mode only uses Logical mode together with Cluster
>>> model completely remove checking for ACPI_FADT_APIC_CLUSTER, as Xen always
>>> fulfills the requirement signaled by the flag.
>>
>> Actually, one remark: The text in the 6.5 spec really only mentions xAPIC
>> mode, so it's not entirely clear whether the two flags actually have any
>> meaning for x2APIC mode.
> 
> Hm, indeed.  That wants to be in a different fix however.

If any at all - it wouldn't be the first time that when adding x2APIC, editing
certain places simply didn't happen. If anything needs changing here, of course
I agree that the further adjustment would want to be a separate change.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/genapic/x2apic.c b/xen/arch/x86/genapic/x2apic.c
index ca1db27157e2..707deef98c27 100644
--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -231,8 +231,7 @@  const struct genapic *__init apic_x2apic_probe(void)
          */
         x2apic_phys = iommu_intremap != iommu_intremap_full ||
                       (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) ||
-                      (IS_ENABLED(CONFIG_X2APIC_PHYSICAL) &&
-                       !(acpi_gbl_FADT.flags & ACPI_FADT_APIC_CLUSTER));
+                      IS_ENABLED(CONFIG_X2APIC_PHYSICAL);
     }
     else if ( !x2apic_phys )
         switch ( iommu_intremap )