Re: [PATCH v3 2/4 gnumach] smp: Parallel SMP init

[Samuel Thibault]

Hello,

Damien Zammit via Bug reports for the GNU Hurd, le sam. 21 déc. 2024 23:55:25 
+0000, a ecrit:
> @@ -341,12 +312,16 @@ start_other_cpus(void)
>       lapic_disable();
>  
>       bspdone = 0;
> +
> +     /* This is set once for all APs to use */
> +     pmap_make_temporary_mapping();
> +
> +     smp_startup_cpus(apic_get_cpu_apic_id(0), apboot_addr);
> +
>       for (cpu = 1; cpu < ncpus; cpu++) {
>               machine_slot[cpu].running = FALSE;
>  
> -             //Start cpu
> -             printf("Starting AP %d\n", cpu);
> -             cpu_start(cpu);

This is introducing a race: we'd want a synchronization barrier between
setting running to FALSE and unleashing the cpu setup with bspdone++.

That being said, do we really need synchronizing on bspdone at all? the
BSP can as well just set all running to FALSE before calling
smp_startup_cpus, and then just wait for running to become TRUE on all
processors.

> +             printf("Waiting for AP %d\n", cpu);
>  
>               bspdone++;
>               do {
> @@ -357,6 +332,14 @@ start_other_cpus(void)
>       }
>       printf("BSP: Completed SMP init\n");
>  
> +     pmap_remove_temporary_mapping();
> +
> +     /* Flush TLB on all cpu groups */
> +     ncpus = (ncpus < 8) ? ncpus : 8;

This magic 8 definitely deserves a macro.

> +     for (cpu = 1; cpu < ncpus; cpu++) {
> +             interrupt_processor(cpu);
> +     }
> +
>       /* Re-enable IOAPIC interrupts as per setup */
>       lapic_enable();
>  }

Samuel