[PATCH v2 0/2] xen/arm: Improve frametable allocation

[PATCH v2 0/2] xen/arm: Improve frametable allocation

[Michal Orzel]

Michal Orzel (2):
  xen/pdx: account for frametable_base_pdx in generic
    pdx_to_page/page_to_pdx
  xen/arm: skip holes in physical address space when setting up
    frametable

 xen/arch/arm/arm32/mmu/mm.c    |  3 +-
 xen/arch/arm/include/asm/mm.h  |  9 +---
 xen/arch/arm/include/asm/pdx.h | 21 ++++++++
 xen/arch/arm/mm.c              |  3 +-
 xen/arch/arm/mmu/mm.c          | 89 +++++++++++++++++++++++++---------
 xen/arch/arm/mpu/mm.c          | 23 ++++-----
 xen/arch/ppc/include/asm/mm.h  |  5 --
 xen/arch/ppc/include/asm/pdx.h | 12 +++++
 xen/arch/ppc/mm-radix.c        |  1 +
 xen/arch/x86/include/asm/pdx.h |  6 +++
 xen/include/xen/pdx.h          | 17 +++++--
 11 files changed, 135 insertions(+), 54 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/pdx.h
 create mode 100644 xen/arch/ppc/include/asm/pdx.h

-- 
2.43.0

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

[Michal Orzel]

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.

While at it, consolidate the arch-specific PDX definitions
(frametable_base_pdx and PDX_GROUP_SHIFT) from the arch mm.h headers
into new asm/pdx.h headers for ARM and PPC. The asm/pdx.h is included
earlier in xen/pdx.h via __has_include, making these definitions available
before they are needed.

Also decouple the __has_include(<asm/pdx.h>) check from the PFN-to-PDX
translation override mechanism. Previously, the existence of asm/pdx.h
was taken to mean the arch provides custom pdx_to_pfn / pfn_to_pdx
implementations. This conflation would prevent ARM and PPC from having
asm/pdx.h (for frametable_base_pdx) without also being forced to define
the translation helpers. Replace the __has_include gate with an explicit
ARCH_DEFINES_PDX_XLATE sentinel that only x86 defines.

No functional change.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - new patch
---
 xen/arch/arm/include/asm/mm.h  |  5 -----
 xen/arch/arm/include/asm/pdx.h | 21 +++++++++++++++++++++
 xen/arch/arm/mm.c              |  1 +
 xen/arch/ppc/include/asm/mm.h  |  5 -----
 xen/arch/ppc/include/asm/pdx.h | 12 ++++++++++++
 xen/arch/ppc/mm-radix.c        |  1 +
 xen/arch/x86/include/asm/pdx.h |  6 ++++++
 xen/include/xen/pdx.h          | 17 ++++++++++++-----
 8 files changed, 53 insertions(+), 15 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/pdx.h
 create mode 100644 xen/arch/ppc/include/asm/pdx.h

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 72a692862420..8180b1e12baf 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -181,11 +181,6 @@ struct page_info
 
 #define maddr_get_owner(ma)   (page_get_owner(maddr_to_page((ma))))
 
-/* PDX of the first page in the frame table. */
-extern unsigned long frametable_base_pdx;
-
-#define PDX_GROUP_SHIFT SECOND_SHIFT
-
 /* Boot-time pagetable setup */
 extern void setup_pagetables(void);
 /* Check that the mapping flag has no W and X together */
diff --git a/xen/arch/arm/include/asm/pdx.h b/xen/arch/arm/include/asm/pdx.h
new file mode 100644
index 000000000000..651df4b210dc
--- /dev/null
+++ b/xen/arch/arm/include/asm/pdx.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef ARM_PDX_H
+#define ARM_PDX_H
+
+#include <asm/lpae.h>
+
+#define PDX_GROUP_SHIFT SECOND_SHIFT
+
+#define ARCH_HAS_FRAMETABLE_BASE_PDX
+extern unsigned long frametable_base_pdx;
+
+#endif /* ARM_PDX_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6eddbcf912ee..faef0efb327c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -16,6 +16,7 @@
 #include <xen/static-shmem.h>
 #include <xen/vmap.h>
 
+#include <asm/pdx.h>
 #include <asm/setup.h>
 
 #include <xsm/xsm.h>
diff --git a/xen/arch/ppc/include/asm/mm.h b/xen/arch/ppc/include/asm/mm.h
index 402d06bdaa9f..d55393ff2aa4 100644
--- a/xen/arch/ppc/include/asm/mm.h
+++ b/xen/arch/ppc/include/asm/mm.h
@@ -163,9 +163,6 @@ struct page_info
 #define FRAMETABLE_VIRT_START  (XEN_VIRT_START + GB(32))
 #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
 
-/* PDX of the first page in the frame table. */
-extern unsigned long frametable_base_pdx;
-
 /* Convert between machine frame numbers and page-info structures. */
 #define mfn_to_page(mfn)                                            \
     (frame_table + (mfn_to_pdx(mfn) - frametable_base_pdx))
