Re: sys/powerpc64: encode PTCR PATS correctly

On Mon, 13 Jul 2026 14:28:53 +0200,
Mark Kettenis <[email protected]> wrote:
> 
> > Date: Mon, 13 Jul 2026 13:46:49 +0200
> > From: Kirill A. Korinsky <[email protected]>
> > 
> > On Wed, 08 Jul 2026 00:35:32 +0200,
> > Kirill A. Korinsky <[email protected]> wrote:
> > > 
> > > Mark, George,
> > > 
> > > Shivang Upadhyay in qemu-devel
> > > https://marc.info/?l=qemu-devel&m=178332702820587&w=2 pointed that we may
> > > have a typo kind of bug in powerpc64's pmap.
> > > 
> > > The PTCR PATS field stores log2(partition table size) - 12.  Since
> > > PATMEMSZ is a power of two, ffs(PATMEMSZ) is log2(PATMEMSZ) + 1; the
> > > old expression wrote 5 for the 64 KiB table, describing 128 KiB to
> > > Book3S v3 hardware.
> > > 
> > > Use fls(PATMEMSZ) - 1 - 12 to express the intended log2 calculation
> > > and encode the 64 KiB partition table as PATS == 4.
> > > 
> > > Ok?
> > > 
> > 
> > Ping?
> 
> I'd still like to see this tested on real hardware.  Seems George is
> alive, so hopefully he'll get around to doing this soonish.
> 
> One minor nit about the proposed diff...
> 
> > Index: sys/arch/powerpc64/powerpc64/pmap.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/arch/powerpc64/powerpc64/pmap.c,v
> > diff -u -p -r1.66 pmap.c
> > --- sys/arch/powerpc64/powerpc64/pmap.c	21 Aug 2025 00:10:21 -0000	1.66
> > +++ sys/arch/powerpc64/powerpc64/pmap.c	7 Jul 2026 20:20:55 -0000
> > @@ -124,7 +124,7 @@ uint64_t pmap_ptab_mask;
> >  struct pate *pmap_pat;
> >  
> >  #define PATMEMSZ	(64 * 1024)
> > -#define PATSIZE		(ffs(PATMEMSZ) - 12)
> > +#define PATSIZE		(fls(PATMEMSZ) - 1 - 12)
> 
> I think I'd prefer to keep using ffs().  As long as the size is a
> power of two there should be no issue.  But if it isn't, ffs() would
> pass a smaller size to the hardware instead of a larger size, which
> seems safer.  I also would use an extra set of parentheses to express
> that the "- 1" is part of the log2 calculation.  So...
> 
> #define PATSIZE		((ffs(PATMEMSZ) - 1) - 12)
> 
> 

Make sense, here an updated trivial diff which I do have locally now:

Index: sys/arch/powerpc64/powerpc64/pmap.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/pmap.c,v
diff -u -p -r1.66 pmap.c
--- sys/arch/powerpc64/powerpc64/pmap.c	21 Aug 2025 00:10:21 -0000	1.66
+++ sys/arch/powerpc64/powerpc64/pmap.c	13 Jul 2026 13:10:39 -0000
@@ -124,7 +124,7 @@ uint64_t pmap_ptab_mask;
 struct pate *pmap_pat;
 
 #define PATMEMSZ	(64 * 1024)
-#define PATSIZE		(ffs(PATMEMSZ) - 12)
+#define PATSIZE		(ffs(PATMEMSZ) - 1 - 12)
 
 struct pte_desc {
 	/* Linked list of phys -> virt entries */


-- 
wbr, Kirill