Hello,
We're converging :)
Samuel
Damien Zammit, le ven. 11 nov. 2022 23:21:15 +0000, a ecrit:
> -void idt_init(void)
> +static void
> +idt_fill(struct real_gate *myidt)
> {
> #ifdef MACH_PV_DESCRIPTORS
> if (hyp_set_trap_table(kvtolin(idt_inittab)))
> @@ -47,18 +49,29 @@ void idt_init(void)
> /* Initialize the exception vectors from the idt_inittab. */
> while (iie->entrypoint)
> {
> - fill_idt_gate(iie->vector, iie->entrypoint, KERNEL_CS,
> iie->type, 0);
> + fill_idt_gate(myidt, iie->vector, iie->entrypoint, KERNEL_CS,
> iie->type, 0);
> iie++;
> }
> +#endif /* MACH_PV_DESCRIPTORS */
? No, keep #endif where it was. In the PV case there is just one idt,
set with hyp_set_trap_table, and we don't use lidt.
>
> - /* Load the IDT pointer into the processor. */
> + /* Load the IDT pointer into the processor */
Leave as it is: English wants a period, and says there are two spaces
after the period :)
> diff --git a/i386/i386/ktss.c b/i386/i386/ktss.c
> index 0d21d3eb..5cc260fc 100644
> --- a/i386/i386/ktss.c
> +++ b/i386/i386/ktss.c
> @@ -35,12 +35,13 @@
> #include "seg.h"
> #include "gdt.h"
> #include "ktss.h"
> +#include "mp_desc.h"
>
> /* A kernel TSS with a complete I/O bitmap. */
> struct task_tss ktss;
>
> void
> -ktss_init(void)
> +ktss_fill(struct task_tss *myktss, struct real_descriptor *mygdt)
> {
> /* XXX temporary exception stack */
> static int exception_stack[1024];
Mmm, will that not be a problem to use the same exception stack for all
processors? I guess a [NCPUS] could be needed here.
> /* Initialize the master TSS. */
> #ifdef __x86_64__
> - ktss.tss.rsp0 = (unsigned long)(exception_stack+1024);
> + myktss->tss.rsp0 = (unsigned long)(exception_stack+1024);
> #else /* ! __x86_64__ */
> - ktss.tss.ss0 = KERNEL_DS;
> - ktss.tss.esp0 = (unsigned long)(exception_stack+1024);
> + myktss->tss.ss0 = KERNEL_DS;
> + myktss->tss.esp0 = (unsigned long)(exception_stack+1024);
> #endif /* __x86_64__ */
>
> diff --git a/i386/i386at/int_init.c b/i386/i386at/int_init.c
> index 6da627dd..da552106 100644
> --- a/i386/i386at/int_init.c
> +++ b/i386/i386at/int_init.c
> @@ -22,29 +22,48 @@
> + fill_idt_gate(myidt, CALL_SINGLE_FUNCTION_BASE,
> int_entry_table[i], KERNEL_CS,
> ACC_PL_K|ACC_INTR_GATE, 0);
Use i++, so that
> +#ifdef APIC
> + fill_idt_gate(myidt, IOAPIC_SPURIOUS_BASE,
> + int_entry_table[25], KERNEL_CS,
> ACC_PL_K|ACC_INTR_GATE, 0);
> #endif
Here you won't hardcode a magic 25.
> diff --git a/i386/i386at/interrupt.S b/i386/i386at/interrupt.S
> index 7480fba9..9eccf022 100644
> --- a/i386/i386at/interrupt.S
> +++ b/i386/i386at/interrupt.S
> @@ -41,6 +41,9 @@ ENTRY(interrupt)
> cmpl $255,%eax /* was this a spurious intr? */
> je _no_eoi /* if so, just return */
> #endif
> + cmpl $251,%eax /* was this a SMP call single function
> request? */
Use CALL_SINGLE_FUNCTION_BASE here as well.
> + je _call_single
> +
> subl $28,%esp /* Two local variables + 5 parameters */
> movl %eax,S_IRQ /* save irq number */
> call spl7 /* set ipl */
> @@ -118,4 +121,17 @@ _isa_eoi:
> addl $28,%esp /* pop local variables */
> _no_eoi:
> ret
Add a newline here, to separate this new path.
> +_call_single:
> + subl $8,%esp /* One param */
> + movl %eax,S_ARG0 /* save irq number */
No, S_ARG0 is valid only inside the callee. #define some new S2_IRQ and
S2_IPL macros to 0(%esp) and 4(%esp).
> + call spl7 /* set ipl */
> + movl %eax,S_ARG1 /* save previous ipl */
> + call EXT(pmap_update_interrupt) /* TODO: Allow other functions */
> + movl S_ARG1,%eax /* restore previous ipl */
> + call splx_cli /* restore previous ipl */
> + cli
> + movl S_ARG0,%eax /* restore irq number */
The irq number doesn't seem to be used?
> + call EXT(lapic_eoi) /* lapic EOI */
> + addl $8,%esp
> + ret
> END(interrupt)
While at it, it would be useful to update ./x86_64/interrupt.S and
x86_64/locore.S too, so they remain coherent.
Samuel
