Re: [PATCH 2/3] mini-os: mm: switch need_pgt() to use walk_pt()

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Jürgen Groß, le jeu. 01 août 2024 07:56:36 +0200, a ecrit:
> On 31.07.24 23:27, Samuel Thibault wrote:
> > Hello,
> > 
> > Juergen Gross, le mer. 31 juil. 2024 15:00:25 +0200, a ecrit:
> > > -pgentry_t *need_pgt(unsigned long va)
> > > +static int need_pgt_func(unsigned long va, unsigned int lvl, bool 
> > > is_leaf,
> > > +                         pgentry_t *pte, void *par)
> > >   {
> > [...]
> > > +    if ( lvl == L1_FRAME || (*pte & _PAGE_PRESENT) )
> > 
> > Did you mean (*pte & _PAGE_PSE)?
> 
> No.

I don't understand: it doesn't map what I know of need_pgt and what the
existing code is doing.

AIKI, the point of need_pgt is to make sure there is a L1 page table
entry for a VA, and return it, so the caller can put in it at pte for a
mfn or such. In the case a PSE is met, we don't go further, and it's up
to the caller to decide what it wants to do (most often it's actually
unexpected and asserted out). In both cases, the PRESENT bit of the
pte whose address is returned does not matter, most often it's the
caller which will set it.

The existing code for need_pgt thus always adds page table entries down
to level1 (except if _PAGE_PSE is met, i.e. a large page was already set
up): the termination was:

[... walk down to level 2]
-    if ( tab[offset] & _PAGE_PSE )
-        return &tab[offset];
[... walk down to level 1]
-    return &tab[offset];

i.e. we always return either a PSE L2 entry, or an L1 entry, and never
above (while keeping sure that the entries above are present).

> I want to bail out if the PTE does not require a page table to be
> added to it. This is the case if the PTE is valid

There being an entry at e.g. level 3 (without PSE) does not mean that
we have an entry at level 1 for the VA, so we have to continue adding
levels for the VA in the sparse page table tree.

> An invalid PTE with PSE set is still invalid, so the PSE bit has no
> real meaning.

Mmm, I don't think we ever create entries with PSE but not PRESENT (i.e.
we don't "plan" to have PSE entries), but even if we did, if we "plan"
to have PSE entries, we should return that entry, and it's up to the
caller to fill it (and make it PRESENT)

Samuel