Ok, I understand. I agree with split apic_init and smp init. But I prefer that the apic init calls be in a separated function, instead directly in machine_init()
El sáb, 24 sept 2022 a las 20:14, Etienne Brateau (< [email protected]>) escribió: > APIC is also useful for MSI/MSI-X (for PCI). So you might want to have > APIC without SMP. > > Le sam. 24 sept. 2022 à 19:39, Almudena Garcia <[email protected]> > a écrit : > >> At first question: why do you want to enable APIC without SMP. >> In my known, APIC is only useful when there are multiple processors. Even >> IOAPIC has the purpose of distribute IO inputs to multiple processors. >> >> +++ b/i386/i386at/model_dep.c >> @@ -66,6 +66,7 @@ >> #include <i386/locore.h> >> #include <i386/model_dep.h> >> #include <i386/smp.h> >> +#include <i386at/acpi_parse_apic.h> >> #include <i386at/autoconf.h> >> #include <i386at/biosmem.h> >> #include <i386at/elf.h> >> @@ -170,10 +171,14 @@ void machine_init(void) >> hyp_init(); >> #else /* MACH_HYP */ >> >> -#if (NCPUS > 1) && defined(APIC) >> - smp_init(); >> +#if defined(APIC) >> + if (acpi_apic_init() != ACPI_SUCCESS) { >> + panic("APIC not found, unable to boot"); >> + } >> ioapic_configure(); >> lapic_enable_timer(); >> +#if (NCPUS > 1) >> + smp_init(); >> >> #warning FIXME: Rather unmask them from their respective drivers >> /* kd */ >> @@ -183,6 +188,7 @@ void machine_init(void) >> /* com1 */ >> unmask_irq(3); >> #endif /* NCPUS > 1 */ >> +#endif /* APIC */ >> >> #ifdef LINUX_DEV >> >> I don't like put this code in this function. I don't like extremely long >> functions, so I prefer put this in its own function, instead directly in >> machine_init(). >> >> >> El sáb, 24 sept 2022 a las 18:33, Etienne Brateau (< >> [email protected]>) escribió: >> >>> When we want to enable APIC, we must initialize the structure or >>> otherwise, it will try to access to a not initialized memory addresses. >>> --- >>> i386/i386/smp.c | 10 ++-------- >>> i386/i386at/model_dep.c | 10 ++++++++-- >>> 2 files changed, 10 insertions(+), 10 deletions(-) >>> >>> diff --git a/i386/i386/smp.c b/i386/i386/smp.c >>> index f64fb7f7..d7523a73 100644 >>> --- a/i386/i386/smp.c >>> +++ b/i386/i386/smp.c >>> @@ -20,7 +20,6 @@ >>> >>> #include <i386/i386/apic.h> >>> #include <i386/i386/smp.h> >>> -#include <i386/i386at/acpi_parse_apic.h> >>> >>> #include <kern/smp.h> >>> >>> @@ -42,12 +41,7 @@ static void smp_data_init(void) >>> */ >>> int smp_init(void) >>> { >>> - int apic_success; >>> + smp_data_init(); >>> >>> - apic_success = acpi_apic_init(); >>> - if (apic_success == ACPI_SUCCESS) { >>> - smp_data_init(); >>> - } >>> - >>> - return apic_success; >>> + return 0; >>> } >>> diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c >>> index 105aedb2..1819526b 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 <i386at/acpi_parse_apic.h> >>> #include <i386at/autoconf.h> >>> #include <i386at/biosmem.h> >>> #include <i386at/elf.h> >>> @@ -170,10 +171,14 @@ void machine_init(void) >>> hyp_init(); >>> #else /* MACH_HYP */ >>> >>> -#if (NCPUS > 1) && defined(APIC) >>> - smp_init(); >>> +#if defined(APIC) >>> + if (acpi_apic_init() != ACPI_SUCCESS) { >>> + panic("APIC not found, unable to boot"); >>> + } >>> ioapic_configure(); >>> lapic_enable_timer(); >>> +#if (NCPUS > 1) >>> + smp_init(); >>> >>> #warning FIXME: Rather unmask them from their respective drivers >>> /* kd */ >>> @@ -183,6 +188,7 @@ void machine_init(void) >>> /* com1 */ >>> unmask_irq(3); >>> #endif /* NCPUS > 1 */ >>> +#endif /* APIC */ >>> >>> #ifdef LINUX_DEV >>> /* >>> -- >>> 2.37.3 >>> >>> >>>
