Re: [PATCH gnumach] smp: Remove hardcoded AP_BOOT_ADDR

[Samuel Thibault]

Damien Zammit, le lun. 29 janv. 2024 10:07:30 +0000, a ecrit:
> -     ljmp    $BOOT_CS, $M(0f)
> +     xorl    %eax, %eax
> +     mov     %cs, %ax
> +     shll    $4, %eax
> +     addl    $M(0f), %eax
> +     movl    %eax, M(ljmp_offset32)

This won't work with pipelined processors, which assume a complete
separation between code and data, and will thus have already loaded
the jmp instruction before your modify it.

Rather either perform the relocation from the C code, or use a variable,
which you can refer from the ljmp instruction.

> +     /* ljmpl with relocation */
> +     .byte 0x66
> +     .byte 0xea
> +ljmp_offset32:
> +     .long 0xffffffff
> +     .word BOOT_CS
> +
>  0:
>       .code32
> +     /* Protected mode! */
>       movw    $BOOT_DS, %ax
>       movw    %ax, %ds
>       movw    %ax, %es
> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
> index 467f2728..76b4a79c 100644
> --- a/i386/i386/mp_desc.c
> +++ b/i386/i386/mp_desc.c
> @@ -99,6 +99,7 @@ interrupt_stack_alloc(void)
>   */
>  int bspdone;
>  
> +phys_addr_t apboot_addr;
>  extern void *apboot, *apbootend;
>  extern volatile ApicLocalUnit* lapic;
>  
> @@ -296,7 +297,7 @@ cpu_start(int cpu)
>  
>      printf("Trying to enable: %d\n", apic_id);
>  
> -    smp_startup_cpu(apic_id, AP_BOOT_ADDR);
> +    smp_startup_cpu(apic_id, apboot_addr);
>  
>      printf("Started cpu %d (lapic id %04x)\n", cpu, apic_id);
>  
> @@ -309,7 +310,7 @@ start_other_cpus(void)
>       int ncpus = smp_get_numcpus();
>  
>       //Copy cpu initialization assembly routine
> -     memcpy((void*)phystokv(AP_BOOT_ADDR), (void*) &apboot,
> +     memcpy((void*) phystokv(apboot_addr), (void*) &apboot,
>              (uint32_t)&apbootend - (uint32_t)&apboot);
>  
>  #ifndef APIC
> diff --git a/i386/i386/mp_desc.h b/i386/i386/mp_desc.h
> index fea42cd3..bcc68662 100644
> --- a/i386/i386/mp_desc.h
> +++ b/i386/i386/mp_desc.h
> @@ -46,8 +46,6 @@
>  #include "gdt.h"
>  #include "ldt.h"
>  
> -#define AP_BOOT_ADDR 0x7000
> -
>  /*
>   * The descriptor tables are together in a structure
>   * allocated one per processor (except for the boot processor).
> @@ -78,6 +76,11 @@ extern uint8_t solid_intstack[];
>  
>  extern int bspdone;
>  
> +/*
> + * Address to hold AP boot code, held in ASM
> + */
> +extern phys_addr_t apboot_addr;
> +
>  /*
>   * Each CPU calls this routine to set up its descriptor tables.
>   */
> diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
> index b0a55754..fb95029f 100644
> --- a/i386/i386at/model_dep.c
> +++ b/i386/i386at/model_dep.c
> @@ -66,6 +66,7 @@
>  #include <i386/locore.h>
>  #include <i386/model_dep.h>
>  #include <i386/smp.h>
> +#include <i386/seg.h>
>  #include <i386at/acpi_parse_apic.h>
>  #include <i386at/autoconf.h>
>  #include <i386at/biosmem.h>
> @@ -125,6 +126,9 @@ char *kernel_cmdline = "";
>  
>  extern char  version[];
>  
> +/* Realmode temporary GDT */
> +extern struct pseudo_descriptor gdt_descr_tmp;
> +
>  /* If set, reboot the system on ctrl-alt-delete.  */
>  boolean_t    rebootflag = FALSE;     /* exported to kdintr */
>  
> @@ -201,6 +205,20 @@ void machine_init(void)
>        */
>       pmap_unmap_page_zero();
>  #endif
> +
> +#ifdef APIC
> +     /*
> +      * Grab an early page for AP boot code
> +      */
> +     /* FIXME: this may not allocate from below 1MB, if within first 16MB */
> +     apboot_addr = vm_page_to_pa(vm_page_grab_contig(PAGE_SIZE, 
> VM_PAGE_SEL_DMA));
> +     assert (apboot_addr < 0x100000);
> +
> +     /*
> +      * Patch the realmode gdt with the correct offset
> +      */
> +     gdt_descr_tmp.linear_base += apboot_addr;
> +#endif
>  }
>  
>  /* Conserve power on processor CPU.  */
> -- 
> 2.43.0
> 
> 
> 

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