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 16:52, Orzel, Michal wrote:
> 
> 
> On 05-May-26 15:05, Jan Beulich wrote:
>> On 30.04.2026 14:51, Michal Orzel wrote:
>>> --- a/xen/include/xen/pdx.h
>>> +++ b/xen/include/xen/pdx.h
>>> @@ -132,8 +132,9 @@ void set_pdx_range(unsigned long smfn, unsigned long 
>>> emfn);
>>>   */
>>>  bool __mfn_valid(unsigned long mfn);
>>>  
>>> -#define page_to_pdx(pg)  ((pg) - frame_table)
>>> -#define pdx_to_page(pdx) gcc11_wrap(frame_table + (pdx))
>>> +#define page_to_pdx(pg) \
>>> +    ((unsigned long)((pg) - frame_table) + frametable_base_pdx)
>>> +#define pdx_to_page(pdx) gcc11_wrap(frame_table + ((pdx) - 
>>> frametable_base_pdx))
>>
>> If you alter these, ...
>>
>>>  #define mfn_to_pdx(mfn) pfn_to_pdx(mfn_x(mfn))
>>>  #define pdx_to_mfn(pdx) _mfn(pdx_to_pfn(pdx))
>>
>> ... how come these can remain unaltered? Maybe you have some special
>> arrangements in Arm code, but surely in generic code transformations done
>> should be uniform. After all
>>
>>     ASSERT(page_to_pdx(pg) == mfn_to_pdx(page_to_mfn(pg)));
>>
>> (and alike) ought to be universally true for valid inputs.
> The invariant holds. There are two transformations on different
> boundaries:
> 
>   - PFN <-> PDX: the compression scheme — lives in mfn_to_pdx /
>     pdx_to_mfn.
>   - PDX <-> frame-table index: +/- frametable_base_pdx — lives in
>     page_to_pdx / pdx_to_page (and Arm's page_to_mfn / mfn_to_page).
> 
> On x86 the second is the identity (frametable_base_pdx == 0), so it's
> invisible. On Arm it isn't, so it has to appear in the macros that
> cross that boundary. Pushing it into mfn_to_pdx as well would mix the
> two boundaries and double-apply on Arm (page_to_mfn already adds it).

That's yet more odd. These transformations should equally apply to
MFN <-> page (i.e. frame table index) and MFN <-> PDX translations.
PDX really is meant to be the frame table index, and at the same
time (scaled by PAGE_SHIFT) the direct map index. Both (generally
huge) tables equally benefit from whatever compression is in use,
and hence also ought to equally benefit from that
frametable_base_pdx-only sub-form of offset compression. The
anomaly of shrinking only one of the two pretty clearly shouldn't
be extended past Arm, and ideally would be addressed there at some
point.

Jan