YAMAMOTO Takashi wrote:
> hi,
> 
> (it's the same as a mail which seems to be blocked by mailinglist
> about two weeks ago.)
> 
> the following diff is to make i386 pmap similar to amd64.
> is it ok to commit?
> 
> ftp://ftp.netbsd.org/pub/NetBSD/misc/yamt/i386pmap.syncwithamd64.diff

I'd say I don't like most of it for the following reasons:

- license complication for no actual benefit.
- Most of the patch is just to replace "PD" and "PT" with "L2" and "L1",
  respectively. I'd say "if it ain't broken, don't fix it." for this case.
  Personally, "PD" and "PT" are easier to read, or at least, less confusing.
- why is a single leal instruction replaced with two movl/addl
  instructions? There are a couple more places with similar changes.
   2657 -       /* Calculate end of text segment, rounded to a page. */
   2658 -       leal    (RELOC(etext)+PGOFSET),%edx
   2659 +       /*
   2660 +        * Compute etext - KERNBASE. This can't be > 4G, or we can't deal
   2661 +        * with it anyway, since we can't load it in 32 bit mode. So use
   2662 +        * the bottom 32 bits.
   2663 +        */
   2664 +       movl    $RELOC(etext),%edx
   2665 +       addl    $PGOFSET,%edx
   2666         andl    $~PGOFSET,%edx
- have no idea why '1ULL' is replace '1' here. '1' is perfectly fine
  with the 32-bit world, AFAIK.
    112 +#define        NBPD_L1         (1ULL << L1_SHIFT) /* # bytes mapped by L1 ent (4K) */
    113 +#define        NBPD_L2         (1ULL << L2_SHIFT) /* # bytes mapped by L2 ent (4MB) */

Jun-Young