@@ -220,8 +217,6 @@ static inline void put_page_and_type(struct page_info *page)
 #define set_gpfn_from_mfn(mfn, pfn) BUG_ON("unimplemented")
 #define mfn_to_gfn(d, mfn) ({ BUG_ON("unimplemented"); _gfn(0); })
 
-#define PDX_GROUP_SHIFT XEN_PT_SHIFT_LVL_3
-
 static inline unsigned long domain_get_maximum_gpfn(struct domain *d)
 {
     BUG_ON("unimplemented");
diff --git a/xen/arch/ppc/include/asm/pdx.h b/xen/arch/ppc/include/asm/pdx.h
new file mode 100644
index 000000000000..4290c7bc9c36
--- /dev/null
+++ b/xen/arch/ppc/include/asm/pdx.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef PPC_PDX_H
+#define PPC_PDX_H
+
+#include <asm/page.h>
+
+#define PDX_GROUP_SHIFT XEN_PT_SHIFT_LVL_3
+
+#define ARCH_HAS_FRAMETABLE_BASE_PDX
+extern unsigned long frametable_base_pdx;
+
+#endif /* PPC_PDX_H */
diff --git a/xen/arch/ppc/mm-radix.c b/xen/arch/ppc/mm-radix.c
index d5385ec9dd4b..2eddd86822e0 100644
--- a/xen/arch/ppc/mm-radix.c
+++ b/xen/arch/ppc/mm-radix.c
@@ -10,6 +10,7 @@
 
 #include <asm/early_printk.h>
 #include <asm/page.h>
+#include <asm/pdx.h>
 #include <asm/processor.h>
 #include <asm/regs.h>
 #include <asm/msr.h>
diff --git a/xen/arch/x86/include/asm/pdx.h b/xen/arch/x86/include/asm/pdx.h
index 6be7e1185eb1..5e660be17e39 100644
--- a/xen/arch/x86/include/asm/pdx.h
+++ b/xen/arch/x86/include/asm/pdx.h
@@ -3,8 +3,12 @@
 #ifndef X86_PDX_H
 #define X86_PDX_H
 
+#ifndef CONFIG_PDX_NONE
+
 #include <asm/alternative.h>
 
+#define ARCH_DEFINES_PDX_XLATE
+
 /*
  * Introduce a macro to avoid repeating the same asm goto block in each helper.
  * Note the macro is strictly tied to the code in the helpers.
@@ -59,6 +63,8 @@ static inline paddr_t directmapoff_to_maddr(unsigned long offset)
 
 #undef PDX_ASM_GOTO_SKIP
 
+#endif /* !CONFIG_PDX_NONE */
+
 #endif /* X86_PDX_H */
 
 /*
diff --git a/xen/include/xen/pdx.h b/xen/include/xen/pdx.h
index 856fc3e8a0e6..59c257651953 100644
--- 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))
 
 #define mfn_to_pdx(mfn) pfn_to_pdx(mfn_x(mfn))
 #define pdx_to_mfn(pdx) _mfn(pdx_to_pfn(pdx))
@@ -244,6 +245,14 @@ static inline paddr_t directmapoff_to_maddr_xlate(unsigned long offset)
 
 #endif /* CONFIG_PDX_OFFSET_COMPRESSION */
 
+#if __has_include(<asm/pdx.h>)
+# include <asm/pdx.h>
+#endif
+
+#ifndef ARCH_HAS_FRAMETABLE_BASE_PDX
+#define frametable_base_pdx 0
+#endif
+
 #ifdef CONFIG_PDX_NONE
 
 /* Without PDX compression we can skip some computations */
@@ -283,9 +292,7 @@ static inline void pfn_pdx_compression_reset(void)
  *
  * Do not use _xlate suffixed functions, always use the non _xlate variants.
  */
-#if __has_include(<asm/pdx.h>)
-# include <asm/pdx.h>
-#else
+#ifndef ARCH_DEFINES_PDX_XLATE
 # define pdx_to_pfn pdx_to_pfn_xlate
 # define pfn_to_pdx pfn_to_pdx_xlate
 # define maddr_to_directmapoff maddr_to_directmapoff_xlate
-- 
2.43.0

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

[Luca Fancellu]

Hi Michal,

> On 30 Apr 2026, at 13:51, Michal Orzel <michal.orzel@amd.com> 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

“correct for all architectures” I think only RISC-V needs some work to eventually
be able to use the generic macros?

> today.
> 
> While at it, consolidate the arch-specific PDX definitions
> (frametable_base_pdx and PDX_GROUP_SHIFT) from the arch mm.h headers
> into new asm/pdx.h headers for ARM and PPC. The asm/pdx.h is included
> earlier in xen/pdx.h via __has_include, making these definitions available
> before they are needed.
> 
> Also decouple the __has_include(<asm/pdx.h>) check from the PFN-to-PDX
> translation override mechanism. Previously, the existence of asm/pdx.h
> was taken to mean the arch provides custom pdx_to_pfn / pfn_to_pdx
> implementations. This conflation would prevent ARM and PPC from having
> asm/pdx.h (for frametable_base_pdx) without also being forced to define
> the translation helpers. Replace the __has_include gate with an explicit
> ARCH_DEFINES_PDX_XLATE sentinel that only x86 defines.
> 
> No functional change.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---

The changes looks good to me, for the Arm and common part:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common

I’ve also tested on Arm64, Arm32, x86_64 with virtual platforms.

Cheers,
Luca

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

On Fri, 1 May 2026, Luca Fancellu wrote:
> The changes looks good to me, for the Arm and common part:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> # arm, common
> 
> I’ve also tested on Arm64, Arm32, x86_64 with virtual platforms.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

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

[Roger Pau Monné]

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.

Thanks, Roger.

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

[Orzel, Michal]

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. 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.

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

~Michal

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

[Roger Pau Monné]

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.

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

[Orzel, Michal]

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.

> 
>> 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?
Switching the compression method would be a bigger change, and with feature
freeze on Friday I'd prefer not to get into that now. The current approach
is minimal and self-contained and works with mask and no-pdx which is what we
use nowadays: frametable_base_pdx already existed on ARM and PPC, we're just
making the generic macros aware of it as Julien requested (in v1 I just
overwrote the macro in local file). We can revisit the compression strategy as a
follow-up next release.

~Michal

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

[Roger Pau Monné]

On Tue, May 05, 2026 at 09:35:05AM +0200, 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.

Right, you need to use a PDX compression to fit.  My preference IMO
would be to add the leading offset into the PDX mask compression
algorithm if that's what ARM uses by default now.  The generic case
really means no compression, and that's a 1:1 map between physical
addresses and PDX.  Anything that's not an identity mapping between
those two address spaces implies some kind of compression.

> > 
> >> 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?
> Switching the compression method would be a bigger change, and with feature
> freeze on Friday I'd prefer not to get into that now. The current approach
> is minimal and self-contained and works with mask and no-pdx which is what we
> use nowadays: frametable_base_pdx already existed on ARM and PPC, we're just
> making the generic macros aware of it as Julien requested (in v1 I just
> overwrote the macro in local file). We can revisit the compression strategy as a
> follow-up next release.

Right, I'm not going to oppose to this, but I also don't think it's
the right way to go.  This seems like a bodge on the side of PDX
compression, when it should have instead been integrated into it.  The
more that the offset compression logic will already do that removal of
the leading empty space up to the first RAM region, and hence such
adjustment of PDX values in that case is redundant at best.

Also, you only do this PDX adjustment for the frametable, but not the
direct map?

Thanks, Roger.

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

[Jan Beulich]

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

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

[Orzel, Michal]

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

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

[Jan Beulich]

On 05.05.2026 12:46, Orzel, Michal wrote:
> 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.

I'm not convinced of that. If you need offset by default, why not enable it by
default (right now, and potentially even as a backport if there's any bug that
is being fixed)?

Jan

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

[Orzel, Michal]

On 05-May-26 12:49, Jan Beulich wrote:
> On 05.05.2026 12:46, Orzel, Michal wrote:
>> 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.
> 
> I'm not convinced of that. If you need offset by default, why not enable it by
> default (right now, and potentially even as a backport if there's any bug that
> is being fixed)?
As said before, we also need offset when using just PDX grouping and no compression.

~Michal

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

[Roger Pau Monné]

On Tue, May 05, 2026 at 01:46:51PM +0200, Orzel, Michal wrote:
> 
> 
> On 05-May-26 12:49, Jan Beulich wrote:
> > On 05.05.2026 12:46, Orzel, Michal wrote:
> >> 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.
> > 
> > I'm not convinced of that. If you need offset by default, why not enable it by
> > default (right now, and potentially even as a backport if there's any bug that
> > is being fixed)?
> As said before, we also need offset when using just PDX grouping and no compression.

But you don't really mean no compression?  The offset itself that you
subtract is a transformation, and hence a compression, as the physical
and PDX address spaces are no longer identity mapped?  Maybe those
systems should have never worked with PDX_NONE, and instead required
a PDX compression in place (one that would remove the offset from 0 to
the first RAM range).

It's an incomplete conversion IMO, as ARM applies it to the
frametable, but not the direct map.

Thanks, Roger.

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

[Orzel, Michal]

On 05-May-26 14:15, Roger Pau Monné wrote:
> On Tue, May 05, 2026 at 01:46:51PM +0200, Orzel, Michal wrote:
>>
>>
>> On 05-May-26 12:49, Jan Beulich wrote:
>>> On 05.05.2026 12:46, Orzel, Michal wrote:
>>>> 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.
>>>
>>> I'm not convinced of that. If you need offset by default, why not enable it by
>>> default (right now, and potentially even as a backport if there's any bug that
>>> is being fixed)?
>> As said before, we also need offset when using just PDX grouping and no compression.
> 
> But you don't really mean no compression?  The offset itself that you
> subtract is a transformation, and hence a compression, as the physical
> and PDX address spaces are no longer identity mapped?  Maybe those
> systems should have never worked with PDX_NONE, and instead required
> a PDX compression in place (one that would remove the offset from 0 to
> the first RAM range).
I'd argue the PDX <-> PFN mapping itself is still identity here — with
PDX_NONE, pfn_to_pdx(x) == x and pdx_to_pfn(x) == x. frametable_base_pdx
is not a PDX-space transformation; it's an offset that's only applied
when *indexing into the frame table*

Conceptually it's closer to "skip the leading hole in the lookup arrays"
than to a compression of the PFN/PDX number space. It also sits on top
of (and is orthogonal to) whatever PDX scheme is selected.

> 
> It's an incomplete conversion IMO, as ARM applies it to the
> frametable, but not the direct map.
It is applied to the directmap on arm64 — see directmap_base_pdx.

So both the frametable and (on arm64) the directmap apply the
equivalent offset. They use separate variables (frametable_base_pdx vs.
directmap_base_pdx) but the principle is the same.

> 
> Thanks, Roger.

~Michal

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

[Jan Beulich]

On 05.05.2026 13:46, Orzel, Michal wrote:
> On 05-May-26 12:49, Jan Beulich wrote:
>> On 05.05.2026 12:46, Orzel, Michal wrote:
>>> 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.
>>
>> I'm not convinced of that. If you need offset by default, why not enable it by
>> default (right now, and potentially even as a backport if there's any bug that
>> is being fixed)?
> As said before, we also need offset when using just PDX grouping and no compression.

And as also said before, this really is poor man's offset compression then. That
may be tolerable if you insist that's best for Arm, yet then I'd suggest to limit
that offset to just the "no compression" case. It's redundant with offset
compression, and it may be (possible to make) redundant with mask compression.
If the latter can't be arranged for, an offset may want introducing there as well.
But it shouldn't exist independent of the compression scheme used.

Jan

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

[Orzel, Michal]

On 05-May-26 15:00, Jan Beulich wrote:
> On 05.05.2026 13:46, Orzel, Michal wrote:
>> On 05-May-26 12:49, Jan Beulich wrote:
>>> On 05.05.2026 12:46, Orzel, Michal wrote:
>>>> 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.
>>>
>>> I'm not convinced of that. If you need offset by default, why not enable it by
>>> default (right now, and potentially even as a backport if there's any bug that
>>> is being fixed)?
>> As said before, we also need offset when using just PDX grouping and no compression.
> 
> And as also said before, this really is poor man's offset compression then. That
> may be tolerable if you insist that's best for Arm, yet then I'd suggest to limit
> that offset to just the "no compression" case. It's redundant with offset
> compression, and it may be (possible to make) redundant with mask compression.
> If the latter can't be arranged for, an offset may want introducing there as well.
> But it shouldn't exist independent of the compression scheme used.
Having a single per-scheme mechanism rather than an extra independent offset is
cleaner. But I don't think we can limit frametable_base_pdx to PDX_NONE today:

 - Mask compression doesn't fold a leading [0, first_ram_pdx) zero
   prefix into anything. So the PDX of
   the first RAM frame stays at first_ram_pdx, and without the offset
   the frame table virtual extent is max_pdx * sizeof(page_info)
   rather than (max_pdx - first_ram_pdx) * sizeof(page_info).

   For systems with a high RAM base (the 5TB example I gave earlier
   needs ~70GB just to skip the leading hole, vs. 32GB FRAMETABLE_SIZE
   on arm64) the (max_pdx > FRAMETABLE_NR) check then fails and we
   panic before mapping anything. pdx_group_valid (which patch 2/2
   adds) avoids backing those leading groups with physical memory, but
   it doesn't shrink the virtual extent — only the offset does.

 - With offset compression you're right that the leading hole could be
   absorbed into the lookup table, making the extra offset redundant.
   But Arm doesn't currently select offset compression, it's non-selectable,
untested and switching
   the default is a separate (and bigger) discussion that I don't think
   should block this fix given the state of the release.

So as it stands, the offset is needed on Arm for both PDX_NONE and
PDX_MASK_COMPRESSION. Folding it into the mask scheme (and dropping it
for offset compression) is a reasonable cleanup, but it's a refactor
of the compression layer itself, not something I'd like to mix into
this series.


~Michal

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

[Roger Pau Monné]

On Tue, May 05, 2026 at 04:44:16PM +0200, Orzel, Michal wrote:
> 
> 
> On 05-May-26 15:00, Jan Beulich wrote:
> > On 05.05.2026 13:46, Orzel, Michal wrote:
> >> On 05-May-26 12:49, Jan Beulich wrote:
> >>> On 05.05.2026 12:46, Orzel, Michal wrote:
> >>>> 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.
> >>>
> >>> I'm not convinced of that. If you need offset by default, why not enable it by
> >>> default (right now, and potentially even as a backport if there's any bug that
> >>> is being fixed)?
> >> As said before, we also need offset when using just PDX grouping and no compression.
> > 
> > And as also said before, this really is poor man's offset compression then. That
> > may be tolerable if you insist that's best for Arm, yet then I'd suggest to limit
> > that offset to just the "no compression" case. It's redundant with offset
> > compression, and it may be (possible to make) redundant with mask compression.
> > If the latter can't be arranged for, an offset may want introducing there as well.
> > But it shouldn't exist independent of the compression scheme used.
> Having a single per-scheme mechanism rather than an extra independent offset is
> cleaner. But I don't think we can limit frametable_base_pdx to PDX_NONE today:
> 
>  - Mask compression doesn't fold a leading [0, first_ram_pdx) zero
>    prefix into anything. So the PDX of
>    the first RAM frame stays at first_ram_pdx, and without the offset
>    the frame table virtual extent is max_pdx * sizeof(page_info)
>    rather than (max_pdx - first_ram_pdx) * sizeof(page_info).
> 
>    For systems with a high RAM base (the 5TB example I gave earlier
>    needs ~70GB just to skip the leading hole, vs. 32GB FRAMETABLE_SIZE
>    on arm64) the (max_pdx > FRAMETABLE_NR) check then fails and we
>    panic before mapping anything. pdx_group_valid (which patch 2/2
>    adds) avoids backing those leading groups with physical memory, but
>    it doesn't shrink the virtual extent — only the offset does.
> 
>  - With offset compression you're right that the leading hole could be
>    absorbed into the lookup table, making the extra offset redundant.
>    But Arm doesn't currently select offset compression, it's non-selectable,
> untested and switching
>    the default is a separate (and bigger) discussion that I don't think
>    should block this fix given the state of the release.
> 
> So as it stands, the offset is needed on Arm for both PDX_NONE and
> PDX_MASK_COMPRESSION. Folding it into the mask scheme (and dropping it
> for offset compression) is a reasonable cleanup, but it's a refactor
> of the compression layer itself, not something I'd like to mix into
> this series.

Right, then it's likely best to avoid generalizing frametable_base_pdx
and instead focus on integrating it with the PDX mask compression
algorithm at a possibly later time (after release)?

Thanks, Roger.

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

[Jan Beulich]

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.

Jan

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

[Orzel, Michal]

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).

