[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/6] x86/mm: p2m_add_foreign() is HVM-only




On 05.01.21 10:48, Jan Beulich wrote:

Hi Jan


On 04.01.2021 17:57, Oleksandr Tyshchenko wrote:
Hello all.

[Sorry for the possible format issues]

On Tue, Dec 22, 2020 at 12:41 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
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 <jbeulich@xxxxxxxx>
Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
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).
The specific .config may matter. The specific compiler version may
also matter. Things work fine for me, both for the shim config and
a custom !HVM one, with gcc10.

ok, after updating my a little bit ancient compiler to the latest possible (?) on xenial gcc-9 the build issue had gone away. Sorry for the noise.



--
Regards,

Oleksandr Tyshchenko




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.