bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 3/3 gnumach] i386: Refactor int stacks to be per cpu for S


From: Almudena Garcia
Subject: Re: [PATCH v2 3/3 gnumach] i386: Refactor int stacks to be per cpu for SMP
Date: Fri, 3 Feb 2023 13:12:39 +0100

+#ifndef MACH_HYP
+    /* Turn paging on.
+     * TODO: Why does setting the WP bit here cause a crash?
+     */
+    set_cr0(get_cr0() | CR0_PG /* | CR0_WP */);
+    set_cr0(get_cr0() & ~(CR0_CD | CR0_NW));
+    if (CPU_HAS_FEATURE(CPU_FEATURE_PGE))
+        set_cr4(get_cr4() | CR4_PGE);

Again, you need some checks more. And, why not separate this in a function?
Check my original function

https://github.com/AlmuHS/GNUMach_SMP/blob/a31999c018c22d23402d3e02b5dfd595d4a29230/i386/i386/mp_desc.c#L365-L392

I copy its code here, if you don't want to follow the link

void paging_setup(){

#if PAE
    set_cr3(pdpbase_addr);
#ifndef	MACH_HYP
    if (!CPU_HAS_FEATURE(CPU_FEATURE_PAE))
        set_cr4(get_cr4() | CR4_PAE);
#endif	/* MACH_HYP */
#else
    set_cr3(kernel_page_dir_addr);
#endif	/* PAE */
#ifndef	MACH_HYP
    /* Turn paging on.
     * Also set the WP bit so that on 486 or better processors
     * page-level write protection works in kernel mode.
     */
    set_cr0(get_cr0() | CR0_PG | CR0_WP);
    set_cr0(get_cr0() & ~(CR0_CD | CR0_NW));

    if (CPU_HAS_FEATURE(CPU_FEATURE_PGE))
        set_cr4(get_cr4() | CR4_PGE);

#endif	/* MACH_HYP */

    flush_instr_queue();
    flush_tlb();

}

El vie, 3 feb 2023 a las 11:03, Damien Zammit (<damien@zamaudio.com>) escribió:
This also serialises the AP bringup, so paging can be enabled per cpu
one by one.

Also-by: Almudena Garcia <liberamenso10000@gmail.com>
---
 i386/i386/mp_desc.c     | 226 ++++++++++++++++++++++++++++------------
 i386/i386/mp_desc.h     |   7 +-
 i386/i386at/boothdr.S   |  18 +++-
 i386/i386at/ioapic.c    |   5 +-
 i386/i386at/model_dep.c |  33 +++---
 5 files changed, 197 insertions(+), 92 deletions(-)

diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
index bcf2fbe7..0db29291 100644
--- a/i386/i386/mp_desc.c
+++ b/i386/i386/mp_desc.c
@@ -24,25 +24,36 @@
  * the rights to redistribute these changes.
  */

-#if    NCPUS > 1
-
-#include <string.h>
-
 #include <kern/cpu_number.h>
 #include <kern/debug.h>
 #include <kern/printf.h>
+#include <kern/smp.h>
+#include <kern/startup.h>
+#include <kern/kmutex.h>
 #include <mach/machine.h>
 #include <mach/xen.h>
 #include <vm/vm_kern.h>

 #include <i386/mp_desc.h>
 #include <i386/lock.h>
+#include <i386/apic.h>
+#include <i386/locore.h>
+#include <i386/gdt.h>
+#include <i386at/idt.h>
+#include <i386at/int_init.h>
+#include <i386/cpu.h>
+#include <i386/smp.h>
+
 #include <i386at/model_dep.h>
 #include <machine/ktss.h>
+#include <machine/smp.h>
 #include <machine/tss.h>
 #include <machine/io_perm.h>
 #include <machine/vm_param.h>

+#include <i386at/acpi_parse_apic.h>
+#include <string.h>
+
 /*
  * The i386 needs an interrupt stack to keep the PCB stack from being
  * overrun by interrupts.  All interrupt stacks MUST lie at lower addresses
@@ -52,20 +63,35 @@
 /*
  * Addresses of bottom and top of interrupt stacks.
  */
-vm_offset_t    interrupt_stack[NCPUS];
 vm_offset_t    int_stack_top[NCPUS];
 vm_offset_t    int_stack_base[NCPUS];

-/*
- * Barrier address.
- */
-vm_offset_t    int_stack_high;
+/* Interrupt stack allocation */
+uint8_t solid_intstack[NCPUS*INTSTACK_SIZE] __aligned(NCPUS*INTSTACK_SIZE);
+
+void
+interrupt_stack_alloc(void)
+{
+       int i;
+
+       /*
+        * Set up pointers to the top of the interrupt stack.
+        */

+       for (i = 0; i < NCPUS; i++) {
+               int_stack_base[i] = (vm_offset_t) &solid_intstack[i * INTSTACK_SIZE];
+               int_stack_top[i] = (vm_offset_t) &solid_intstack[(i + 1) * INTSTACK_SIZE] - 4;
+       }
+}
+
+#if    NCPUS > 1
 /*
- * First cpu`s interrupt stack.
+ * Flag to mark SMP init by BSP complete
  */