~Michal

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

[Jan Beulich]

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

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

[Orzel, Michal]

On 05-May-26 18:11, Jan Beulich wrote:
> 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.
Fair enough. I'll drop this patch and update the second with a local change (as
in v1 but this time with a comment explaining why) not to extend this behavior
past Arm and we can try to make things generic next release.

~Michal

[PATCH v2 2/2] xen/arm: skip holes in physical address space when setting up frametable

[Michal Orzel]

Refactor setup_frametable_mappings() into init_frametable(), modeled
after x86's implementation. Instead of mapping one contiguous frametable
covering ram_start to ram_end (including holes), iterate the
pdx_group_valid bitmap to allocate and map frametable memory only for
valid PDX groups, skipping gaps in the physical address space. At the
moment we don't really take into account pdx_group_valid bitmap.

This reduces memory consumption on systems with sparse RAM layouts by
not allocating frametable entries for non-existent memory regions.

The chunk allocator rounds chunk_size up to PAGE_SIZE only, rather than
to a larger mapping granularity, to avoid overshooting past chunk
boundaries into subsequent gaps or valid regions. This rounding has no
impact for in-loop chunks given that chunk size is multiple of 14MB on
Arm64 and 2MB on Arm32. The rounding matters only for the last out-of-loop
chunk. Physical allocations use 32MB alignment so that map_pages_to_xen()
can use the contiguous bit for larger TLB entries where virtual alignment
also permits.

