> Again, we need a memory barrier, to prevent the compiler from emitting
these writes in the wrong order.
Really, these writes are using a temporary struct. There are a only write
to the real register, which is done assigning the temporary struct to the
real register through the HAL
El mar., 15 nov. 2022 1:40, Samuel Thibault <samuel.thiba...@gnu.org>
escribió:
> Damien Zammit, le ven. 11 nov. 2022 23:21:22 +0000, a ecrit:
> > diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> > index d30084e2..6a5fa754 100644
> > --- a/i386/i386/apic.c
> > +++ b/i386/i386/apic.c
> > @@ -19,6 +19,8 @@
> > Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111,
> USA. */
> >
> > #include <i386/apic.h>
> > +#include <i386/cpu.h>
> > +#include <i386at/idt.h>
> > #include <string.h>
> > #include <vm/vm_kern.h>
> > #include <kern/printf.h>
> > @@ -116,11 +118,27 @@ uint16_t
> > apic_get_cpu_apic_id(int kernel_id)
> > {
> > if (kernel_id >= NCPUS)
> > - return -1;
> > + return 0;
>
> As already mentioned:
>
> Better make it return an int16_t to be able to return -1? Otherwise I
> guess we can confuse that 0 with acpi_id 0?
>
> >
> > return apic_data.cpu_lapic_list[kernel_id];
> > }
> >
> > +
> > +/*
> > + * apic_get_cpu_kernel_id: returns the kernel_id of a cpu.
> > + * Receives as input the APIC ID of a CPU.
> > + */
> > +int
> > +apic_get_cpu_kernel_id(uint16_t apic_id)
> > +{
> > + int i = 0;
> > +
> > + while(apic_data.cpu_lapic_list[i] != apic_id && i < apic_data.ncpus)
> > + i++;
>
> Rather just use
>
> for (i = 0; i < apic_data.ncpus; i++)
> if (apic_data.cpu_lapic_list[i] == apic_id)
> return i;
>
> which is much more canonical and thus more maintainable.
>
> > @@ -235,6 +249,61 @@ void apic_print_info(void)
> > }
> > }
> >
> > +void apic_send_ipi(unsigned dest_shorthand, unsigned deliv_mode,
> unsigned dest_mode, unsigned level, unsigned trig_mode, unsigned vector,
> unsigned dest_id)
> > +{
> > + IcrLReg icrl_values;
> > + IcrHReg icrh_values;
> > +
> > + icrl_values.destination_shorthand = dest_shorthand;
> > + icrl_values.delivery_mode = deliv_mode;
> > + icrl_values.destination_mode = dest_mode;
> > + icrl_values.level = level;
> > + icrl_values.trigger_mode = trig_mode;
> > + icrl_values.vector = vector;
> > + icrh_values.destination_field = dest_id;
> > +
> > + lapic->icr_high = icrh_values;
> > + lapic->icr_low = icrl_values;
>
> Again, we need a memory barrier, to prevent the compiler from emitting
> these writes in the wrong order.
>
> > --- a/i386/i386at/ioapic.c
> > +++ b/i386/i386at/ioapic.c
> > - splon(s);
> > + /* Enable interrupts for the first time on BSP */
> > + asm("sti");
>
> cpu_intr_enable is available for this.
>
> Samuel
>
>