-extern char            _intstack[];    /* bottom */
-extern char            _eintstack[];   /* top */
+int bspdone;
+
+extern void *apboot, *apbootend;
+extern volatile ApicLocalUnit* lapic;

 /*
  * Multiprocessor i386/i486 systems use a separate copy of the
@@ -77,7 +103,7 @@ extern char          _eintstack[];   /* top */
  */

 /*
- * Allocated descriptor tables.
+ * Descriptor tables.
  */
 struct mp_desc_table   *mp_desc_table[NCPUS] = { 0 };

@@ -102,12 +128,13 @@ extern struct real_descriptor     ldt[LDTSZ];
  * Allocate and initialize the per-processor descriptor tables.
  */

-struct mp_desc_table *
+int
 mp_desc_init(int mycpu)
 {
        struct mp_desc_table *mpt;
+       vm_offset_t mem;

-       if (mycpu == master_cpu) {
+       if (mycpu == 0) {
                /*
                 * Master CPU uses the tables built at boot time.
                 * Just set the TSS and GDT pointers.
@@ -118,61 +145,28 @@ mp_desc_init(int mycpu)
        }
        else {
                /*
-                * Other CPUs allocate the table from the bottom of
-                * the interrupt stack.
+                * Allocate tables for other CPUs
                 */
-               mpt = (struct mp_desc_table *) interrupt_stack[mycpu];
+               if (!init_alloc_aligned(sizeof(struct mp_desc_table), &mem))
+                       panic("not enough memory for descriptor tables");
+               mpt = (struct mp_desc_table *)phystokv(mem);

                mp_desc_table[mycpu] = mpt;
                mp_ktss[mycpu] = &mpt->ktss;
                mp_gdt[mycpu] = mpt->gdt;

                /*
-                * Copy the tables
+                * Zero the tables
                 */
-               memcpy(mpt->idt,
-                 idt,
-                 sizeof(idt));
-               memcpy(mpt->gdt,
-                 gdt,
-                 sizeof(gdt));
-               memcpy(mpt->ldt,
-                 ldt,
-                 sizeof(ldt));
-               memset(&mpt->ktss, 0,
-                 sizeof(struct task_tss));
+               memset(mpt->idt, 0, sizeof(idt));
+               memset(mpt->gdt, 0, sizeof(gdt));
+               memset(mpt->ldt, 0, sizeof(ldt));
+               memset(&mpt->ktss, 0, sizeof(struct task_tss));

-               /*
-                * Fix up the entries in the GDT to point to
-                * this LDT and this TSS.
-                */
-#ifdef MACH_RING1
-               panic("TODO %s:%d\n",__FILE__,__LINE__);
-#else  /* MACH_RING1 */
-               _fill_gdt_sys_descriptor(mpt->gdt, KERNEL_LDT,
-                       (unsigned)&mpt->ldt,
-                       LDTSZ * sizeof(struct real_descriptor) - 1,
-                       ACC_P|ACC_PL_K|ACC_LDT, 0);
-               _fill_gdt_sys_descriptor(mpt->gdt, KERNEL_TSS,
-                       (unsigned)&mpt->ktss,
-                       sizeof(struct task_tss) - 1,
-                       ACC_P|ACC_PL_K|ACC_TSS, 0);
-
-               mpt->ktss.tss.ss0 = KERNEL_DS;
-               mpt->ktss.tss.io_bit_map_offset = IOPB_INVAL;
-               mpt->ktss.barrier = 0xFF;
-#endif /* MACH_RING1 */
-
-               return mpt;
+               return mycpu;
        }
 }

-static kern_return_t intel_startCPU(int slot_num)
-{
-       printf("TODO: intel_startCPU\n");
-       return KERN_FAILURE;
-}
-
 /* XXX should be adjusted per CPU speed */
 int simple_lock_pause_loop = 100;

@@ -206,24 +200,124 @@ void
 interrupt_processor(int cpu)
 {
        printf("interrupt cpu %d\n",cpu);
+       smp_pmap_update(apic_get_cpu_apic_id(cpu));
+}
+
+void
+cpu_setup(int cpu)
+{
+    printf("AP=(%u) before\n", cpu);
+
+    pmap_make_temporary_mapping();
+    printf("AP=(%u) tempmap done\n", cpu);
+
+#ifndef MACH_HYP
+    /* Turn paging on.
+     * TODO: Why does setting the WP bit here cause a crash?
+     */
+    set_cr0(get_cr0() | CR0_PG /* | CR0_WP */);
+    set_cr0(get_cr0() & ~(CR0_CD | CR0_NW));
+    if (CPU_HAS_FEATURE(CPU_FEATURE_PGE))
+        set_cr4(get_cr4() | CR4_PGE);
+#endif  /* MACH_HYP */
+    flush_instr_queue();
+    printf("AP=(%u) paging done\n", cpu);
+
+    mp_desc_init(cpu);
+    printf("AP=(%u) mpdesc done\n", cpu);
+
+    ap_gdt_init(cpu);
+    printf("AP=(%u) gdt done\n", cpu);
+
+    ap_idt_init(cpu);
+    printf("AP=(%u) idt done\n", cpu);
+
+    ap_int_init(cpu);
+    printf("AP=(%u) int done\n", cpu);
+
+    ap_ldt_init(cpu);
+    printf("AP=(%u) ldt done\n", cpu);
+
+    ap_ktss_init(cpu);
+    printf("AP=(%u) ktss done\n", cpu);
+
+    pmap_remove_temporary_mapping();
+    printf("AP=(%u) remove tempmap done\n", cpu);
+
+    pmap_set_page_dir();
+    flush_tlb();
+    printf("AP=(%u) reset page dir done\n", cpu);
+
+    /* Initialize machine_slot fields with the cpu data */
+    machine_slot[cpu].cpu_subtype = CPU_SUBTYPE_AT386;
+    machine_slot[cpu].cpu_type = machine_slot[0].cpu_type;
+
+    lapic_enable();
+    cpu_launch_first_thread(THREAD_NULL);
+}
+
+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);
+
+    do {
+        asm volatile ("pause" : : : "memory");
+    } while (bspdone != cpu);
+
+    __sync_synchronize();
+
+    cpu_setup(cpu);
 }

 kern_return_t
 cpu_start(int cpu)
 {
-       if (machine_slot[cpu].running)
-               return KERN_FAILURE;
+    assert(machine_slot[cpu].running != TRUE);
+
+    uint16_t apic_id = apic_get_cpu_apic_id(cpu);
+
+    printf("Trying to enable: %d\n", apic_id);

-       return intel_startCPU(cpu);
+    smp_startup_cpu(apic_id, AP_BOOT_ADDR);
+
+    printf("Started cpu %d (lapic id %04x)\n", cpu, apic_id);
+
+    return KERN_SUCCESS;
 }

 void
 start_other_cpus(void)
 {
-       int cpu;
-       for (cpu = 0; cpu < NCPUS; cpu++)
-               if (cpu != cpu_number())
-                       cpu_start(cpu);
-}
+       int ncpus = smp_get_numcpus();
+
+       //Copy cpu initialization assembly routine
+       memcpy((void*)phystokv(AP_BOOT_ADDR), (void*) &apboot,
+              (uint32_t)&apbootend - (uint32_t)&apboot);