Update the MPU implementation to match the new init_frametable()
signature. Since MPU has no virtual address translation (ma == va),
hole-skipping is not possible and the frametable remains a single
contiguous allocation.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - fix overshoot problem with 32MB rounding
---
 xen/arch/arm/arm32/mmu/mm.c   |  3 +-
 xen/arch/arm/include/asm/mm.h |  4 +-
 xen/arch/arm/mm.c             |  2 +-
 xen/arch/arm/mmu/mm.c         | 89 ++++++++++++++++++++++++++---------
 xen/arch/arm/mpu/mm.c         | 23 ++++-----
 5 files changed, 82 insertions(+), 39 deletions(-)

diff --git a/xen/arch/arm/arm32/mmu/mm.c b/xen/arch/arm/arm32/mmu/mm.c
index 5e4766ddcf65..0b595baa11b3 100644
--- a/xen/arch/arm/arm32/mmu/mm.c
+++ b/xen/arch/arm/arm32/mmu/mm.c
@@ -178,8 +178,7 @@ void __init setup_mm(void)
 
     setup_directmap_mappings(mfn_x(directmap_mfn_start), xenheap_pages);
 
-    /* Frame table covers all of RAM region, including holes */
-    setup_frametable_mappings(ram_start, ram_end);
+    init_frametable(ram_start);
 
     /*
      * The allocators may need to use map_domain_page() (such as for
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 8180b1e12baf..2616a1305d58 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -191,8 +191,8 @@ extern void *early_fdt_map(paddr_t fdt_paddr);
 extern void remove_early_mappings(void);
 /* Prepare the memory subystem to bring-up the given secondary CPU */
 extern int prepare_secondary_mm(int cpu);
