Re: [PATCH 01/16] x86/P2M: rename p2m_remove_page()

[George Dunlap <dunlapg@xxxxxxxxx>]

On Mon, Jul 5, 2021 at 5:05 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:

This is in preparation to re-using the original name.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Hey Jan,

This series overall looks good; thanks for taking this on.

Functionally this patch looks good; just one question...

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -788,8 +788,8 @@ void p2m_final_teardown(struct domain *d
 #ifdef CONFIG_HVM

 static int __must_check
-p2m_remove_page(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
-                unsigned int page_order)
+p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
+                 unsigned int page_order)

One question that's naturally raised for both this and the following patch is, what is the new naming "scheme" for these renamed functions, and how do they relate to the old scheme?

Overall it seems like the intention is that "guest_physmap_..." can be called on a domain which may be PV or HVM, while "p2m_..." should only be called on HVM domains.

There's also "..._entry" vs "..._page".  Is the p2m_remove_page / p2m_remove_entry distinction have a meaning, and is it the same meaning as guest_physmap_add_page / guest_physmap_add_entry?  Or is it similar to p2m_init_nestedp2m / p2m_nestedp2m_init -- we need both functions and  don't want to make the names longer?

 -George