Re: [PATCH v2 1/2] xen/pdx: account for frametable_base_pdx in generic pdx_to_page/page_to_pdx

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 05, 2026 at 08:48:15AM +0200, Orzel, Michal wrote:
> 
> 
> On 04-May-26 17:28, Roger Pau Monné wrote:
> > On Thu, Apr 30, 2026 at 02:51:02PM +0200, Michal Orzel wrote:
> >> The generic pdx_to_page() and page_to_pdx() macros in xen/pdx.h assume
> >> the frame table starts at PDX 0, which is only true on x86. ARM
> >> uses a non-zero frametable_base_pdx to offset into the frame table (PPC 
> >> also
> >> defines it).
> >>
> >> Fix the generic macros to subtract/add frametable_base_pdx, defaulting
> >> to 0 when the arch does not define it. This makes the generic macros
> >> correct for all architectures, even though they are only used on x86
> >> today.
> > 
> > Hm, I assume this offset was added because the original mask PDX
> > compression won't (usually) compress the gap between 0 and the start
> > of RAM.  However the newish offset PDX compression should be able to
> > compress from 0 to start of RAM, and hence you don't need to apply
> > an extra PDX offset there?
> > 
> > If that's indeed the case it might be better to integrate
> > frametable_base_pdx into the mask compression algorithm itself, so
> > that on some arches it's a mask plus a decrease.
> The offset is needed regardless of whether compression is used. With
> CONFIG_PDX_NONE (no compression, PDX == MFN), if RAM starts at e.g.
> 0x80000000, the first valid PDX is 0x80000.

OK, so you are doing some (kind of) address space compression (removing
the leading empty range to the first RAM region) even when PDX is
disabled.

> Without frametable_base_pdx
> the frame table would have to be indexed from 0, wasting
> 0x80000 * sizeof(page_info) of memory just to cover the hole before RAM.

But you don't really "waste" memory, just address space?  Oh, maybe
not on ARM as it doesn't use pdx_group_valid?  And so you
unconditionally populate the frametable from PDX 0 to max PDX.

> So frametable_base_pdx is really a frame table indexing offset, not
> something tied to the compression algorithm.

Right, it just seems odd to do that extra subtraction when using
offset compression, as in that case the compression logic itself
should remove that leading gap when RAM doesn't start at 0.

Instead of generalizing and expanding the usage of frametable_base_pdx
it might be better to implement support for pdx_group_valid when
populating the frame table, and switch by default to the offset
compression method that will already remove any leading unpopulated
spaces?

Thanks, Roger.