| Message ID | e43028a51f8cea02421d0856376faada8ab186d4.1726840133.git.matthew.barnes@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/APIC: Remove x2APIC pure cluster mode | expand |
On 23.09.2024 16:35, Matthew Barnes wrote: > With the introduction of mixed x2APIC mode (using cluster addressing for > IPIs and physical for external interrupts) the use of pure cluster mode > doesn't have any benefit. > > Remove the mode itself, leaving only the code required for logical > addressing when sending IPIs. > > Implements: https://gitlab.com/xen-project/xen/-/issues/189 > > Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On Mon, 2024-09-23 at 15:35 +0100, Matthew Barnes wrote: > With the introduction of mixed x2APIC mode (using cluster addressing > for > IPIs and physical for external interrupts) the use of pure cluster > mode > doesn't have any benefit. > > Remove the mode itself, leaving only the code required for logical > addressing when sending IPIs. > > Implements: https://gitlab.com/xen-project/xen/-/issues/189 > > Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com> > --- > CHANGELOG.md | 1 + Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> ~ Oleksii > docs/misc/xen-command-line.pandoc | 4 +-- > xen/arch/x86/Kconfig | 12 -------- > xen/arch/x86/genapic/x2apic.c | 50 +++-------------------------- > -- > 4 files changed, 7 insertions(+), 60 deletions(-) > > diff --git a/CHANGELOG.md b/CHANGELOG.md > index 26e7d8dd2ac4..335e98b2e1a7 100644 > --- a/CHANGELOG.md > +++ b/CHANGELOG.md > @@ -9,6 +9,7 @@ The format is based on [Keep a > Changelog](https://keepachangelog.com/en/1.0.0/) > ### Changed > - On x86: > - Prefer ACPI reboot over UEFI ResetSystem() run time service > call. > + - Remove x2APIC cluster mode, leaving only physical and mixed > modes. > > ### Added > > diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen- > command-line.pandoc > index 959cf45b55d9..5ce63044ade8 100644 > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -2842,10 +2842,10 @@ the watchdog. > Permit use of x2apic setup for SMP environments. > > ### x2apic-mode (x86) > -> `= physical | cluster | mixed` > +> `= physical | mixed` > > > Default: `physical` if **FADT** mandates physical mode, otherwise > set at > -> build time by CONFIG_X2APIC_{PHYSICAL,LOGICAL,MIXED}. > +> build time by CONFIG_X2APIC_{PHYSICAL,MIXED}. > > In the case that x2apic is in use, this option switches between > modes to > address APICs in the system as interrupt destinations. > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig > index 62f0b5e0f4c5..ab862b083fce 100644 > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -275,18 +275,6 @@ config X2APIC_PHYSICAL > destination inter processor interrupts (IPIs) slightly > slower than > Logical Destination mode. > > -config X2APIC_CLUSTER > - bool "Cluster Destination mode" > - help > - When using this mode APICs are addressed using the Cluster > Logical > - Destination mode. > - > - Cluster Destination has the benefit of sending IPIs faster > since > - multiple APICs can be targeted as destinations of a single > IPI. > - However the vector space is shared between all CPUs on the > cluster, > - and hence using this mode reduces the number of available > vectors > - when compared to Physical mode. > - > config X2APIC_MIXED > bool "Mixed Destination mode" > help > diff --git a/xen/arch/x86/genapic/x2apic.c > b/xen/arch/x86/genapic/x2apic.c > index d531035fa42c..c277f4f79b0a 100644 > --- a/xen/arch/x86/genapic/x2apic.c > +++ b/xen/arch/x86/genapic/x2apic.c > @@ -63,26 +63,6 @@ static void cf_check > init_apic_ldr_x2apic_cluster(void) > cpumask_set_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu)); > } > > -static const cpumask_t *cf_check > vector_allocation_cpumask_x2apic_cluster( > - int cpu) > -{ > - return per_cpu(cluster_cpus, cpu); > -} > - > -static unsigned int cf_check cpu_mask_to_apicid_x2apic_cluster( > - const cpumask_t *cpumask) > -{ > - unsigned int cpu = cpumask_any(cpumask); > - unsigned int dest = per_cpu(cpu_2_logical_apicid, cpu); > - const cpumask_t *cluster_cpus = per_cpu(cluster_cpus, cpu); > - > - for_each_cpu ( cpu, cluster_cpus ) > - if ( cpumask_test_cpu(cpu, cpumask) ) > - dest |= per_cpu(cpu_2_logical_apicid, cpu); > - > - return dest; > -} > - > static void cf_check send_IPI_self_x2apic(uint8_t vector) > { > apic_wrmsr(APIC_SELF_IPI, vector); > @@ -169,17 +149,6 @@ static const struct genapic > __initconst_cf_clobber apic_x2apic_phys = { > .send_IPI_self = send_IPI_self_x2apic > }; > > -static const struct genapic __initconst_cf_clobber > apic_x2apic_cluster = { > - APIC_INIT("x2apic_cluster", NULL), > - .int_delivery_mode = dest_LowestPrio, > - .int_dest_mode = 1 /* logical delivery */, > - .init_apic_ldr = init_apic_ldr_x2apic_cluster, > - .vector_allocation_cpumask = > vector_allocation_cpumask_x2apic_cluster, > - .cpu_mask_to_apicid = cpu_mask_to_apicid_x2apic_cluster, > - .send_IPI_mask = send_IPI_mask_x2apic_cluster, > - .send_IPI_self = send_IPI_self_x2apic > -}; > - > /* > * Mixed x2APIC mode: use physical for external (device) interrupts, > and > * cluster for inter processor interrupts. Such mode has the > benefits of not > @@ -252,15 +221,13 @@ static int8_t __initdata x2apic_phys = -1; > boolean_param("x2apic_phys", x2apic_phys); > > enum { > - unset, physical, cluster, mixed > + unset, physical, mixed > } static __initdata x2apic_mode = unset; > > static int __init cf_check parse_x2apic_mode(const char *s) > { > if ( !cmdline_strcmp(s, "physical") ) > x2apic_mode = physical; > - else if ( !cmdline_strcmp(s, "cluster") ) > - x2apic_mode = cluster; > else if ( !cmdline_strcmp(s, "mixed") ) > x2apic_mode = mixed; > else > @@ -274,7 +241,7 @@ const struct genapic *__init > apic_x2apic_probe(void) > { > /* Honour the legacy cmdline setting if it's the only one > provided. */ > if ( x2apic_mode == unset && x2apic_phys >= 0 ) > - x2apic_mode = x2apic_phys ? physical : cluster; > + x2apic_mode = x2apic_phys ? physical : mixed; > > if ( x2apic_mode == unset ) > { > @@ -286,21 +253,12 @@ const struct genapic *__init > apic_x2apic_probe(void) > else > x2apic_mode = IS_ENABLED(CONFIG_X2APIC_MIXED) ? mixed > : (IS_ENABLED(CONFIG_X2APIC_PHYSICAL) ? > physical > - : > cluster); > + : > mixed); > } > > if ( x2apic_mode == physical ) > return &apic_x2apic_phys; > > - if ( x2apic_mode == cluster && iommu_intremap != > iommu_intremap_full ) > - { > - printk("WARNING: x2APIC cluster mode is not supported %s > interrupt remapping -" > - " forcing mixed mode\n", > - iommu_intremap == iommu_intremap_off ? "without" > - : "with > restricted"); > - x2apic_mode = mixed; > - } > - > if ( !this_cpu(cluster_cpus) ) > { > update_clusterinfo(NULL, CPU_UP_PREPARE, > @@ -309,7 +267,7 @@ const struct genapic *__init > apic_x2apic_probe(void) > register_cpu_notifier(&x2apic_cpu_nfb); > } > > - return x2apic_mode == cluster ? &apic_x2apic_cluster : > &apic_x2apic_mixed; > + return &apic_x2apic_mixed; > } > > void __init check_x2apic_preenabled(void)
On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: > With the introduction of mixed x2APIC mode (using cluster addressing for > IPIs and physical for external interrupts) the use of pure cluster mode > doesn't have any benefit. > > Remove the mode itself, leaving only the code required for logical > addressing when sending IPIs. > > Implements: https://gitlab.com/xen-project/xen/-/issues/189 There's at least one extra bit which I would also like to see removed, either in this patch, or as following patch. In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and pending_mask. After dropping cluster mode for external interrupts, those fields could become integers AFACT, as now interrupts can only target a single CPU opposed to a logical CPU set. Thanks, Roger.
On 24/09/2024 4:10 pm, Roger Pau Monné wrote: > On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: >> With the introduction of mixed x2APIC mode (using cluster addressing for >> IPIs and physical for external interrupts) the use of pure cluster mode >> doesn't have any benefit. >> >> Remove the mode itself, leaving only the code required for logical >> addressing when sending IPIs. >> >> Implements: https://gitlab.com/xen-project/xen/-/issues/189 We use the Resolves: tag for this. Can fix on commit. > There's at least one extra bit which I would also like to see removed, > either in this patch, or as following patch. > > In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and > pending_mask. After dropping cluster mode for external interrupts, > those fields could become integers AFACT, as now interrupts can only > target a single CPU opposed to a logical CPU set. A separate patch for sure, but that sounds like a great improvement. ~Andrew
On 24.09.2024 17:10, Roger Pau Monné wrote: > On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: >> With the introduction of mixed x2APIC mode (using cluster addressing for >> IPIs and physical for external interrupts) the use of pure cluster mode >> doesn't have any benefit. >> >> Remove the mode itself, leaving only the code required for logical >> addressing when sending IPIs. >> >> Implements: https://gitlab.com/xen-project/xen/-/issues/189 > > There's at least one extra bit which I would also like to see removed, > either in this patch, or as following patch. > > In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and > pending_mask. After dropping cluster mode for external interrupts, > those fields could become integers AFACT, as now interrupts can only > target a single CPU opposed to a logical CPU set. Yeah, that very much wants to be a separate change. Jan
On 23/09/2024 3:35 pm, Matthew Barnes wrote: > With the introduction of mixed x2APIC mode (using cluster addressing for > IPIs and physical for external interrupts) the use of pure cluster mode > doesn't have any benefit. > > Remove the mode itself, leaving only the code required for logical > addressing when sending IPIs. > > Implements: https://gitlab.com/xen-project/xen/-/issues/189 > > Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com> > --- > CHANGELOG.md | 1 + > docs/misc/xen-command-line.pandoc | 4 +-- > xen/arch/x86/Kconfig | 12 -------- > xen/arch/x86/genapic/x2apic.c | 50 +++---------------------------- > 4 files changed, 7 insertions(+), 60 deletions(-) > > diff --git a/CHANGELOG.md b/CHANGELOG.md > index 26e7d8dd2ac4..335e98b2e1a7 100644 > --- a/CHANGELOG.md > +++ b/CHANGELOG.md > @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) > ### Changed > - On x86: > - Prefer ACPI reboot over UEFI ResetSystem() run time service call. > + - Remove x2APIC cluster mode, leaving only physical and mixed modes. > > ### Added > There's a removed section just below this, which this line ought to be in. Also, it's cluster mode for external interrupts. The "external interrupts" bit is what distinguishes it from mixed mode. Can fix both on commit too. ~Andrew
On Tue, Sep 24, 2024 at 04:27:36PM +0100, Andrew Cooper wrote: > On 24/09/2024 4:10 pm, Roger Pau Monné wrote: > > On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: > >> With the introduction of mixed x2APIC mode (using cluster addressing for > >> IPIs and physical for external interrupts) the use of pure cluster mode > >> doesn't have any benefit. > >> > >> Remove the mode itself, leaving only the code required for logical > >> addressing when sending IPIs. > >> > >> Implements: https://gitlab.com/xen-project/xen/-/issues/189 > > We use the Resolves: tag for this. Can fix on commit. > > > There's at least one extra bit which I would also like to see removed, > > either in this patch, or as following patch. > > > > In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and > > pending_mask. After dropping cluster mode for external interrupts, > > those fields could become integers AFACT, as now interrupts can only > > target a single CPU opposed to a logical CPU set. > > A separate patch for sure, but that sounds like a great improvement. Oh, there are quite some fields of struct genapic that are not needed anymore, since both implementations set it to the same function. It would be helpful to prune them. Thanks, Roger.
On 24.09.2024 18:14, Roger Pau Monné wrote: > On Tue, Sep 24, 2024 at 04:27:36PM +0100, Andrew Cooper wrote: >> On 24/09/2024 4:10 pm, Roger Pau Monné wrote: >>> On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: >>>> With the introduction of mixed x2APIC mode (using cluster addressing for >>>> IPIs and physical for external interrupts) the use of pure cluster mode >>>> doesn't have any benefit. >>>> >>>> Remove the mode itself, leaving only the code required for logical >>>> addressing when sending IPIs. >>>> >>>> Implements: https://gitlab.com/xen-project/xen/-/issues/189 >> >> We use the Resolves: tag for this. Can fix on commit. >> >>> There's at least one extra bit which I would also like to see removed, >>> either in this patch, or as following patch. >>> >>> In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and >>> pending_mask. After dropping cluster mode for external interrupts, >>> those fields could become integers AFACT, as now interrupts can only >>> target a single CPU opposed to a logical CPU set. >> >> A separate patch for sure, but that sounds like a great improvement. > > Oh, there are quite some fields of struct genapic that are not needed > anymore, since both implementations set it to the same function. It > would be helpful to prune them. Pruning where possible - yes. But "both" won't cover it, as we have 4 instances of the struct (not just the two x2apic ones). Jan
On Tue, Sep 24, 2024 at 06:21:47PM +0200, Jan Beulich wrote: > On 24.09.2024 18:14, Roger Pau Monné wrote: > > On Tue, Sep 24, 2024 at 04:27:36PM +0100, Andrew Cooper wrote: > >> On 24/09/2024 4:10 pm, Roger Pau Monné wrote: > >>> On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: > >>>> With the introduction of mixed x2APIC mode (using cluster addressing for > >>>> IPIs and physical for external interrupts) the use of pure cluster mode > >>>> doesn't have any benefit. > >>>> > >>>> Remove the mode itself, leaving only the code required for logical > >>>> addressing when sending IPIs. > >>>> > >>>> Implements: https://gitlab.com/xen-project/xen/-/issues/189 > >> > >> We use the Resolves: tag for this. Can fix on commit. > >> > >>> There's at least one extra bit which I would also like to see removed, > >>> either in this patch, or as following patch. > >>> > >>> In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and > >>> pending_mask. After dropping cluster mode for external interrupts, > >>> those fields could become integers AFACT, as now interrupts can only > >>> target a single CPU opposed to a logical CPU set. > >> > >> A separate patch for sure, but that sounds like a great improvement. > > > > Oh, there are quite some fields of struct genapic that are not needed > > anymore, since both implementations set it to the same function. It > > would be helpful to prune them. > > Pruning where possible - yes. But "both" won't cover it, as we have 4 > instances of the struct (not just the two x2apic ones). Yeah, realized that afterwards, we still have the xAPIC flat mode, which is using logical delivery mode, but target a single CPU. So getting rid of the cpumask in arch_irq_desc seem possible, however there might be nothing to prune in struct genapic. Regards, Roger.
On 24/09/2024 6:10 pm, Roger Pau Monné wrote: > On Tue, Sep 24, 2024 at 06:21:47PM +0200, Jan Beulich wrote: >> On 24.09.2024 18:14, Roger Pau Monné wrote: >>> On Tue, Sep 24, 2024 at 04:27:36PM +0100, Andrew Cooper wrote: >>>> On 24/09/2024 4:10 pm, Roger Pau Monné wrote: >>>>> On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: >>>>>> With the introduction of mixed x2APIC mode (using cluster addressing for >>>>>> IPIs and physical for external interrupts) the use of pure cluster mode >>>>>> doesn't have any benefit. >>>>>> >>>>>> Remove the mode itself, leaving only the code required for logical >>>>>> addressing when sending IPIs. >>>>>> >>>>>> Implements: https://gitlab.com/xen-project/xen/-/issues/189 >>>> We use the Resolves: tag for this. Can fix on commit. >>>> >>>>> There's at least one extra bit which I would also like to see removed, >>>>> either in this patch, or as following patch. >>>>> >>>>> In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and >>>>> pending_mask. After dropping cluster mode for external interrupts, >>>>> those fields could become integers AFACT, as now interrupts can only >>>>> target a single CPU opposed to a logical CPU set. >>>> A separate patch for sure, but that sounds like a great improvement. >>> Oh, there are quite some fields of struct genapic that are not needed >>> anymore, since both implementations set it to the same function. It >>> would be helpful to prune them. >> Pruning where possible - yes. But "both" won't cover it, as we have 4 >> instances of the struct (not just the two x2apic ones). > Yeah, realized that afterwards, we still have the xAPIC flat mode, > which is using logical delivery mode, but target a single CPU. So > getting rid of the cpumask in arch_irq_desc seem possible, however > there might be nothing to prune in struct genapic. Logical delivery mode for external interrupts in xAPIC is just as broken/useless as the code we've just deleted. If that's the only thing in the way of more cleanup, we delete it too. ~Andrew
On Tue, Sep 24, 2024 at 07:20:44PM +0100, Andrew Cooper wrote: > On 24/09/2024 6:10 pm, Roger Pau Monné wrote: > > On Tue, Sep 24, 2024 at 06:21:47PM +0200, Jan Beulich wrote: > >> On 24.09.2024 18:14, Roger Pau Monné wrote: > >>> On Tue, Sep 24, 2024 at 04:27:36PM +0100, Andrew Cooper wrote: > >>>> On 24/09/2024 4:10 pm, Roger Pau Monné wrote: > >>>>> On Mon, Sep 23, 2024 at 03:35:59PM +0100, Matthew Barnes wrote: > >>>>>> With the introduction of mixed x2APIC mode (using cluster addressing for > >>>>>> IPIs and physical for external interrupts) the use of pure cluster mode > >>>>>> doesn't have any benefit. > >>>>>> > >>>>>> Remove the mode itself, leaving only the code required for logical > >>>>>> addressing when sending IPIs. > >>>>>> > >>>>>> Implements: https://gitlab.com/xen-project/xen/-/issues/189 > >>>> We use the Resolves: tag for this. Can fix on commit. > >>>> > >>>>> There's at least one extra bit which I would also like to see removed, > >>>>> either in this patch, or as following patch. > >>>>> > >>>>> In struct arch_irq_desc we have 3 cpumasks: cpu_mask, old_cpu_mask and > >>>>> pending_mask. After dropping cluster mode for external interrupts, > >>>>> those fields could become integers AFACT, as now interrupts can only > >>>>> target a single CPU opposed to a logical CPU set. > >>>> A separate patch for sure, but that sounds like a great improvement. > >>> Oh, there are quite some fields of struct genapic that are not needed > >>> anymore, since both implementations set it to the same function. It > >>> would be helpful to prune them. > >> Pruning where possible - yes. But "both" won't cover it, as we have 4 > >> instances of the struct (not just the two x2apic ones). > > Yeah, realized that afterwards, we still have the xAPIC flat mode, > > which is using logical delivery mode, but target a single CPU. So > > getting rid of the cpumask in arch_irq_desc seem possible, however > > there might be nothing to prune in struct genapic. > > Logical delivery mode for external interrupts in xAPIC is just as > broken/useless as the code we've just deleted. > > If that's the only thing in the way of more cleanup, we delete it too. Bah, xAPIC flat delivery mode needs to be adjusted to use physical delivery for external interrupts, as the vector space is shared between all CPUs in that mode. This must be done ahead of turning the arch_irq_desc cpumasks into integers. Sorry, this is turning into a more work that I originally expected, mostly because I wasn't taking into account that xAPIC was still using logical mode for external interrupts. Thanks, Roger.
diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e7d8dd2ac4..335e98b2e1a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Changed - On x86: - Prefer ACPI reboot over UEFI ResetSystem() run time service call. + - Remove x2APIC cluster mode, leaving only physical and mixed modes. ### Added diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 959cf45b55d9..5ce63044ade8 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -2842,10 +2842,10 @@ the watchdog. Permit use of x2apic setup for SMP environments. ### x2apic-mode (x86) -> `= physical | cluster | mixed` +> `= physical | mixed` > Default: `physical` if **FADT** mandates physical mode, otherwise set at -> build time by CONFIG_X2APIC_{PHYSICAL,LOGICAL,MIXED}. +> build time by CONFIG_X2APIC_{PHYSICAL,MIXED}. In the case that x2apic is in use, this option switches between modes to address APICs in the system as interrupt destinations. diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 62f0b5e0f4c5..ab862b083fce 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -275,18 +275,6 @@ config X2APIC_PHYSICAL destination inter processor interrupts (IPIs) slightly slower than Logical Destination mode. -config X2APIC_CLUSTER - bool "Cluster Destination mode" - help - When using this mode APICs are addressed using the Cluster Logical - Destination mode. - - Cluster Destination has the benefit of sending IPIs faster since - multiple APICs can be targeted as destinations of a single IPI. - However the vector space is shared between all CPUs on the cluster, - and hence using this mode reduces the number of available vectors - when compared to Physical mode. - config X2APIC_MIXED bool "Mixed Destination mode" help diff --git a/xen/arch/x86/genapic/x2apic.c b/xen/arch/x86/genapic/x2apic.c index d531035fa42c..c277f4f79b0a 100644 --- a/xen/arch/x86/genapic/x2apic.c +++ b/xen/arch/x86/genapic/x2apic.c @@ -63,26 +63,6 @@ static void cf_check init_apic_ldr_x2apic_cluster(void) cpumask_set_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu)); } -static const cpumask_t *cf_check vector_allocation_cpumask_x2apic_cluster( - int cpu) -{ - return per_cpu(cluster_cpus, cpu); -} - -static unsigned int cf_check cpu_mask_to_apicid_x2apic_cluster( - const cpumask_t *cpumask) -{ - unsigned int cpu = cpumask_any(cpumask); - unsigned int dest = per_cpu(cpu_2_logical_apicid, cpu); - const cpumask_t *cluster_cpus = per_cpu(cluster_cpus, cpu); - - for_each_cpu ( cpu, cluster_cpus ) - if ( cpumask_test_cpu(cpu, cpumask) ) - dest |= per_cpu(cpu_2_logical_apicid, cpu); - - return dest; -} - static void cf_check send_IPI_self_x2apic(uint8_t vector) { apic_wrmsr(APIC_SELF_IPI, vector); @@ -169,17 +149,6 @@ static const struct genapic __initconst_cf_clobber apic_x2apic_phys = { .send_IPI_self = send_IPI_self_x2apic }; -static const struct genapic __initconst_cf_clobber apic_x2apic_cluster = { - APIC_INIT("x2apic_cluster", NULL), - .int_delivery_mode = dest_LowestPrio, - .int_dest_mode = 1 /* logical delivery */, - .init_apic_ldr = init_apic_ldr_x2apic_cluster, - .vector_allocation_cpumask = vector_allocation_cpumask_x2apic_cluster, - .cpu_mask_to_apicid = cpu_mask_to_apicid_x2apic_cluster, - .send_IPI_mask = send_IPI_mask_x2apic_cluster, - .send_IPI_self = send_IPI_self_x2apic -}; - /* * Mixed x2APIC mode: use physical for external (device) interrupts, and * cluster for inter processor interrupts. Such mode has the benefits of not @@ -252,15 +221,13 @@ static int8_t __initdata x2apic_phys = -1; boolean_param("x2apic_phys", x2apic_phys); enum { - unset, physical, cluster, mixed + unset, physical, mixed } static __initdata x2apic_mode = unset; static int __init cf_check parse_x2apic_mode(const char *s) { if ( !cmdline_strcmp(s, "physical") ) x2apic_mode = physical; - else if ( !cmdline_strcmp(s, "cluster") ) - x2apic_mode = cluster; else if ( !cmdline_strcmp(s, "mixed") ) x2apic_mode = mixed; else @@ -274,7 +241,7 @@ const struct genapic *__init apic_x2apic_probe(void) { /* Honour the legacy cmdline setting if it's the only one provided. */ if ( x2apic_mode == unset && x2apic_phys >= 0 ) - x2apic_mode = x2apic_phys ? physical : cluster; + x2apic_mode = x2apic_phys ? physical : mixed; if ( x2apic_mode == unset ) { @@ -286,21 +253,12 @@ const struct genapic *__init apic_x2apic_probe(void) else x2apic_mode = IS_ENABLED(CONFIG_X2APIC_MIXED) ? mixed : (IS_ENABLED(CONFIG_X2APIC_PHYSICAL) ? physical - : cluster); + : mixed); } if ( x2apic_mode == physical ) return &apic_x2apic_phys; - if ( x2apic_mode == cluster && iommu_intremap != iommu_intremap_full ) - { - printk("WARNING: x2APIC cluster mode is not supported %s interrupt remapping -" - " forcing mixed mode\n", - iommu_intremap == iommu_intremap_off ? "without" - : "with restricted"); - x2apic_mode = mixed; - } - if ( !this_cpu(cluster_cpus) ) { update_clusterinfo(NULL, CPU_UP_PREPARE, @@ -309,7 +267,7 @@ const struct genapic *__init apic_x2apic_probe(void) register_cpu_notifier(&x2apic_cpu_nfb); } - return x2apic_mode == cluster ? &apic_x2apic_cluster : &apic_x2apic_mixed; + return &apic_x2apic_mixed; } void __init check_x2apic_preenabled(void)
With the introduction of mixed x2APIC mode (using cluster addressing for IPIs and physical for external interrupts) the use of pure cluster mode doesn't have any benefit. Remove the mode itself, leaving only the code required for logical addressing when sending IPIs. Implements: https://gitlab.com/xen-project/xen/-/issues/189 Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com> --- CHANGELOG.md | 1 + docs/misc/xen-command-line.pandoc | 4 +-- xen/arch/x86/Kconfig | 12 -------- xen/arch/x86/genapic/x2apic.c | 50 +++---------------------------- 4 files changed, 7 insertions(+), 60 deletions(-)