Hello,
Damien Zammit, le lun. 05 févr. 2024 11:34:09 +0000, a ecrit:
> Outstanding: Find a way to allocate memory below 1MiB.
We could make the biosmem's biosmem_bootstrap_common code reserve the
first physical page it finds from the multiboot mem map.
> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
> index 071aa292..6a040ab8 100644
> --- a/i386/i386/mp_desc.c
> +++ b/i386/i386/mp_desc.c
> @@ -292,17 +292,24 @@ cpu_ap_main()
> kern_return_t
> cpu_start(int cpu)
> {
> + int err, i, vec;
> +
> assert(machine_slot[cpu].running != TRUE);
>
> uint16_t apic_id = apic_get_cpu_apic_id(cpu);
>
> - printf("Trying to enable: %d\n", apic_id);
> + vec = apboot_addr >> 12;
>
> - smp_startup_cpu(apic_id, apboot_addr);
> + printf("Trying to enable: %d at 0x%x\n", apic_id, vec);
>
> - printf("Started cpu %d (lapic id %04x)\n", cpu, apic_id);
> + err = smp_startup_cpu(apic_id, vec);
I'd say rather keep the >>12 part inside smp_startup_cpu, like it was.
> - return KERN_SUCCESS;
> + if (!err) {
> + printf("Started cpu %d (lapic id %04x)\n", cpu, apic_id);
> + return KERN_SUCCESS;
> + }
> + printf("FATAL: Cannot init AP %d\n", cpu);
> + for (;;);
> }
>
> void
> diff --git a/i386/i386/smp.c b/i386/i386/smp.c
> index 87f59913..397defa6 100644
> --- a/i386/i386/smp.c
> +++ b/i386/i386/smp.c
> @@ -18,10 +18,14 @@
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA. */
>
> +#include <string.h>
> #include <i386/apic.h>
> #include <i386/smp.h>
> #include <i386/cpu.h>
> +#include <i386/pio.h>
> +#include <i386/vm_param.h>
> #include <i386at/idt.h>
> +#include <i386at/cram.h>
> #include <i386at/acpi_parse_apic.h>
> #include <kern/printf.h>
> #include <mach/machine.h>
> @@ -75,59 +79,105 @@ void smp_pmap_update(unsigned apic_id)
> smp_send_ipi(apic_id, CALL_PMAP_UPDATE);
> }
>
> -/* See Intel IA32/64 Software Developer's Manual 3A Section 8.4.4.1 */
> -void smp_startup_cpu(unsigned apic_id, unsigned vector)
> +static void
> +wait_for_ipi(void)
> {
> - /* Clear APIC errors */
> - lapic->error_status.r = 0;
> + int loops = 100000;
This is arbitrary and can arbitrarily break if processor conditions
change. Does the manual specify a reactivity bound time? Can we not use
the hpet counter as a clock source?
> + while (lapic->icr_low.delivery_status == SEND_PENDING) {
> + cpu_pause();
> + loops--;
> + if (loops == 0) {
> + printf("FATAL: wait_for_ipi: busy\n");
> + for (;;);
> + }
> + }
> +}
> +
> +static int
> +smp_send_ipi_init(int apic_id)
> +{
> + int err;
>
> - printf("Sending IPIs to APIC ID %u...", apic_id);
> + lapic->error_status.r = 0;
>
> /* Assert INIT IPI */
> - apic_send_ipi(NO_SHORTHAND, INIT, PHYSICAL, ASSERT, LEVEL, 0, apic_id);
> + apic_send_ipi(NO_SHORTHAND, INIT, PHYSICAL, ASSERT, EDGE, 0, apic_id);
This is an involved change, can we have a reference on why in the end it
should be edge?
> /* Wait for delivery */
> - do {
> - cpu_pause();
> - } while(lapic->icr_low.delivery_status == SEND_PENDING);
> + wait_for_ipi();
> + hpet_mdelay(10);
>
> /* Deassert INIT IPI */
> - apic_send_ipi(NO_SHORTHAND, INIT, PHYSICAL, DE_ASSERT, LEVEL, 0,
> apic_id);
> + apic_send_ipi(NO_SHORTHAND, INIT, PHYSICAL, DE_ASSERT, EDGE, 0, apic_id);
Ditto.
> /* Wait for delivery */
> - do {
> - cpu_pause();
> - } while(lapic->icr_low.delivery_status == SEND_PENDING);
> + wait_for_ipi();
>
> - /* Wait 10 msec */
> - hpet_mdelay(10);
> + err = lapic->error_status.r;
> + if (err) {
> + printf("ESR error upon INIT 0x%x\n", err);
> + }
> + return 0;
> +}
>
> - /* Clear APIC errors */
> - lapic->error_status.r = 0;
> +static int
> +smp_send_ipi_startup(int apic_id, int vector)
> +{
> + int err;
>
> - /* First StartUp IPI */
> - apic_send_ipi(NO_SHORTHAND, STARTUP, PHYSICAL, ASSERT, LEVEL, vector >>
> 12, apic_id);
> + lapic->error_status.r = 0;
>
> - /* Wait 200 usec */
> - hpet_udelay(200);
> + /* StartUp IPI */
> + apic_send_ipi(NO_SHORTHAND, STARTUP, PHYSICAL, ASSERT, EDGE, vector,
> apic_id);
Ditto.
> /* Wait for delivery */
> - do {
> - cpu_pause();
> - } while(lapic->icr_low.delivery_status == SEND_PENDING);
> + wait_for_ipi();
>
> - /* Second StartUp IPI */
> - apic_send_ipi(NO_SHORTHAND, STARTUP, PHYSICAL, ASSERT, LEVEL, vector >>
> 12, apic_id);
> + err = lapic->error_status.r;
> + if (err) {
> + printf("ESR error upon STARTUP 0x%x\n", err);
> + }
> + return 0;
> +}
>
> - /* Wait 200 usec */
> +/* See Intel IA32/64 Software Developer's Manual 3A Section 8.4.4.1 */
> +int smp_startup_cpu(unsigned apic_id, unsigned vector)
> +{
> + int err;
> +
> +#if 0
> + /* This is a legacy method of INIT that only works with
> + * old hardware that does not support SIPIs.
> + * Must use INIT DEASSERT LEVEL triggered IPI to use this block.
> + * (At least one AMD FCH does not support this IPI mode,
> + * See AMD BKDG FAM16h document # 48751 page 461).
> + */
> +
> + /* Tell CMOS to warm reset through through 40:67 */
> + outb(CMOS_ADDR, CMOS_SHUTDOWN);
> + outb(CMOS_DATA, CM_JMP_467);
> +
> + /* Set warm reset vector to point to AP startup code */
> + uint16_t dword[2];
> + dword[0] = 0;
> + dword[1] = vector << 8;
Rather use 12-4, otherwise 8 is surprising.
> + memcpy((uint8_t *)phystokv(0x467), dword, 4);
> +#endif
> +
> + /* Local cache flush */
> + asm("wbinvd":::"memory");
> +
> + printf("Sending IPIs to APIC ID %u...\n", apic_id);
> + err = smp_send_ipi_init(apic_id);
> + hpet_mdelay(10);
> + err = smp_send_ipi_startup(apic_id, vector);
> + hpet_udelay(200);
> + err = smp_send_ipi_startup(apic_id, vector);
> hpet_udelay(200);
> -
> - /* Wait for delivery */
> - do {
> - cpu_pause();
> - } while(lapic->icr_low.delivery_status == SEND_PENDING);
>
> printf("done\n");
> + return 0;
> }
>
> /*