-/* Map a frame table to cover physical addresses ps through pe */
-extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
+/* Map a frame table */
+void init_frametable(paddr_t ram_start);
 /* Helper function to setup memory management */
 void setup_mm_helper(void);
 /* map a physical range in virtual memory */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index faef0efb327c..7297cca01551 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -63,7 +63,7 @@ void __init setup_mm(void)
 
     setup_mm_helper();
 
-    setup_frametable_mappings(ram_start, ram_end);
+    init_frametable(ram_start);
 
     init_staticmem_pages();
     init_sharedmem_pages();
diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
index 6604f3bf4e6a..dfc888c8ee0e 100644
--- a/xen/arch/arm/mmu/mm.c
+++ b/xen/arch/arm/mmu/mm.c
@@ -6,18 +6,45 @@
 #include <xen/mm.h>
 #include <xen/mm-frame.h>
 #include <xen/pdx.h>
+#include <xen/sizes.h>
 #include <xen/string.h>
 
-/* Map a frame table to cover physical addresses ps through pe */
-void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
+static void __init init_frametable_chunk(unsigned long pdx_s,
+                                         unsigned long pdx_e)
 {
-    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(pe), -1)) -
-                            mfn_to_pdx(maddr_to_mfn(ps)) + 1;
-    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
-    mfn_t base_mfn;
-    const unsigned long mapping_size = frametable_size < MB(32) ? MB(2)
-                                                                : MB(32);
+    unsigned long nr_pdxs = pdx_e - pdx_s;
+    unsigned long chunk_size = nr_pdxs * sizeof(struct page_info);
+    unsigned long virt;
     int rc;
