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
