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

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 05-May-26 12:40, Jan Beulich wrote:
> 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".
In this case, if you don't want to generalize the macros, how should we proceed
on Arm if we still need the offset to cover the PDX_NONE variant that we also
use? In v1 I just created a local override but Julien wanted to generalize the
macros instead. The discussion about switching the default on Arm from mask to
offset that is not even selectable on Arm needs to wait for the new release 
cycle.

~Michal