+    mfn_t base_mfn;
+
+    /*
+     * In-loop chunks span whole PDX groups, which are always page-size
+     * aligned. The last chunk ending at max_pdx may not be, so round up.
+     */
+    chunk_size = ROUNDUP(chunk_size, PAGE_SIZE);
+
+    /*
+     * Align the allocation to the contiguous mapping size so that
+     * map_pages_to_xen() can use the contiguous bit.
+     */
+    base_mfn = alloc_boot_pages(chunk_size >> PAGE_SHIFT,
+                                MB(32) >> PAGE_SHIFT);
+
+    virt = (unsigned long)pdx_to_page(pdx_s);
+    rc = map_pages_to_xen(virt, base_mfn, chunk_size >> PAGE_SHIFT,
+                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+    if ( rc )
+        panic("Unable to setup the frametable mappings\n");
+
+    memset(pdx_to_page(pdx_s), 0, nr_pdxs * sizeof(struct page_info));
+    memset(pdx_to_page(pdx_e), -1,
+           chunk_size - nr_pdxs * sizeof(struct page_info));
+}
+
+void __init init_frametable(paddr_t ram_start)
+{
+    unsigned int sidx, nidx, max_idx;
 
     /*
      * The size of paddr_t should be sufficient for the complete range of
@@ -26,24 +53,40 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     BUILD_BUG_ON((sizeof(paddr_t) * BITS_PER_BYTE) < PADDR_BITS);
     BUILD_BUG_ON(sizeof(struct page_info) != PAGE_INFO_SIZE);
 
-    if ( frametable_size > FRAMETABLE_SIZE )
-        panic("The frametable cannot cover the physical region %#"PRIpaddr" - %#"PRIpaddr"\n",
-              ps, pe);
+    /* init_frametable_chunk() allocation alignment assumes 4KB granule */
+    BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
 
-    frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
-    /* Round up to 2M or 32M boundary, as appropriate. */
-    frametable_size = ROUNDUP(frametable_size, mapping_size);
-    base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
+    /* In-loop chunks must produce page-aligned frametable regions */
+    BUILD_BUG_ON((PDX_GROUP_COUNT * sizeof(struct page_info)) % PAGE_SIZE);
 
-    rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
-                          frametable_size >> PAGE_SHIFT,
-                          PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
-    if ( rc )
-        panic("Unable to setup the frametable mappings.\n");
+    max_idx = DIV_ROUND_UP(max_pdx, PDX_GROUP_COUNT);
+    frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ram_start));
+
+    /*
+     * pdx_to_page(pdx_s) in init_frametable_chunk must be page-aligned
+     * for map_pages_to_xen(). Aligning to PDX_GROUP_COUNT guarantees this
+     * because PDX_GROUP_COUNT * sizeof(page_info) is always a multiple of
+     * PAGE_SIZE by construction.
+     */
+    frametable_base_pdx = ROUNDDOWN(frametable_base_pdx, PDX_GROUP_COUNT);
+
+    if ( (max_pdx - frametable_base_pdx) > FRAMETABLE_NR )
+        panic("Frametable too small\n");
+
+    for ( sidx = (frametable_base_pdx / PDX_GROUP_COUNT); ; sidx = nidx )
+    {
+        unsigned int eidx;
+
+        eidx = find_next_zero_bit(pdx_group_valid, max_idx, sidx);
+        nidx = find_next_bit(pdx_group_valid, max_idx, eidx);
+
+        if ( nidx >= max_idx )
+            break;
+
+        init_frametable_chunk(sidx * PDX_GROUP_COUNT, eidx * PDX_GROUP_COUNT);
+    }
 
