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

Juergen Gross posted 3 patches 3 months, 1 week ago
[PATCH v2 2/3] mini-os: mm: switch need_pgt() to use walk_pt()
Posted by Juergen Gross 3 months, 1 week ago
Instead of open coding a page table walk, use walk_pt() in need_pgt().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- add comment and ASSERT() (Samuel Thibault)
---
 arch/x86/mm.c | 72 +++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 42 deletions(-)

diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 9849b985..84a6d7f0 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -523,57 +523,45 @@ static pgentry_t *get_pgt(unsigned long va)
  * return a valid PTE for a given virtual address. If PTE does not exist,
  * allocate page-table pages.
  */
-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)
 {
+    pgentry_t **result = par;
     unsigned long pt_mfn;
-    pgentry_t *tab;
     unsigned long pt_pfn;
-    unsigned offset;
+    unsigned int idx;
 
-    tab = pt_base;
-    pt_mfn = virt_to_mfn(pt_base);
+    if ( !is_leaf )
+        return 0;
 
-#if defined(__x86_64__)
-    offset = l4_table_offset(va);
-    if ( !(tab[offset] & _PAGE_PRESENT) )
-    {
-        pt_pfn = virt_to_pfn(alloc_page());
-        if ( !pt_pfn )
-            return NULL;
-        new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);
-    }
-    ASSERT(tab[offset] & _PAGE_PRESENT);
-    pt_mfn = pte_to_mfn(tab[offset]);
-    tab = mfn_to_virt(pt_mfn);
-#endif
-    offset = l3_table_offset(va);
-    if ( !(tab[offset] & _PAGE_PRESENT) ) 
-    {
-        pt_pfn = virt_to_pfn(alloc_page());
-        if ( !pt_pfn )
-            return NULL;
-        new_pt_frame(&pt_pfn, pt_mfn, offset, L2_FRAME);
-    }
-    ASSERT(tab[offset] & _PAGE_PRESENT);
-    pt_mfn = pte_to_mfn(tab[offset]);
-    tab = mfn_to_virt(pt_mfn);
-    offset = l2_table_offset(va);
-    if ( !(tab[offset] & _PAGE_PRESENT) )
+    if ( lvl == L1_FRAME || (*pte & _PAGE_PRESENT) )
     {
-        pt_pfn = virt_to_pfn(alloc_page());
-        if ( !pt_pfn )
-            return NULL;
-        new_pt_frame(&pt_pfn, pt_mfn, offset, L1_FRAME);
+        /*
+         * The PTE is not addressing a page table (is_leaf is true). If we are
+         * either at the lowest level or we have a valid large page, we don't
+         * need to allocate a page table.
+         */
+        ASSERT(lvl == L1_FRAME || (*pte & _PAGE_PSE));
+        *result = pte;
+        return 1;
     }
-    ASSERT(tab[offset] & _PAGE_PRESENT);
-    if ( tab[offset] & _PAGE_PSE )
-        return &tab[offset];
 
-    pt_mfn = pte_to_mfn(tab[offset]);
-    tab = mfn_to_virt(pt_mfn);
+    pt_mfn = virt_to_mfn(pte);
+    pt_pfn = virt_to_pfn(alloc_page());
+    if ( !pt_pfn )
+        return -1;
+    idx = idx_from_va_lvl(va, lvl);
+    new_pt_frame(&pt_pfn, pt_mfn, idx, lvl - 1);
 
-    offset = l1_table_offset(va);
-    return &tab[offset];
+    return 0;
+}
+
+pgentry_t *need_pgt(unsigned long va)
+{
+    pgentry_t *tab = NULL;
+
+    walk_pt(va, va, need_pgt_func, &tab);
+    return tab;
 }
 EXPORT_SYMBOL(need_pgt);
 
