Re: sys/powerpc64: encode PTCR PATS correctly
Mark Kettenis <[email protected]> Mon, 13 Jul 2026 14:28:53 +0200
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
> 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) > > struct pte_desc { > /* Linked list of phys -> virt entries */ > > > -- > wbr, Kirill > >