+#ifndef APIC
+       lapic_enable(); /* Enable lapic only once */
+#endif
+       unsigned cpu;
+
+       splhigh();
+
+       bspdone = 0;
+       for (cpu = 1; cpu < ncpus; cpu++) {
+               machine_slot[cpu].running = FALSE;
+
+               //Start cpu
+               printf("Starting AP %d\n", cpu);
+               cpu_start(cpu);
+
+               bspdone++;
+               do {
+                       asm volatile ("pause" : : : "memory");
+               } while (machine_slot[cpu].running == FALSE);
+
+               __sync_synchronize();
+       }
+       printf("BSP: Completed SMP init\n");
+}
 #endif /* NCPUS > 1 */
diff --git a/i386/i386/mp_desc.h b/i386/i386/mp_desc.h
index ede8775f..59d50e77 100644
--- a/i386/i386/mp_desc.h
+++ b/i386/i386/mp_desc.h
@@ -46,6 +46,8 @@
 #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).
@@ -72,11 +74,12 @@ extern struct task_tss              *mp_ktss[NCPUS];
  */
 extern struct real_descriptor  *mp_gdt[NCPUS];

+extern uint8_t solid_intstack[];

 /*
  * Each CPU calls this routine to set up its descriptor tables.
  */
-extern struct mp_desc_table *  mp_desc_init(int);
+extern int mp_desc_init(int);


 extern void interrupt_processor(int cpu);
@@ -90,4 +93,6 @@ extern kern_return_t cpu_start(int cpu);

 extern kern_return_t cpu_control(int cpu, const int *info, unsigned int count);

+extern void interrupt_stack_alloc(void);
+
 #endif /* _I386_MP_DESC_H_ */
