Re: [PATCH 5/6 gnumach] smp: Use logical destination not physical apic id

[Samuel Thibault]

Applied, thanks!

Damien Zammit via Bug reports for the GNU Hurd, le lun. 09 déc. 2024 12:17:53 
+0000, a ecrit:
> Since modern x86 cpus only support 4 bits of destination field
> in ICR, we could only address up to 16 processors, assuming
> their physical APIC ID was < 0x10.  Some processors eg AMD fam15h
> have physical apic ids starting at 0x10 but only support 4 bits.
> So these lapics are unaddressable using physical destination mode.
> 
> Therefore, we switch to using logical destinations for IPIs which
> gives us 8 bits of unique mask for addressing up to 8 groups of processors.
> 
> INIT and STARTUP is not changed here.
> 
> ---
>  i386/i386/apic.c      | 11 ++++++++---
>  i386/i386/apic.h      |  4 ++++
>  i386/i386/ast_check.c |  2 +-
>  i386/i386/mp_desc.c   |  2 +-
>  i386/i386/smp.c       | 14 +++++++-------
>  i386/i386/smp.h       |  4 ++--
>  6 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> index 0b5be50f..41b33599 100644
> --- a/i386/i386/apic.c
> +++ b/i386/i386/apic.c
> @@ -328,15 +328,20 @@ lapic_setup(void)
>      unsigned long flags;
>      int apic_id;
>      volatile uint32_t dummy;
> +    int cpu = cpu_number_slow();
>  
>      cpu_intr_save(&flags);
>  
> -    apic_id = apic_get_current_cpu();
> +    apic_id = apic_get_cpu_apic_id(cpu);
>  
> +    /* Flat model */
>      dummy = lapic->dest_format.r;
> -    lapic->dest_format.r = 0xffffffff;               /* flat model */
> +    lapic->dest_format.r = 0xffffffff;
> +
> +    /* Every 8th cpu is in the same logical group */
>      dummy = lapic->logical_dest.r;
> -    lapic->logical_dest.r = lapic->apic_id.r;        /* target self */
> +    lapic->logical_dest.r = 0x01000000 << (APIC_LOGICAL_ID(cpu));
> +
>      dummy = lapic->lvt_lint0.r;
>      lapic->lvt_lint0.r = dummy | LAPIC_DISABLE;
>      dummy = lapic->lvt_lint1.r;
> diff --git a/i386/i386/apic.h b/i386/i386/apic.h
> index 9eef0d8b..ec910456 100644
> --- a/i386/i386/apic.h
> +++ b/i386/i386/apic.h
> @@ -312,6 +312,10 @@ extern uint32_t *hpet_addr;
>  #define APIC_MSR_X2APIC                0x400 /* LAPIC is in x2APIC mode */
>  #define APIC_MSR_ENABLE                0x800 /* LAPIC is enabled */
>  
> +/* Since Logical Destination Register only has 8 bits of mask,
> + * we can only address 8 unique groups of cpus for IPIs.  */
> +#define APIC_LOGICAL_ID(cpu)             ((cpu) % 8)
> +
>  /* Set or clear a bit in a 255-bit APIC mask register.
>     These registers are spread through eight 32-bit registers.  */
>  #define APIC_SET_MASK_BIT(reg, bit) \
> diff --git a/i386/i386/ast_check.c b/i386/i386/ast_check.c
> index 61cd5e87..8bf69a68 100644
> --- a/i386/i386/ast_check.c
> +++ b/i386/i386/ast_check.c
> @@ -50,7 +50,7 @@ void init_ast_check(const processor_t processor)
>   */
>  void cause_ast_check(const processor_t processor)
>  {
> -    smp_remote_ast(apic_get_cpu_apic_id(processor->slot_num));
> +    smp_remote_ast(APIC_LOGICAL_ID(processor->slot_num));
>  }
>  
>  #endif       /* NCPUS > 1 */
> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
> index 8455f8ef..f13a391a 100644
> --- a/i386/i386/mp_desc.c
> +++ b/i386/i386/mp_desc.c
> @@ -210,7 +210,7 @@ cpu_control(int cpu, const int *info, unsigned int count)
>  void
>  interrupt_processor(int cpu)
>  {
> -     smp_pmap_update(apic_get_cpu_apic_id(cpu));
> +     smp_pmap_update(APIC_LOGICAL_ID(cpu));
>  }
>  
>  static void
> diff --git a/i386/i386/smp.c b/i386/i386/smp.c
> index 05e9de67..5861796a 100644
> --- a/i386/i386/smp.c
> +++ b/i386/i386/smp.c
> @@ -48,19 +48,19 @@ static void smp_data_init(void)
>  
>  }
>  
> -static void smp_send_ipi(unsigned apic_id, unsigned vector)
> +static void smp_send_ipi(unsigned logical_id, unsigned vector)
>  {
>      unsigned long flags;
>  
>      cpu_intr_save(&flags);
>  
> -    apic_send_ipi(NO_SHORTHAND, FIXED, PHYSICAL, ASSERT, EDGE, vector, 
> apic_id);
> +    apic_send_ipi(NO_SHORTHAND, FIXED, LOGICAL, ASSERT, EDGE, vector, 
> logical_id);
>  
>      do {
>          cpu_pause();
>      } while(lapic->icr_low.delivery_status == SEND_PENDING);
>  
> -    apic_send_ipi(NO_SHORTHAND, FIXED, PHYSICAL, DE_ASSERT, EDGE, vector, 
> apic_id);
> +    apic_send_ipi(NO_SHORTHAND, FIXED, LOGICAL, DE_ASSERT, EDGE, vector, 
> logical_id);
>  
>      do {
>          cpu_pause();
> @@ -69,14 +69,14 @@ static void smp_send_ipi(unsigned apic_id, unsigned 
> vector)
>      cpu_intr_restore(flags);
>  }
>  
> -void smp_remote_ast(unsigned apic_id)
> +void smp_remote_ast(unsigned logical_id)
>  {
> -    smp_send_ipi(apic_id, CALL_AST_CHECK);
> +    smp_send_ipi(logical_id, CALL_AST_CHECK);
>  }
>  
> -void smp_pmap_update(unsigned apic_id)
> +void smp_pmap_update(unsigned logical_id)
>  {
> -    smp_send_ipi(apic_id, CALL_PMAP_UPDATE);
> +    smp_send_ipi(logical_id, CALL_PMAP_UPDATE);
>  }
>  
>  static void
> diff --git a/i386/i386/smp.h b/i386/i386/smp.h
> index 73d273ef..a34f79da 100644
> --- a/i386/i386/smp.h
> +++ b/i386/i386/smp.h
> @@ -24,8 +24,8 @@
>  #include <mach/machine/vm_types.h>
>  
>  int smp_init(void);
> -void smp_remote_ast(unsigned apic_id);
> -void smp_pmap_update(unsigned apic_id);
> +void smp_remote_ast(unsigned logical_id);
> +void smp_pmap_update(unsigned logical_id);
>  int smp_startup_cpu(unsigned apic_id, phys_addr_t start_eip);
>  
>  #define cpu_pause() asm volatile ("pause" : : : "memory")
> -- 
> 2.45.2
> 
> 
> 

-- 
Samuel
<s> cool, j'ai un rapport a rendre pour le 31 decembre a minuit...
 -+- #ens-mim - bonne année ! -+-