-    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
-    memset(&frame_table[nr_pdxs], -1,
-           frametable_size - (nr_pdxs * sizeof(struct page_info)));
+    init_frametable_chunk(sidx * PDX_GROUP_COUNT, max_pdx);
 }
 
 /*
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index aff88bd3a9c1..9c568831c128 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -186,16 +186,15 @@ static int is_mm_attr_match(pr_t *region, unsigned int attributes)
     return 0;
 }
 
-/* Map a frame table to cover physical addresses ps through pe */
-void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
+/*
+ * Allocate a contiguous frame table covering ram_start through max_pdx.
+ * Unlike the MMU version, MPU cannot skip holes because there is no virtual
+ * address translation (ma == va).
+ */
+void __init init_frametable(paddr_t ram_start)
 {
+    unsigned long nr_pdxs, frametable_size;
     mfn_t base_mfn;
-    paddr_t aligned_ps = ROUNDUP(ps, PAGE_SIZE);
-    paddr_t aligned_pe = ROUNDDOWN(pe, PAGE_SIZE);
-
-    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(aligned_pe), -1)) -
-                            mfn_to_pdx(maddr_to_mfn(aligned_ps)) + 1;
-    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
 
     /*
      * The size of paddr_t should be sufficient for the complete range of
@@ -204,11 +203,13 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     BUILD_BUG_ON((sizeof(paddr_t) * BITS_PER_BYTE) < PADDR_BITS);
     BUILD_BUG_ON(sizeof(struct page_info) != PAGE_INFO_SIZE);
 
+    frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ram_start));
+    nr_pdxs = max_pdx - frametable_base_pdx;
+    frametable_size = nr_pdxs * sizeof(struct page_info);
+
     if ( frametable_size > FRAMETABLE_SIZE )
-        panic("The frametable cannot cover the physical region %#"PRIpaddr" - %#"PRIpaddr"\n",
-              ps, pe);
+        panic("Frametable too small\n");
 
-    frametable_base_pdx = paddr_to_pdx(aligned_ps);
     frametable_size = ROUNDUP(frametable_size, PAGE_SIZE);
 
     base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 1);
-- 
2.43.0

Re: [PATCH v2 2/2] xen/arm: skip holes in physical address space when setting up frametable

[Luca Fancellu]

Hi Michal,

> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index faef0efb327c..7297cca01551 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -63,7 +63,7 @@ void __init setup_mm(void)
> 
>     setup_mm_helper();
> 
> -    setup_frametable_mappings(ram_start, ram_end);
> +    init_frametable(ram_start);

I think that now ram_end and bank_end can be removed

> 
>     init_staticmem_pages();
>     init_sharedmem_pages();
> diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
> index 6604f3bf4e6a..dfc888c8ee0e 100644
> --- a/xen/arch/arm/mmu/mm.c
> +++ b/xen/arch/arm/mmu/mm.c
> @@ -6,18 +6,45 @@
> #include <xen/mm.h>
> #include <xen/mm-frame.h>
> #include <xen/pdx.h>
> +#include <xen/sizes.h>
> #include <xen/string.h>
> 
> -/* Map a frame table to cover physical addresses ps through pe */
> -void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
> +static void __init init_frametable_chunk(unsigned long pdx_s,
> +                                         unsigned long pdx_e)
> {
> -    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(pe), -1)) -
> -                            mfn_to_pdx(maddr_to_mfn(ps)) + 1;
> -    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
> -    mfn_t base_mfn;
> -    const unsigned long mapping_size = frametable_size < MB(32) ? MB(2)
> -                                                                : MB(32);
> +    unsigned long nr_pdxs = pdx_e - pdx_s;
> +    unsigned long chunk_size = nr_pdxs * sizeof(struct page_info);
> +    unsigned long virt;
>     int rc;
> +    mfn_t base_mfn;
> +
> +    /*
> +     * In-loop chunks span whole PDX groups, which are always page-size
> +     * aligned. The last chunk ending at max_pdx may not be, so round up.
> +     */
> +    chunk_size = ROUNDUP(chunk_size, PAGE_SIZE);
> +
> +    /*
> +     * Align the allocation to the contiguous mapping size so that
> +     * map_pages_to_xen() can use the contiguous bit.
> +     */
> +    base_mfn = alloc_boot_pages(chunk_size >> PAGE_SHIFT,
> +                                MB(32) >> PAGE_SHIFT);

