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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.05.2026 09:35, Orzel, Michal wrote:
> On 05-May-26 09:13, Roger Pau Monné wrote:
>> 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.
> With pdx_group_valid (which this series adds) we wouldn't waste
> physical memory for the leading gap. But we'd still waste virtual address
> space and the FRAMETABLE_NR check (max_pdx > FRAMETABLE_NR) becomes tighter
> because the full range from PDX 0 must fit. For example with RAM starting at 
> 5TB
> the virtual offset before the first usable entry would be ~70GB — more than 
> the
> entire 32GB FRAMETABLE_SIZE on ARM64.

Yet still - this is exactly one of the situations offset compression means
to cover. I'm entirely with Roger as to it being undesirable to build a
special case variant of "offset compression" into "no compression".

Jan