diff --git a/i386/i386at/boothdr.S b/i386/i386at/boothdr.S
index 82d4b34a..d1d1fa51 100644
--- a/i386/i386at/boothdr.S
+++ b/i386/i386at/boothdr.S
@@ -1,6 +1,6 @@

 #include <mach/machine/asm.h>
-
+#include <i386/apic.h>
 #include <i386/i386asm.h>

        /*
@@ -54,7 +54,18 @@ boot_entry:
        movw    %ax,%ss

        /* Switch to our own interrupt stack.  */
-       movl    $_intstack+INTSTACK_SIZE,%esp
+       movl    $solid_intstack+INTSTACK_SIZE-4, %esp
+       andl    $0xfffffff0,%esp
+
+       /* Enable local apic */
+       xorl    %eax, %eax
+       xorl    %edx, %edx
+       movl    $APIC_MSR, %ecx
+       rdmsr
+       orl     $APIC_MSR_ENABLE, %eax
+       orl     $APIC_MSR_BSP, %eax
+       movl    $APIC_MSR, %ecx
+       wrmsr

        /* Reset EFLAGS to a known state.  */
        pushl   $0
@@ -91,9 +102,6 @@ iplt_done:
        /* Jump into C code.  */
        call    EXT(c_boot_entry)

-       .comm   _intstack,INTSTACK_SIZE
-       .comm   _eintstack,0
-
 .align 16
        .word 0
 boot_gdt_descr:
diff --git a/i386/i386at/ioapic.c b/i386/i386at/ioapic.c
index 003690ed..f7b0d1d3 100644
--- a/i386/i386at/ioapic.c
+++ b/i386/i386at/ioapic.c
@@ -186,9 +186,8 @@ lapic_enable_timer(void)
     /* Some buggy hardware requires this set again */
     lapic->divider_config.r = LAPIC_TIMER_DIVIDE_16;

-    /* Enable interrupts for the first time on BSP */
-    asm("sti");
-    printf("LAPIC timer configured\n");
+    /* Enable interrupts for the first time */
+    printf("LAPIC timer configured on cpu%d\n", cpu_number());
 }

 void
diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
index 98408c06..e0f669b6 100644
--- a/i386/i386at/model_dep.c
+++ b/i386/i386at/model_dep.c
@@ -134,11 +134,9 @@ extern char        version[];
 /* If set, reboot the system on ctrl-alt-delete.  */
 boolean_t      rebootflag = FALSE;     /* exported to kdintr */

-/* Interrupt stack.  */
-static char int_stack[INTSTACK_SIZE] __aligned(INTSTACK_SIZE);
-#if NCPUS <= 1
-vm_offset_t int_stack_top[1], int_stack_base[1];
-#endif
+/* Interrupt stacks  */
+extern vm_offset_t int_stack_top[], int_stack_base[];
+extern uint8_t solid_intstack[];    /* bottom */

 #ifdef LINUX_DEV
 extern void linux_init(void);
@@ -171,15 +169,16 @@ void machine_init(void)
        hyp_init();
 #else  /* MACH_HYP */

-#if defined(APIC)
-       if (acpi_apic_init() != ACPI_SUCCESS) {
-               panic("APIC not found, unable to boot");
-       }
-       ioapic_configure();
-       lapic_enable_timer();
 #if (NCPUS > 1)
+       acpi_apic_init();
        smp_init();
+#endif
+#if defined(APIC)
+       ioapic_configure();
+#endif
+       startrtclock();

+#if defined(APIC)
 #warning FIXME: Rather unmask them from their respective drivers
        /* kd */
        unmask_irq(1);
@@ -187,8 +186,7 @@ void machine_init(void)
        unmask_irq(4);
        /* com1 */
        unmask_irq(3);
-#endif /* NCPUS > 1 */
-#endif /* APIC */
+#endif

 #ifdef LINUX_DEV
        /*
@@ -483,8 +481,7 @@ i386at_init(void)
        hyp_p2m_init();
 #endif /* MACH_XEN */

-       int_stack_base[0] = (vm_offset_t)&int_stack;
-       int_stack_top[0] = int_stack_base[0] + INTSTACK_SIZE - 4;
+       interrupt_stack_alloc();
 }

 /*
@@ -576,7 +573,6 @@ void c_boot_entry(vm_offset_t bi)
 #endif /* MACH_KDB */

        machine_slot[0].is_cpu = TRUE;
-       machine_slot[0].running = TRUE;
        machine_slot[0].cpu_subtype = CPU_SUBTYPE_AT386;

        switch (cpu_type)
@@ -622,8 +618,11 @@ timemmap(dev_t dev, vm_offset_t off, vm_prot_t prot)
 void
 startrtclock(void)
 {
-#ifndef APIC
+#ifdef APIC
+       lapic_enable_timer();
+#else
        clkstart();
+       unmask_irq(0);
 #endif
 }

--
2.34.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]