Re: [PATCH v2 gnumach] apic: Use cpuid to read the apic id for speed

[Samuel Thibault]

Applied, thanks!

Damien Zammit, le mer. 16 août 2023 01:44:50 +0000, a ecrit:
> ---
>  i386/i386/apic.c       | 11 +++++------
>  i386/i386/cpu_number.h | 20 +++++++++++++++++++-
>  i386/i386/mp_desc.c    |  3 +--
>  x86_64/locore.S        |  6 +++---
>  4 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> index 2bb8e3f1..3a51f506 100644
> --- a/i386/i386/apic.c
> +++ b/i386/i386/apic.c
> @@ -185,7 +185,11 @@ apic_get_num_ioapics(void)
>  int
>  apic_get_current_cpu(void)
>  {
> -    return (lapic->apic_id.r >> 24) & 0xff;
> +    unsigned int eax, ebx, ecx, edx;
> +    eax = 1;
> +    ecx = 0;
> +    cpuid(eax, ebx, ecx, edx);
> +    return (ebx >> 24);
>  }
>  
>  
> @@ -295,11 +299,6 @@ lapic_enable(void)
>      cpu_intr_save(&flags);
>  
>      apic_id = apic_get_current_cpu();
> -    if (apic_id < 0)
> -      {
> -        printf("apic_get_current_cpu() failed, assuming BSP\n");
> -        apic_id = 0;
> -      }
>  
>      dummy = lapic->dest_format.r;
>      lapic->dest_format.r = 0xffffffff;               /* flat model */
> diff --git a/i386/i386/cpu_number.h b/i386/i386/cpu_number.h
> index c00896e8..df086370 100644
> --- a/i386/i386/cpu_number.h
> +++ b/i386/i386/cpu_number.h
> @@ -39,12 +39,30 @@
>  #define      CX(addr, reg)   addr(,reg,8)
>  #endif
>  
> -#define      CPU_NUMBER(reg) \
> +#define      CPU_NUMBER_NO_STACK(reg)        \
>       movl    %cs:lapic, reg          ;\
>       movl    %cs:APIC_ID(reg), reg   ;\
>       shrl    $24, reg                ;\
>       movl    %cs:CX(cpu_id_lut, reg), reg    ;\
>  
> +/* Never call CPU_NUMBER(%esi) */
> +#define CPU_NUMBER(reg)              \
> +     pushl   %esi            ;\
> +     pushl   %eax            ;\
> +     pushl   %ebx            ;\
> +     pushl   %ecx            ;\
> +     pushl   %edx            ;\
> +     movl    $1, %eax        ;\
> +     cpuid                   ;\
> +     shrl    $24, %ebx       ;\
> +     movl    %cs:CX(cpu_id_lut, %ebx), %esi  ;\
> +     popl    %edx            ;\
> +     popl    %ecx            ;\
> +     popl    %ebx            ;\
> +     popl    %eax            ;\
> +     movl    %esi, reg       ;\
> +     popl    %esi            ;\
> +
>  #ifndef __ASSEMBLER__
>  #include "kern/cpu_number.h"
>  int cpu_number(void);
> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
> index 88fbb50a..f1a1f989 100644
> --- a/i386/i386/mp_desc.c
> +++ b/i386/i386/mp_desc.c
> @@ -275,8 +275,7 @@ cpu_setup(int cpu)
>  void
>  cpu_ap_main()
>  {
> -    unsigned apic_id = (((ApicLocalUnit*)phystokv(lapic_addr))->apic_id.r >> 
> 24) & 0xff;
> -    int cpu = apic_get_cpu_kernel_id(apic_id);
> +    int cpu = cpu_number();
>  
>      do {
>       cpu_pause();
> diff --git a/x86_64/locore.S b/x86_64/locore.S
> index a330d56b..c75feb23 100644
> --- a/x86_64/locore.S
> +++ b/x86_64/locore.S
> @@ -1171,7 +1171,7 @@ syscall_entry_2:
>       movq    %rdx,R_CS(%rsp)         /* fix cs */
>       movq    %rbx,R_EFLAGS(%rsp)     /* fix eflags */
>  
> -     CPU_NUMBER(%edx)
> +     CPU_NUMBER_NO_STACK(%edx)
>       TIME_TRAP_SENTRY
>  
>       movq    CX(EXT(kernel_stack),%rdx),%rbx
> @@ -1371,7 +1371,7 @@ ENTRY(syscall64)
>        * save only the callee-preserved status according to the C ABI,
>        * plus RIP and EFLAGS for sysret
>        */
> -     CPU_NUMBER(%r11)
> +     CPU_NUMBER_NO_STACK(%r11)
>       movq    CX(EXT(active_threads),%r11),%r11 /* point to current thread */
>       movq    TH_PCB(%r11),%r11               /* point to pcb */
>       addq    $ PCB_ISS,%r11                  /* point to saved state */
> @@ -1405,7 +1405,7 @@ ENTRY(syscall64)
>       mov     %r10,%rcx               /* fix arg3 location according to C ABI 
> */
>  
>       /* switch to kernel stack, then we can enable interrupts */
> -     CPU_NUMBER(%r11)
> +     CPU_NUMBER_NO_STACK(%r11)
>       movq    CX(EXT(kernel_stack),%r11),%rsp
>       sti
>  
> -- 
> 2.40.1
> 
> 
> 

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