Hello all. [Sorry for the possible format issues]
On Tue, Dec 22, 2020 at 12:41 PM Andrew Cooper <andrew.coop...@citrix.com> wrote: > On 21/12/2020 08:10, Jan Beulich wrote: > > On 17.12.2020 20:18, Andrew Cooper wrote: > >> On 15/12/2020 16:26, Jan Beulich wrote: > >>> This is together with its only caller, xenmem_add_to_physmap_one(). > >> I can't parse this sentence. Perhaps "... as is it's only caller," as a > >> follow-on from the subject sentence. > >> > >>> Move > >>> the latter next to p2m_add_foreign(), allowing this one to become > static > >>> at the same time. > >>> > >>> Signed-off-by: Jan Beulich <jbeul...@suse.com> > >> Acked-by: Andrew Cooper <andrew.coop...@citrix.com> > > So I had to ask Andrew to revert this (I was already at home when > > noticing the breakage), as it turned out to break the shim build. > > The problem is that xenmem_add_to_physmap() is non-static and > > hence can't be eliminated altogether by the compiler when !HVM. > > We could make the function conditionally static > > "#if !defined(CONFIG_X86) && !defined(CONFIG_HVM)", but this > > looks uglier to me than this extra hunk: > > > > --- unstable.orig/xen/common/memory.c > > +++ unstable/xen/common/memory.c > > @@ -788,7 +788,11 @@ int xenmem_add_to_physmap(struct domain > > union add_to_physmap_extra extra = {}; > > struct page_info *pages[16]; > > > > - ASSERT(paging_mode_translate(d)); > > + if ( !paging_mode_translate(d) ) > > + { > > + ASSERT_UNREACHABLE(); > > + return -EACCES; > > + } > > > > if ( xatp->space == XENMAPSPACE_gmfn_foreign ) > > extra.foreign_domid = DOMID_INVALID; > > > > Andrew, please let me know whether your ack stands with this (or > > said alternative) added, or whether you'd prefer me to re-post. > > Yeah, this is probably neater than the ifdefary. My ack stands. > > ~Andrew > I might miss something or did incorrect tests, but ... ... trying to build current staging (7ba2ab495be54f608cb47440e1497b2795bd301a) for x86 (with # CONFIG_HVM is not set) I got the following: /media/b/build/build/tmp/work/x86_64-xt-linux/domd-image-weston/1.0-r0/repo/build/tmp/work/aarch64-poky-linux/xen/4.14.0+gitAUTOINC+2c6e5a8ceb-r0/git/xen/common/memory.c:941: undefined reference to `xenmem_add_to_physmap_one' /media/b/build/build/tmp/work/x86_64-xt-linux/domd-image-weston/1.0-r0/repo/build/tmp/work/aarch64-poky-linux/xen/4.14.0+gitAUTOINC+2c6e5a8ceb-r0/git/xen/common/memory.c:941:(.text+0x1e391): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `xenmem_add_to_physmap_one' ld: /media/b/build/build/tmp/work/x86_64-xt-linux/domd-image-weston/1.0-r0/repo/build/tmp/work/aarch64-poky-linux/xen/4.14.0+gitAUTOINC+2c6e5a8ceb-r0/git/xen/.xen-syms.0: hidden symbol `xenmem_add_to_physmap_one' isn't defined ld: final link failed: Bad value It is worth mentioning that I do not use pvshim_defconfig (I disable HVM support via menuconfig manually before building). -- Regards, Oleksandr Tyshchenko