Re: x86: move the LAPIC va

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 20/10/2016 à 18:58, Joerg Sonnenberger a écrit :
> On Thu, Oct 20, 2016 at 06:11:59PM +0200, Maxime Villard wrote:
> > Le 16/10/2016 à 16:32, Joerg Sonnenberger a écrit :
> > > On Sun, Oct 16, 2016 at 01:52:18PM +0200, Maxime Villard wrote:
> > > > Yes, I thought about that. But how do you access the fields from assembly?
> > > > At the end of the day, you still need offsets, and it was better to have
> > > > different pointers.
> > >
> > > Just like fields in the trap frame for example are accessed. Put them in
> > > genassym.cf and use the created offset in the actual store. It doesn't
> > > even add another command.
> >
> > That's not what I meant, but it doesn't matter anymore.
>
> This would read a lot less confrontational if you included that it
> doesn't matter because the patch style already changed to that.

? I thought it was clear I had agreed with you on the design, and I thought
my commits were enlightening enough in this regard. So I didn't feel like it
was worth screwing around more, hence my short answer.

You will notice the "it WAS better".

> > > > I've removed lapic_tpr, which means there is only one pointer to the LAPIC
> > > > page and each register access is done with offsets. I have reupdated and
> > > > resimplified my patch. The only thing we do now is allocating one page of
> > > > va at boot time, and add a level of indirection in the asm code.
> > >
> > > The access via read/write reg is effectively the same as above. I'm not
> > > a fan of the inline functions as they are due to the multiple layers of
> > > casts...
> >
> > What I'm personally not fan of is the name of these functions. But whatever.
> > I guess I'll commit this patch in a week or two, unless someone disagrees.
>
> Well, the third chunk in i386/vector.S is broken. You are changing %ebx
> right before iret. To avoid other issues, the fourth chunk should just be
> using %ebx too as that's obviously rewritten just afterwards.

Mmh, I see what you mean. But after investigating it in more details, it
seems to me there is already a problem in intrddbipi: the GP registers are
not pushed on the stack, and not poped on return. I may be reading it wrong,
but it seems to me there should be a pair of INTRENTRY and INTRFASTEXIT,
with the appropriate trapno and errno pushed according to our stackframe
struct definition.

Anyway, thanks for pointing this out; I've reupdated the patch and just
added a pair of push and pop around %ebx as a workaround. Also I've changed
the fourth chunk as you said, with the amd64 version along the way.