Re: [PATCH v3 gnumach] percpu area using gs segment

[Samuel Thibault]

Damien Zammit, le sam. 23 sept. 2023 10:57:08 +0000, a ecrit:
> diff --git a/i386/i386/cpu_number.c b/i386/i386/cpu_number.c
> index ef19e11f..83c86d82 100644
> --- a/i386/i386/cpu_number.c
> +++ b/i386/i386/cpu_number.c
> @@ -20,11 +20,17 @@
>  #include <i386/smp.h>
>  #include <i386/cpu.h>
>  #include <i386/mp_desc.h>
> +#include <i386/percpu.h>
>  #include <kern/printf.h>
> 
>  #if NCPUS > 1
> -int cpu_number(void)
> +inline int cpu_number_slow(void)
>  {
>         return cpu_id_lut[apic_get_current_cpu()];
>  }
> +
> +inline int cpu_number(void)
> +{
> +       return percpu_get(int, cpu_id);
> +}
>  #endif

For an inline to actually get optimized, you have to put it in the .h
file, otherwise the compiler won't be able to inline it at all since it
won't have the definition when compiling other .c files.



> diff --git a/i386/i386/gdt.h b/i386/i386/gdt.h
> index 5def73cb..c7da012a 100644
> --- a/i386/i386/gdt.h
> +++ b/i386/i386/gdt.h
> @@ -77,11 +77,11 @@
>  
>  /*                   0x58               used by user TSS in 64bit mode */
>  
> +#ifndef __ASSEMBLER__
>  
>  extern struct real_descriptor gdt[GDTSZ];
>  
> @@ -117,4 +117,5 @@ extern struct real_descriptor gdt[GDTSZ];
>  extern void gdt_init(void);
>  extern void ap_gdt_init(int cpu);
>  
> +#endif /* __ASSEMBLER__ */
>  #endif /* _I386_GDT_ */
> diff --git a/i386/i386/i386asm.sym b/i386/i386/i386asm.sym
> index 436e296a..b19dfe7c 100644
> --- a/i386/i386/i386asm.sym
> +++ b/i386/i386/i386asm.sym
> @@ -154,17 +156,11 @@ expr    NPTES                                           
> PTES_PER_PAGE
>  expr INTEL_PTE_VALID|INTEL_PTE_WRITE                 INTEL_PTE_KERNEL
>  
>  expr IDTSZ
> -expr GDTSZ
> -expr LDTSZ
>  
>  expr KERNEL_RING
>  
>  expr KERNEL_CS
>  expr KERNEL_DS
> -expr KERNEL_TSS
> -#ifndef      MACH_PV_DESCRIPTORS
> -expr KERNEL_LDT
> -#endif       /* MACH_PV_DESCRIPTORS */
>  
>  expr (VM_MIN_KERNEL_ADDRESS>>PDESHIFT)*sizeof(pt_entry_t)    KERNELBASEPDE
>  
> diff --git a/i386/i386/locore.S b/i386/i386/locore.S
> index 55aa9d60..870db785 100644
> --- a/i386/i386/locore.S
> +++ b/i386/i386/locore.S
> @@ -33,6 +33,7 @@
>  #include <i386/proc_reg.h>
>  #include <i386/trap.h>
>  #include <i386/seg.h>
> +#include <i386/gdt.h>
>  #include <i386/ldt.h>
>  #include <i386/i386asm.h>
>  #include <i386/cpu_number.h>

As I wrote before, please put these into a separate commit, so that
people looking at the commit that does actually change behavior doesn't
contain unrelated changes that are just cleanups.



> diff --git a/i386/i386/percpu.c b/i386/i386/percpu.c
> new file mode 100644
> index 00000000..b2b8afa7
> --- /dev/null
> +++ b/i386/i386/percpu.c
> +#include <i386/smp.h>
> +#include <i386/apic.h>
> +#include <i386/percpu.h>
> +
> +struct percpu percpu_array[NCPUS] __aligned(0x8000) = {0};

Why aligning?


Samuel