Re: [MINI-OS PATCH 1/2] mm: provide a way to do very early page table allocations

[Jürgen Groß]

On 28.07.25 16:09, Jan Beulich wrote:

On 08.07.2025 08:37, Juergen Gross wrote:

Add a small pool of statically allocated memory pages to be handed out
for very early page table allocations.

This will make it possible to do virtual allocations e.g. for mapping
the shared info page.

Signed-off-by: Juergen Gross <jgr...@suse.com>
---
  arch/x86/mm.c | 16 +++++++++++++++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index bdff38fd..3f5c7ea7 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -640,12 +640,17 @@ void change_readonly(bool readonly)
   * return a valid PTE for a given virtual address. If PTE does not exist,
   * allocate page-table pages.
   */
+#define N_PTS 4

Wouldn't it be prudent to have a comment here mentioning how this number
was derived, i.e. what's known to be covered? (To map the shared info
page I expect you really only need 3? Hence without a comment things may
remain unclear.)

Yes, 3 would have been enough. OTOH having another spare doesn't hurt, as
the memory will be allocated anyway.

I'll add a comment in this regard.

+static char early_pt[PAGE_SIZE * N_PTS] __attribute__((aligned(PAGE_SIZE)));

Maybe better early_pt[N_PTS][PAGE_SIZE], simplifying the allocation
code below a little?

Yes, good idea.

+static unsigned long n_early_pt = N_PTS;

unsigned int would do, I expect? With the suggestion above this could
then also use ARRAY_SIZE(), at which point there would be no real need
for N_PTS anymore.

True.

  static int need_pgt_func(unsigned long va, unsigned int lvl, bool is_leaf,
                           pgentry_t *pte, void *par)
  {
      pgentry_t **result = par;
      unsigned long pt_mfn;
      unsigned long pt_pfn;
+    unsigned long pt_addr;
      unsigned int idx;

@@ -664,7 +669,16 @@ static int need_pgt_func(unsigned long va, unsigned int 
lvl, bool is_leaf,
      }

-    pt_pfn = virt_to_pfn(alloc_page());
+    if ( n_early_pt )
+    {
+        n_early_pt--;
+        pt_addr = (unsigned long)&early_pt[n_early_pt * PAGE_SIZE];
+    }
+    else
+    {
+        pt_addr = alloc_page();
+    }

if ( !is_leaf ) pt_mfn = virt_to_mfn(pte);

The failure pattern when one fails to increase early_pt[] is likely going
to be problematic. Wouldn't it be better to check for failure here?

Hmm, not sure this is true. I tried the shared info mapping without adding
the special early alloc code first and finding the bug was quite easy.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature