num_processors is 0 by default and only gets incremented when local APICs
are registered.
Make init_apic_mappings(), which tries to enable the local APIC in the case
that no SMP configuration was found set num_processors to 1.
This allows to remove yet another check for the local APIC and yet another
place which registers the boot CPUs local APIC ID.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/kernel/apic/apic.c | 9 ++++++---
arch/x86/kernel/smpboot.c | 18 ------------------
2 files changed, 6 insertions(+), 21 deletions(-)
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2130,9 +2130,12 @@ void __init init_apic_mappings(void)
if (x2apic_mode)
return;
- if (!smp_found_config && !detect_init_APIC()) {
- pr_info("APIC: disable apic facility\n");
- apic_disable();
+ if (!smp_found_config) {
+ if (!detect_init_APIC()) {
+ pr_info("APIC: disable apic facility\n");
+ apic_disable();
+ }
+ num_processors = 1;
}
}
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1397,24 +1397,6 @@ early_param("possible_cpus", _setup_poss
{
int i, possible;
- /* No processor was found in mptable or ACPI MADT */
- if (!num_processors) {
- if (boot_cpu_has(X86_FEATURE_APIC)) {
- int apicid = boot_cpu_physical_apicid;
- int cpu = read_apic_id();
-
- pr_warn("Boot CPU (id %d) not listed by BIOS\n", cpu);
-
- /* Make sure boot cpu is enumerated */
- if (apic->cpu_present_to_apicid(0) == BAD_APICID &&
- apic->apic_id_valid(apicid))
- generic_processor_info(apicid);
- }
-
- if (!num_processors)
- num_processors = 1;
- }
-
i = setup_max_cpus ?: 1;
if (setup_possible_cpus == -1) {
possible = num_processors;
On 24.07.23 15:34, Thomas Gleixner wrote:
> num_processors is 0 by default and only gets incremented when local APICs
> are registered.
>
> Make init_apic_mappings(), which tries to enable the local APIC in the case
> that no SMP configuration was found set num_processors to 1.
>
> This allows to remove yet another check for the local APIC and yet another
> place which registers the boot CPUs local APIC ID.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Acked-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> arch/x86/kernel/apic/apic.c | 9 ++++++---
> arch/x86/kernel/smpboot.c | 18 ------------------
> 2 files changed, 6 insertions(+), 21 deletions(-)
>
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2130,9 +2130,12 @@ void __init init_apic_mappings(void)
> if (x2apic_mode)
> return;
>
> - if (!smp_found_config && !detect_init_APIC()) {
> - pr_info("APIC: disable apic facility\n");
> - apic_disable();
> + if (!smp_found_config) {
> + if (!detect_init_APIC()) {
> + pr_info("APIC: disable apic facility\n");
> + apic_disable();
> + }
> + num_processors = 1;
> }
> }
>
This is introducing a regression for Xen PV guests: those have no ACPI
tables, so smp_found_config will be 0. OTOH num_processors has been set
already using hypervisor data, so setting num_processors to 1 here will
overwrite the previous setting.
Below diff on top is fixing the problem:
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 564d584c8b99..59c12b20c635 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2135,7 +2135,13 @@ void __init init_apic_mappings(void)
pr_info("APIC: disable apic facility\n");
apic_disable();
}
- num_processors = 1;
+
+ /*
+ * Unprivileged Xen PV guests have smp_found_config = 0, but
+ * they have set num_processors already from hypervisor data.
+ */
+ if (!num_processors)
+ num_processors = 1;
}
}
Juergen
On Mon, Jul 31 2023 at 12:17, Juergen Gross wrote:
> On 24.07.23 15:34, Thomas Gleixner wrote:
> This is introducing a regression for Xen PV guests: those have no ACPI
> tables, so smp_found_config will be 0. OTOH num_processors has been set
> already using hypervisor data, so setting num_processors to 1 here will
> overwrite the previous setting.
>
> Below diff on top is fixing the problem:
Fixing? You can't be serious about that.
Why can't XENPV pretend that it has a smp configuration detected,
i.e. setting smp_found_config as any other special get_smp_config()
implementation does?
XENPV is already a major pain to deal with. No need to expand the
related insanity all over the place.
Thanks,
tglx
On Mon, Jul 31 2023 at 14:50, Thomas Gleixner wrote:
> Why can't XENPV pretend that it has a smp configuration detected,
> i.e. setting smp_found_config as any other special get_smp_config()
> implementation does?
The below should do the trick, no?
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -182,7 +182,8 @@ static void __init _get_smp_config(unsig
if (subtract)
set_nr_cpu_ids(nr_cpu_ids - subtract);
#endif
-
+ /* Pretend to be a proper enumerated system */
+ smp_found_config = 1;
}
static void __init xen_pv_smp_prepare_boot_cpu(void)
On 31.07.23 17:57, Thomas Gleixner wrote:
> On Mon, Jul 31 2023 at 14:50, Thomas Gleixner wrote:
>> Why can't XENPV pretend that it has a smp configuration detected,
>> i.e. setting smp_found_config as any other special get_smp_config()
>> implementation does?
>
> The below should do the trick, no?
Something like that, yes.
I'm just hunting another regression in the series. With patch 23 of the
topology series applied the APs of a Xen PV guests won't be onlined. I
guess this is due to missing topology data initialization somewhere in
the Xen related code.
I'll check your suggestion after finding the reason for the regression.
Juergen
>
>
> --- a/arch/x86/xen/smp_pv.c
> +++ b/arch/x86/xen/smp_pv.c
> @@ -182,7 +182,8 @@ static void __init _get_smp_config(unsig
> if (subtract)
> set_nr_cpu_ids(nr_cpu_ids - subtract);
> #endif
> -
> + /* Pretend to be a proper enumerated system */
> + smp_found_config = 1;
> }
>
> static void __init xen_pv_smp_prepare_boot_cpu(void)
On 31.07.23 20:19, Juergen Gross wrote:
> On 31.07.23 17:57, Thomas Gleixner wrote:
>> On Mon, Jul 31 2023 at 14:50, Thomas Gleixner wrote:
>>> Why can't XENPV pretend that it has a smp configuration detected,
>>> i.e. setting smp_found_config as any other special get_smp_config()
>>> implementation does?
>>
>> The below should do the trick, no?
>
> Something like that, yes.
>
> I'm just hunting another regression in the series. With patch 23 of the
> topology series applied the APs of a Xen PV guests won't be onlined. I
> guess this is due to missing topology data initialization somewhere in
> the Xen related code.
>
> I'll check your suggestion after finding the reason for the regression.
>
>>
>>
>> --- a/arch/x86/xen/smp_pv.c
>> +++ b/arch/x86/xen/smp_pv.c
>> @@ -182,7 +182,8 @@ static void __init _get_smp_config(unsig
>> if (subtract)
>> set_nr_cpu_ids(nr_cpu_ids - subtract);
>> #endif
>> -
>> + /* Pretend to be a proper enumerated system */
>> + smp_found_config = 1;
>> }
>> static void __init xen_pv_smp_prepare_boot_cpu(void)
>
It is working fine.
Juergen