[v1,13/18] x86: generalize physmap logic

Commit Message

Daniel P. Smith July 6, 2022, 9:04 p.m. UTC

The existing physmap code is specific to dom0. In this commit, the dom0 physmap
code is generalized for any domain and functions are renamed to reflect their
new general nature.

Signed-off-by: Daniel P. Smith <[e-mail]>
Reviewed-by: Christopher Clark <[e-mail]>
---
 xen/arch/x86/include/asm/dom0_build.h |  2 +-
 xen/arch/x86/pv/dom0_build.c          | 10 +++++-----
 xen/arch/x86/pv/shim.c                |  4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Jan Beulich July 27, 2022, 12:33 p.m. UTC | #1

On 06.07.2022 23:04, Daniel P. Smith wrote:
> The existing physmap code is specific to dom0.

I think this needs better wording. Either you name the function or you
explain what piece of code you're talking about. "physmap" alone is
just not meaningful enough. (Also applies to the title.)

> --- a/xen/arch/x86/include/asm/dom0_build.h
> +++ b/xen/arch/x86/include/asm/dom0_build.h
> @@ -21,7 +21,7 @@ int dom0_construct_pvh(struct boot_domain *bd);
>  unsigned long dom0_paging_pages(const struct domain *d,
>                                  unsigned long nr_pages);
>  
> -void dom0_update_physmap(bool compat, unsigned long pfn,
> +void dom_update_physmap(bool compat, unsigned long pfn,
>                           unsigned long mfn, unsigned long vphysmap_s);

So my initial inclination was to suggest domain_ as a name prefix,
matching what we have elsewhere. But when we're already giving the
thing a new name, its PV-only nature also wants expressing. Hence
I'd like to suggest pv_update_physmap(). And then please fix
indentation of the continuation lines here and below.

> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -34,8 +34,8 @@
>  #define L3_PROT (BASE_PROT|_PAGE_DIRTY)
>  #define L4_PROT (BASE_PROT|_PAGE_DIRTY)
>  
> -void __init dom0_update_physmap(bool compat, unsigned long pfn,
> -                                unsigned long mfn, unsigned long vphysmap_s)
> +void __init dom_update_physmap(
> +    bool compat, unsigned long pfn, unsigned long mfn, unsigned long vphysmap_s)
>  {

Personally I dislike this further change to re-flow the parameter
list, as I see no particular reason for doing so.

Jan

Patch

diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index 571b25ea71..f30e4b860a 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -21,7 +21,7 @@  int dom0_construct_pvh(struct boot_domain *bd);
 unsigned long dom0_paging_pages(const struct domain *d,
                                 unsigned long nr_pages);
 
-void dom0_update_physmap(bool compat, unsigned long pfn,
+void dom_update_physmap(bool compat, unsigned long pfn,
                          unsigned long mfn, unsigned long vphysmap_s);
 
 #endif	/* _DOM0_BUILD_H_ */
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 78ebb03b1b..f1ea0575f0 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -34,8 +34,8 @@ 
 #define L3_PROT (BASE_PROT|_PAGE_DIRTY)
 #define L4_PROT (BASE_PROT|_PAGE_DIRTY)
 
-void __init dom0_update_physmap(bool compat, unsigned long pfn,
-                                unsigned long mfn, unsigned long vphysmap_s)
+void __init dom_update_physmap(
+    bool compat, unsigned long pfn, unsigned long mfn, unsigned long vphysmap_s)
 {
     if ( !compat )
         ((unsigned long *)vphysmap_s)[pfn] = mfn;
@@ -815,7 +815,7 @@  int __init dom0_construct_pv(struct boot_domain *bd)
         if ( pfn > REVERSE_START && (vinitrd_start || pfn < initrd_pfn) )
             mfn = alloc_epfn - (pfn - REVERSE_START);
 #endif
-        dom0_update_physmap(compat, pfn, mfn, vphysmap_start);
+        dom_update_physmap(compat, pfn, mfn, vphysmap_start);
         if ( !(pfn & 0xfffff) )
             process_pending_softirqs();
     }
@@ -831,7 +831,7 @@  int __init dom0_construct_pv(struct boot_domain *bd)
                  !get_page_and_type(page, d, PGT_writable_page) )
                 BUG();
 
-            dom0_update_physmap(compat, pfn, mfn, vphysmap_start);
+            dom_update_physmap(compat, pfn, mfn, vphysmap_start);
             ++pfn;
             if ( !(pfn & 0xfffff) )
                 process_pending_softirqs();
@@ -851,7 +851,7 @@  int __init dom0_construct_pv(struct boot_domain *bd)
 #ifndef NDEBUG
 #define pfn (nr_pages - 1 - (pfn - (alloc_epfn - alloc_spfn)))
 #endif
-            dom0_update_physmap(compat, pfn, mfn, vphysmap_start);
+            dom_update_physmap(compat, pfn, mfn, vphysmap_start);
 #undef pfn
             page++; pfn++;
             if ( !(pfn & 0xfffff) )
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 2ee290a392..fb2a7ef393 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -210,7 +210,7 @@  void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
     {                                                                          \
         share_xen_page_with_guest(mfn_to_page(_mfn(param)), d, SHARE_rw);      \
         replace_va_mapping(d, l4start, va, _mfn(param));                       \
-        dom0_update_physmap(compat,                                            \
+        dom_update_physmap(compat,                                             \
                             PFN_DOWN((va) - va_start), param, vphysmap);       \
     }                                                                          \
     else                                                                       \
@@ -238,7 +238,7 @@  void __init pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
         si->console.domU.mfn = mfn_x(console_mfn);
         share_xen_page_with_guest(mfn_to_page(console_mfn), d, SHARE_rw);
         replace_va_mapping(d, l4start, console_va, console_mfn);
-        dom0_update_physmap(compat, (console_va - va_start) >> PAGE_SHIFT,
+        dom_update_physmap(compat, (console_va - va_start) >> PAGE_SHIFT,
                             mfn_x(console_mfn), vphysmap);
         consoled_set_ring_addr(page);
     }