Re: CVS commit: src/sys/arch/amd64/amd64

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

Le 07/05/2016 23:13, matthew green a écrit :
> Joerg Sonnenberger writes:
> > On Sat, May 07, 2016 at 11:49:21AM +0000, Maxime Villard wrote:
> > > Module Name:	src
> > > Committed By:	maxv
> > > Date:		Sat May  7 11:49:21 UTC 2016
> > >
> > > Modified Files:
> > > 	src/sys/arch/amd64/amd64: locore.S
> > >
> > > Log Message:
> > > clarify
> >
> > WTH. Can you please not mix arbitrary stylistic changes with refactoring
> > and whatever else you have hidden in this?!

I don't like the "arbitrary". I wrote this months ago, and the patch I have
for this file entails many more actual functional changes. I just committed
the stylistic and idiotic parts yesterday, because I was busy doing something
else. The other real changes will come separately soon.

> agreed.  there is at least one functional change here:  PROC0_STK_OFF
> has changed definition.  could you please explain this part?

It's rather simple:

-#define PROC0_STK_OFF	(PROC0_PML4_OFF + PAGE_SIZE)
+#define PROC0_STK_OFF	(PROC0_PML4_OFF + 1 * PAGE_SIZE)
  #define PROC0_PTP3_OFF	(PROC0_STK_OFF + UPAGES * PAGE_SIZE)
  #define PROC0_PTP2_OFF	(PROC0_PTP3_OFF + NKL4_KIMG_ENTRIES * PAGE_SIZE)
  #define PROC0_PTP1_OFF	(PROC0_PTP2_OFF + TABLE_L3_ENTRIES * PAGE_SIZE)

All the macros are in the format NUMBER_OF_PAGES * PAGE_SIZE, so I put 1*
to make clear we are allocating one page.

> additionally, please revert killkpt macro -- it makes it harder to
> understand the assembly as it moves the 1: target into a macro so
> that people will mis-reaad branch/jumps thinking they'll go to the
> following 1:.

No. You can see above that there is the fillkpt macro, that is in charge
of setting up a set of pages. We now have a pair fillkpt/killkpt, which
is way clearer than hard-coded loops.

fillkpt too uses 1: loop 1b as well, and there is no problem with it.

I see by the way that I could have used killkpt for the PML4 entries as
well; I'll commit that right now.

> thanks.
>
>
> .mrg.