This is the code already present and used by x86 in setup_max_pdx(), which
takes into account the current PDX compression, plus the limitation of the
virtual memory layout to return the maximum usable PFN in the system,
possibly truncating the input PFN provided by the caller.

This helper will be used by upcoming PDX related changes that introduce a
new compression algorithm.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/setup.c  | 19 ++-----------------
 xen/common/pdx.c      | 25 +++++++++++++++++++++++++
 xen/include/xen/pdx.h |  8 ++++++++
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 1f5cb67bd0ee..ea670567cbf7 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -721,23 +721,8 @@ static uint64_t __init consider_modules(
 
 static void __init setup_max_pdx(unsigned long top_page)
 {
-    max_pdx = pfn_to_pdx(top_page - 1) + 1;
-
-    if ( max_pdx > (DIRECTMAP_SIZE >> PAGE_SHIFT) )
-        max_pdx = DIRECTMAP_SIZE >> PAGE_SHIFT;
-
-    if ( max_pdx > FRAMETABLE_NR )
-        max_pdx = FRAMETABLE_NR;
-
-    if ( max_pdx > MPT_VIRT_SIZE / sizeof(unsigned long) )
-        max_pdx = MPT_VIRT_SIZE / sizeof(unsigned long);
-
-#ifdef PAGE_LIST_NULL
-    if ( max_pdx >= PAGE_LIST_NULL )
-        max_pdx = PAGE_LIST_NULL - 1;
-#endif
-
-    max_page = pdx_to_pfn(max_pdx - 1) + 1;
+    max_page = get_max_pfn(top_page);
+    max_pdx = pfn_to_pdx(max_page - 1) + 1;
 }
 
 /* A temporary copy of the e820 map that we can mess with during bootstrap. */
diff --git a/xen/common/pdx.c b/xen/common/pdx.c
index b8384e6189df..3004c5f28bdd 100644
--- a/xen/common/pdx.c
+++ b/xen/common/pdx.c
@@ -55,6 +55,31 @@ void set_pdx_range(unsigned long smfn, unsigned long emfn)
         __set_bit(idx, pdx_group_valid);
 }
 
+unsigned long get_max_pfn(unsigned long top_pfn)
+{
+    unsigned long pdx = pfn_to_pdx(top_pfn - 1) + 1;
+
+#ifdef DIRECTMAP_SIZE
+    if ( pdx > (DIRECTMAP_SIZE >> PAGE_SHIFT) )
+        pdx = DIRECTMAP_SIZE >> PAGE_SHIFT;
+#endif
+
+    if ( pdx > FRAMETABLE_NR )
+        pdx = FRAMETABLE_NR;
+
+#ifdef MPT_VIRT_SIZE
+    if ( pdx > MPT_VIRT_SIZE / sizeof(unsigned long) )
+        pdx = MPT_VIRT_SIZE / sizeof(unsigned long);
+#endif
+
+#ifdef PAGE_LIST_NULL
+    if ( pdx >= PAGE_LIST_NULL )
+        pdx = PAGE_LIST_NULL - 1;
+#endif
+
+    return pdx_to_pfn(pdx - 1) + 1;
+}
+
 #ifdef CONFIG_PDX_COMPRESSION
 
 /*
diff --git a/xen/include/xen/pdx.h b/xen/include/xen/pdx.h
index 9faeea3ac9f2..0f580853cb2e 100644
--- a/xen/include/xen/pdx.h
+++ b/xen/include/xen/pdx.h
@@ -92,6 +92,14 @@ void set_pdx_range(unsigned long smfn, unsigned long emfn);
  */
 bool __mfn_valid(unsigned long mfn);
 
+/**
+ * Get maximum usable PFN given the virtual address space restrictions.
+ *
+ * @param pdx Maximum PFN
+ * @return Possibly truncated maximum PFN
+ */
+unsigned long get_max_pfn(unsigned long top_pfn);
+
 #define page_to_pdx(pg)  ((pg) - frame_table)
 #define pdx_to_page(pdx) gcc11_wrap(frame_table + (pdx))
 
-- 
2.49.0

On 11.06.2025 19:16, Roger Pau Monne wrote:
> This is the code already present and used by x86 in setup_max_pdx(), which
> takes into account the current PDX compression, plus the limitation of the
> virtual memory layout to return the maximum usable PFN in the system,
> possibly truncating the input PFN provided by the caller.
> 
> This helper will be used by upcoming PDX related changes that introduce a
> new compression algorithm.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/arch/x86/setup.c  | 19 ++-----------------
>  xen/common/pdx.c      | 25 +++++++++++++++++++++++++
>  xen/include/xen/pdx.h |  8 ++++++++
>  3 files changed, 35 insertions(+), 17 deletions(-)

This is all fine for x86, but on Arm you introduce unreachable code, which
Misra dislikes. Yet then it feels like it's wrong anyway that the function
isn't used there.

Jan

On Thu, Jun 12, 2025 at 11:11:14AM +0200, Jan Beulich wrote:
> On 11.06.2025 19:16, Roger Pau Monne wrote:
> > This is the code already present and used by x86 in setup_max_pdx(), which
> > takes into account the current PDX compression, plus the limitation of the
> > virtual memory layout to return the maximum usable PFN in the system,
> > possibly truncating the input PFN provided by the caller.
> > 
> > This helper will be used by upcoming PDX related changes that introduce a
> > new compression algorithm.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> >  xen/arch/x86/setup.c  | 19 ++-----------------
> >  xen/common/pdx.c      | 25 +++++++++++++++++++++++++
> >  xen/include/xen/pdx.h |  8 ++++++++
> >  3 files changed, 35 insertions(+), 17 deletions(-)
> 
> This is all fine for x86, but on Arm you introduce unreachable code, which
> Misra dislikes. Yet then it feels like it's wrong anyway that the function
> isn't used there.

I was also concerned regarding why ARM doesn't have an equivalent
function.  Is the frametable there supposed to cover up to the maximum
physical address?  In which case there's likely no need for any PDX
compression in the first place?

Thanks, Roger.

On 12.06.2025 12:48, Roger Pau Monné wrote:
> On Thu, Jun 12, 2025 at 11:11:14AM +0200, Jan Beulich wrote:
>> On 11.06.2025 19:16, Roger Pau Monne wrote:
>>> This is the code already present and used by x86 in setup_max_pdx(), which
>>> takes into account the current PDX compression, plus the limitation of the
>>> virtual memory layout to return the maximum usable PFN in the system,
>>> possibly truncating the input PFN provided by the caller.
>>>
>>> This helper will be used by upcoming PDX related changes that introduce a
>>> new compression algorithm.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>>  xen/arch/x86/setup.c  | 19 ++-----------------
>>>  xen/common/pdx.c      | 25 +++++++++++++++++++++++++
>>>  xen/include/xen/pdx.h |  8 ++++++++
>>>  3 files changed, 35 insertions(+), 17 deletions(-)
>>
>> This is all fine for x86, but on Arm you introduce unreachable code, which
>> Misra dislikes. Yet then it feels like it's wrong anyway that the function
>> isn't used there.
> 
> I was also concerned regarding why ARM doesn't have an equivalent
> function.  Is the frametable there supposed to cover up to the maximum
> physical address?  In which case there's likely no need for any PDX
> compression in the first place?

Question goes to Arm folks.

Jan