This fixed 32Mb alignment feels a bit more than we need, If for example the
chunk is less than 32Mb? If we had some variable alignment for chunks less
than 32MB we would maybe help alloc_boot_pages job, in the end if the chunk
is less than 32Mb it won’t get the contiguous bit anyway.

But I’m fine also if you leave it as it is.

With the above fixed:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

I’ve also tested on Arm64 MMU, Arm32 MMU, Arm64 MPU virtual platforms.

Cheers,
Luca

Re: [PATCH v2 2/2] xen/arm: skip holes in physical address space when setting up frametable

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Fri, 1 May 2026, Luca Fancellu wrote:
> This fixed 32Mb alignment feels a bit more than we need, If for example the
> chunk is less than 32Mb? If we had some variable alignment for chunks less
> than 32MB we would maybe help alloc_boot_pages job, in the end if the chunk
> is less than 32Mb it won’t get the contiguous bit anyway.
> 
> But I’m fine also if you leave it as it is.
> 
> With the above fixed:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> I’ve also tested on Arm64 MMU, Arm32 MMU, Arm64 MPU virtual platforms.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH v2 2/2] xen/arm: skip holes in physical address space when setting up frametable

[Orzel, Michal]

On 01-May-26 17:00, Luca Fancellu wrote:
> Hi Michal,
> 
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index faef0efb327c..7297cca01551 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -63,7 +63,7 @@ void __init setup_mm(void)
>>
>>     setup_mm_helper();
>>
>> -    setup_frametable_mappings(ram_start, ram_end);
>> +    init_frametable(ram_start);
> 
> I think that now ram_end and bank_end can be removed
Right, will do.

> 
>>
>>     init_staticmem_pages();
>>     init_sharedmem_pages();
>> diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
>> index 6604f3bf4e6a..dfc888c8ee0e 100644
>> --- a/xen/arch/arm/mmu/mm.c
>> +++ b/xen/arch/arm/mmu/mm.c
>> @@ -6,18 +6,45 @@
>> #include <xen/mm.h>
>> #include <xen/mm-frame.h>
>> #include <xen/pdx.h>
>> +#include <xen/sizes.h>
>> #include <xen/string.h>
>>
>> -/* Map a frame table to cover physical addresses ps through pe */
>> -void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
>> +static void __init init_frametable_chunk(unsigned long pdx_s,
>> +                                         unsigned long pdx_e)
>> {
>> -    unsigned long nr_pdxs = mfn_to_pdx(mfn_add(maddr_to_mfn(pe), -1)) -
>> -                            mfn_to_pdx(maddr_to_mfn(ps)) + 1;
>> -    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
>> -    mfn_t base_mfn;
>> -    const unsigned long mapping_size = frametable_size < MB(32) ? MB(2)
>> -                                                                : MB(32);
>> +    unsigned long nr_pdxs = pdx_e - pdx_s;
>> +    unsigned long chunk_size = nr_pdxs * sizeof(struct page_info);
>> +    unsigned long virt;
>>     int rc;
>> +    mfn_t base_mfn;
>> +
>> +    /*
>> +     * In-loop chunks span whole PDX groups, which are always page-size
>> +     * aligned. The last chunk ending at max_pdx may not be, so round up.
>> +     */
>> +    chunk_size = ROUNDUP(chunk_size, PAGE_SIZE);
>> +
>> +    /*
>> +     * Align the allocation to the contiguous mapping size so that
>> +     * map_pages_to_xen() can use the contiguous bit.
>> +     */
>> +    base_mfn = alloc_boot_pages(chunk_size >> PAGE_SHIFT,
>> +                                MB(32) >> PAGE_SHIFT);
> 
> This fixed 32Mb alignment feels a bit more than we need, If for example the
> chunk is less than 32Mb? If we had some variable alignment for chunks less
> than 32MB we would maybe help alloc_boot_pages job, in the end if the chunk
> is less than 32Mb it won’t get the contiguous bit anyway.
Good point. On Arm64 this affects any chunk spanning fewer than 3 valid PDX
groups (~14MB per group). I'll use 32MB if chunk size >= 32MB, 2MB otherwise.

> 
> But I’m fine also if you leave it as it is.
> 
> With the above fixed:
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
I can take this one but ...

> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
not this one given the change.

~Michal