| Message ID | 55643e68-432a-116a-b68e-2200e364e5da@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/mm: large parts of P2M code and struct p2m_domain are HVM-only | expand |
On Mon, Jul 5, 2021 at 5:06 PM Jan Beulich <jbeulich@suse.com> wrote: > This is to make it easier to see which parts of p2m.c still aren't HVM- > specific: In one case the conditionals sat in an already guarded region, > while in the other case P2M_AUDIT implies HVM. > I think this would be much more easy to understand what's going on if it was more like this: --- x86/p2m: P2M_AUDIT implies CONFIG_HVM Remove one #endif / #ifdef CONFIG_HVM pair to make all the audit code CONFIG_HVM only. This is to make it easier to see which parts of p2m.c still aren't HVM-specific. While here, remove a redundant set of nested #ifdef CONFIG_HVM. --- Reviewed-by: George Dunlap <george.dunlap@citrix.com>
On 04.02.2022 23:13, George Dunlap wrote: > On Mon, Jul 5, 2021 at 5:06 PM Jan Beulich <jbeulich@suse.com> wrote: > >> This is to make it easier to see which parts of p2m.c still aren't HVM- >> specific: In one case the conditionals sat in an already guarded region, >> while in the other case P2M_AUDIT implies HVM. >> > > I think this would be much more easy to understand what's going on if it > was more like this: > > --- > x86/p2m: P2M_AUDIT implies CONFIG_HVM > > Remove one #endif / #ifdef CONFIG_HVM pair to make all the audit code > CONFIG_HVM only. This is to make it easier to see which parts of p2m.c > still aren't HVM-specific. > > While here, remove a redundant set of nested #ifdef CONFIG_HVM. > --- > > Reviewed-by: George Dunlap <george.dunlap@citrix.com> Thanks. Unless you tell me otherwise I'll assume the changed title and description are merely a suggestion, not a requirement for your R-b to apply. I continue to like my variant better; in particular I'd like to not mention P2M_AUDIT in the title and this way avoid "While here, ..." or alike. Jan
--- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1584,11 +1584,10 @@ p2m_flush_table_locked(struct p2m_domain * when discarding them. */ ASSERT(!p2m_is_hostp2m(p2m)); -#ifdef CONFIG_HVM - /* Nested p2m's do not do pod, hence the asserts (and no pod lock)*/ + + /* Nested p2m's do not do pod, hence the asserts (and no pod lock) */ ASSERT(page_list_empty(&p2m->pod.super)); ASSERT(page_list_empty(&p2m->pod.single)); -#endif /* No need to flush if it's already empty */ if ( p2m_is_nestedp2m(p2m) && p2m->np2m_base == P2M_BASE_EADDR ) @@ -2497,7 +2496,6 @@ int p2m_altp2m_propagate_change(struct d return ret; } -#endif /* CONFIG_HVM */ /*** Audit ***/ @@ -2603,8 +2601,6 @@ out_p2m_audit: } #endif /* P2M_AUDIT */ -#ifdef CONFIG_HVM - /* * Add frame from foreign domain to target domain's physmap. Similar to * XENMAPSPACE_gmfn but the frame is foreign being mapped into current,
This is to make it easier to see which parts of p2m.c still aren't HVM- specific: In one case the conditionals sat in an already guarded region, while in the other case P2M_AUDIT implies HVM. Signed-off-by: Jan Beulich <jbeulich@suse.com>