| Message ID | 4b842581-e24d-6b74-eef5-7ac48f0ff0a4@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/p2m: restrict more code to build just for HVM | expand |
On Mon, Apr 12, 2021 at 04:05:41PM +0200, Jan Beulich wrote: > Extend a respective #ifdef from inside set_typed_p2m_entry() to around > all three functions. Add ASSERT_UNREACHABLE() to the latter one's safety > check path. Wouldn't it be better to also move the prototypes in p2m.h into a CONFIG_HVM guarded region, so that it fails at compile time rather than link time? Thanks, Roger.
On 29.04.2021 15:17, Roger Pau Monné wrote: > On Mon, Apr 12, 2021 at 04:05:41PM +0200, Jan Beulich wrote: >> Extend a respective #ifdef from inside set_typed_p2m_entry() to around >> all three functions. Add ASSERT_UNREACHABLE() to the latter one's safety >> check path. > > Wouldn't it be better to also move the prototypes in p2m.h into a > CONFIG_HVM guarded region, so that it fails at compile time rather > than link time? In the header I'm fearing this ending up as spaghetti more than in the .c file. I think where possible we may want to do so once we have a clear / clean set of APIs which are generic vs such which are HVM-specific (which I expect to be the case once p2m.c as a whole becomes HVM-only). Jan
On Thu, Apr 29, 2021 at 04:09:30PM +0200, Jan Beulich wrote: > On 29.04.2021 15:17, Roger Pau Monné wrote: > > On Mon, Apr 12, 2021 at 04:05:41PM +0200, Jan Beulich wrote: > >> Extend a respective #ifdef from inside set_typed_p2m_entry() to around > >> all three functions. Add ASSERT_UNREACHABLE() to the latter one's safety > >> check path. > > > > Wouldn't it be better to also move the prototypes in p2m.h into a > > CONFIG_HVM guarded region, so that it fails at compile time rather > > than link time? > > In the header I'm fearing this ending up as spaghetti more than in > the .c file. I would just move them all into a common CONFIG_HVM section rather than adding CONFIG_HVM guards around each of them. > I think where possible we may want to do so once we > have a clear / clean set of APIs which are generic vs such which > are HVM-specific (which I expect to be the case once p2m.c as a > whole becomes HVM-only). Oh, I would certainly like p2m.c to be HVM-only (see my comment about introducing a p2m-hvm.c). Anyway, my only comment was about the header, so: Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
--- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1260,6 +1260,8 @@ int p2m_finish_type_change(struct domain return rc; } +#ifdef CONFIG_HVM + /* * Returns: * 0 for success @@ -1280,7 +1282,10 @@ static int set_typed_p2m_entry(struct do struct p2m_domain *p2m = p2m_get_hostp2m(d); if ( !paging_mode_translate(d) ) + { + ASSERT_UNREACHABLE(); return -EIO; + } gfn_lock(p2m, gfn, order); omfn = p2m->get_entry(p2m, gfn, &ot, &a, 0, &cur_order, NULL); @@ -1313,7 +1318,6 @@ static int set_typed_p2m_entry(struct do if ( rc ) gdprintk(XENLOG_ERR, "p2m_set_entry: %#lx:%u -> %d (0x%"PRI_mfn")\n", gfn_l, order, rc, mfn_x(mfn)); -#ifdef CONFIG_HVM else if ( p2m_is_pod(ot) ) { pod_lock(p2m); @@ -1321,7 +1325,6 @@ static int set_typed_p2m_entry(struct do BUG_ON(p2m->pod.entry_count < 0); pod_unlock(p2m); } -#endif gfn_unlock(p2m, gfn, order); return rc; @@ -1349,6 +1352,8 @@ int set_mmio_p2m_entry(struct domain *d, p2m_get_hostp2m(d)->default_access); } +#endif /* CONFIG_HVM */ + int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l, p2m_access_t p2ma, unsigned int flag) {
Extend a respective #ifdef from inside set_typed_p2m_entry() to around all three functions. Add ASSERT_UNREACHABLE() to the latter one's safety check path. Signed-off-by: Jan Beulich <jbeulich@suse.com>