Re: more apic cleanups, hammer patch III

[John L. <le...@mo...>]

On Wed, Nov 13, 2002 at 12:11:54PM +0000, Philippe Elie wrote:

> >+#define SET_BITS(dest,val,mask) ((dest) = ((dest) & ~(mask)) | ((val) & 
> >(mask)))
> >+
> 
> <troll>John ?</troll>

I'm nervous too ...

> >+static inline void _set_gate(struct gate_struct * adr, void * fnptr)
> >+{
> >+	unsigned long fn = (unsigned long)fnptr;
> >+
> >+#ifndef CONFIG_X86_64
> >+	SET_GATE_DPL(adr, 0);
> >+#else /* CONFIG_X86_64 */
> >+	SET_GATE_DPL(adr, 3);
> >+	SET_GATE_TARGET_HIGH(adr, fn);
> >+	SET_GATE_CONSTANT_BITS(adr);
> >+#endif /* CONFIG_X86_64 */
> >+
> >+	SET_GATE_TARGET_LOW(adr, fn);
> >+	SET_GATE_TARGET_MID(adr, fn);
> >+	SET_GATE_ZERO1(adr, 0);
> >+	SET_GATE_S(adr, 0);
> >+	SET_GATE_TYPE(adr, NMI_GATE_TYPE);
> >+	SET_GATE_P(adr, 1);
> >+	SET_GATE_SELECTOR(adr, __KERNEL_CS);
> 
> this code is racy, we can't nothing about the race but we
> would try to minimize the windows, the old version used a

Yes. I haven't checked but it might well be the case that the asm
set_gate left the IDT entry consistently meaningful at all points. Even
if not, the window is way smaller.

> temporary to build the entry then copied it in the idt.I
> guess the temporary and the struct gate_struct * should be
> volatile to avoid gcc building the entry directly through
> adr.

Yep, we definitely need a temporary then copy it over.

> consider it as approved with these changes but I prefer you
> wait for John approval too: it's a big change.

I think it's OK with the previous comments fixed

regards
john


-- 
Khendon's Law: If the same point is made twice by the same person,
the thread is over.