Re: [PATCH v3 1/4 gnumach] separate lapic_enable from lapic_setup

[Samuel Thibault]

Applied, thanks!

Damien Zammit, le mer. 07 févr. 2024 05:02:11 +0000, a ecrit:
> This initializes the lapic without
> turning on the IOAPIC interrupts during SMP init.
> 
> ---
>  i386/i386/apic.c     | 15 +++++++++++++--
>  i386/i386/apic.h     |  2 ++
>  i386/i386/mp_desc.c  | 18 +++++++++++++++---
>  i386/i386at/ioapic.c |  1 +
>  4 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> index 700fafd3..feb49c85 100644
> --- a/i386/i386/apic.c
> +++ b/i386/i386/apic.c
> @@ -308,6 +308,18 @@ void apic_send_ipi(unsigned dest_shorthand, unsigned 
> deliv_mode, unsigned dest_m
>  
>  void
>  lapic_enable(void)
> +{
> +    lapic->spurious_vector.r |= LAPIC_ENABLE;
> +}
> +
> +void
> +lapic_disable(void)
> +{
> +    lapic->spurious_vector.r &= ~LAPIC_ENABLE;
> +}
> +
> +void
> +lapic_setup(void)
>  {
>      unsigned long flags;
>      int apic_id;
> @@ -338,8 +350,7 @@ lapic_enable(void)
>      /* Enable LAPIC to send or recieve IPI/SIPIs */
>      dummy = lapic->spurious_vector.r;
>      lapic->spurious_vector.r = IOAPIC_SPURIOUS_BASE
> -                          | LAPIC_ENABLE_DIRECTED_EOI
> -                          | LAPIC_ENABLE;
> +                          | LAPIC_ENABLE_DIRECTED_EOI;
>  
>      lapic->error_status.r = 0;
>  
> diff --git a/i386/i386/apic.h b/i386/i386/apic.h
> index e1d49895..29387d9d 100644
> --- a/i386/i386/apic.h
> +++ b/i386/i386/apic.h
> @@ -244,6 +244,8 @@ int apic_get_total_gsis(void);
>  void picdisable(void);
>  void lapic_eoi(void);
>  void ioapic_irq_eoi(int pin);
> +void lapic_setup(void);
> +void lapic_disable(void);
>  void lapic_enable(void);
>  void lapic_enable_timer(void);
>  void calibrate_lapic_timer(void);
> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
> index 76b4a79c..a5f6b8f6 100644
> --- a/i386/i386/mp_desc.c
> +++ b/i386/i386/mp_desc.c
> @@ -270,6 +270,7 @@ cpu_setup(int cpu)
>      machine_slot[cpu].cpu_type = machine_slot[0].cpu_type;
>  
>      init_fpu();
> +    lapic_setup();
>      lapic_enable();
>      cpu_launch_first_thread(THREAD_NULL);
>  }
> @@ -313,13 +314,21 @@ start_other_cpus(void)
>       memcpy((void*) phystokv(apboot_addr), (void*) &apboot,
>              (uint32_t)&apbootend - (uint32_t)&apboot);
>  
> -#ifndef APIC
> -     lapic_enable(); /* Enable lapic only once */
> -#endif
>       unsigned cpu;
>  
>       splhigh();
>  
> +     /* Disable IOAPIC interrupts (IPIs not affected).
> +      * Clearing this flag is similar to masking all
> +      * IOAPIC interrupts individually.
> +      *
> +      * This is done to prevent IOAPIC interrupts from
> +      * interfering with SMP startup. splhigh() may be enough for BSP,
> +      * but I'm not sure.  We cannot control the lapic
> +      * on APs because we don't have execution on them yet.
> +      */
> +     lapic_disable();
> +
>       bspdone = 0;
>       for (cpu = 1; cpu < ncpus; cpu++) {
>               machine_slot[cpu].running = FALSE;
> @@ -336,5 +345,8 @@ start_other_cpus(void)
>               __sync_synchronize();
>       }
>       printf("BSP: Completed SMP init\n");
> +
> +     /* Re-enable IOAPIC interrupts as per setup */
> +     lapic_enable();
>  }
>  #endif       /* NCPUS > 1 */
> diff --git a/i386/i386at/ioapic.c b/i386/i386at/ioapic.c
> index 270362c3..2553a2c9 100644
> --- a/i386/i386at/ioapic.c
> +++ b/i386/i386at/ioapic.c
> @@ -458,5 +458,6 @@ ioapic_configure(void)
>      }
>  
>      /* Start the IO APIC receiving interrupts */
> +    lapic_setup();
>      lapic_enable();
>  }
> -- 
> 2.43.0
> 
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.