-- 
2.43.0
Re: [PATCH v2 2/3] mini-os: mm: switch need_pgt() to use walk_pt()
Posted by Samuel Thibault 3 months ago
Juergen Gross, le mar. 13 août 2024 15:41:57 +0200, a ecrit:
> Instead of open coding a page table walk, use walk_pt() in need_pgt().
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - add comment and ASSERT() (Samuel Thibault)
> ---
>  arch/x86/mm.c | 72 +++++++++++++++++++++------------------------------
>  1 file changed, 30 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index 9849b985..84a6d7f0 100644
> --- a/arch/x86/mm.c
> +++ b/arch/x86/mm.c
> @@ -523,57 +523,45 @@ static pgentry_t *get_pgt(unsigned long va)
>   * return a valid PTE for a given virtual address. If PTE does not exist,
>   * allocate page-table pages.
>   */
> -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)
>  {
> +    pgentry_t **result = par;
>      unsigned long pt_mfn;
> -    pgentry_t *tab;
>      unsigned long pt_pfn;
> -    unsigned offset;
> +    unsigned int idx;
>  
> -    tab = pt_base;
> -    pt_mfn = virt_to_mfn(pt_base);
> +    if ( !is_leaf )
> +        return 0;
>  
> -#if defined(__x86_64__)
> -    offset = l4_table_offset(va);
> -    if ( !(tab[offset] & _PAGE_PRESENT) )
> -    {
> -        pt_pfn = virt_to_pfn(alloc_page());
> -        if ( !pt_pfn )
> -            return NULL;
> -        new_pt_frame(&pt_pfn, pt_mfn, offset, L3_FRAME);
> -    }
> -    ASSERT(tab[offset] & _PAGE_PRESENT);
> -    pt_mfn = pte_to_mfn(tab[offset]);
> -    tab = mfn_to_virt(pt_mfn);
> -#endif
> -    offset = l3_table_offset(va);
> -    if ( !(tab[offset] & _PAGE_PRESENT) ) 
> -    {
> -        pt_pfn = virt_to_pfn(alloc_page());
> -        if ( !pt_pfn )
> -            return NULL;
> -        new_pt_frame(&pt_pfn, pt_mfn, offset, L2_FRAME);
> -    }
> -    ASSERT(tab[offset] & _PAGE_PRESENT);
> -    pt_mfn = pte_to_mfn(tab[offset]);
> -    tab = mfn_to_virt(pt_mfn);
> -    offset = l2_table_offset(va);
> -    if ( !(tab[offset] & _PAGE_PRESENT) )
> +    if ( lvl == L1_FRAME || (*pte & _PAGE_PRESENT) )
>      {
> -        pt_pfn = virt_to_pfn(alloc_page());
> -        if ( !pt_pfn )
> -            return NULL;
> -        new_pt_frame(&pt_pfn, pt_mfn, offset, L1_FRAME);
> +        /*
> +         * The PTE is not addressing a page table (is_leaf is true). If we are
> +         * either at the lowest level or we have a valid large page, we don't
> +         * need to allocate a page table.
> +         */
> +        ASSERT(lvl == L1_FRAME || (*pte & _PAGE_PSE));
> +        *result = pte;
> +        return 1;
>      }
> -    ASSERT(tab[offset] & _PAGE_PRESENT);
> -    if ( tab[offset] & _PAGE_PSE )
> -        return &tab[offset];
>  
> -    pt_mfn = pte_to_mfn(tab[offset]);
> -    tab = mfn_to_virt(pt_mfn);
> +    pt_mfn = virt_to_mfn(pte);
> +    pt_pfn = virt_to_pfn(alloc_page());
> +    if ( !pt_pfn )
> +        return -1;
> +    idx = idx_from_va_lvl(va, lvl);
> +    new_pt_frame(&pt_pfn, pt_mfn, idx, lvl - 1);
>  
> -    offset = l1_table_offset(va);
> -    return &tab[offset];
> +    return 0;
> +}
> +
> +pgentry_t *need_pgt(unsigned long va)
> +{
> +    pgentry_t *tab = NULL;
> +
> +    walk_pt(va, va, need_pgt_func, &tab);
> +    return tab;
>  }
>  EXPORT_SYMBOL(need_pgt);
>  
> -- 
> 2.43.0
> 

-- 
Samuel
<N> je déteste import
<N> parce que lorsque tu fais du python et que tu oublies le #!/bin/env python et que tu mets le fichier exécutable
<N> import est exécuté
 -+- #ens-mim - pourquoi mon script python change le curseur de la souris ?! -+-