Re: [PATCH] x86/mem_sharing: Fix build folloing VM Fork work

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Apr 9, 2020 at 6:58 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> On 10/04/2020 00:23, Tamas K Lengyel wrote:
> > On Thu, Apr 9, 2020 at 5:00 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> > wrote:
> >> On 09/04/2020 23:38, Tamas K Lengyel wrote:
> >>> On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> >>> wrote:
> >>>> On 09/04/2020 22:24, Tamas K Lengyel wrote:
> >>>>> On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper 
> >>>>> <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>>>> A default build fails with:
> >>>>>>
> >>>>>>   mem_sharing.c: In function ‘copy_special_pages’:
> >>>>>>   mem_sharing.c:1649:9: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first 
> >>>>>> use in this function)
> >>>>>>            HVM_PARAM_STORE_PFN,
> >>>>>>            ^~~~~~~~~~~~~~~~~~~
> >>>>>>
> >>>>>> I suspect this is a rebasing issue with respect to c/s 12f0c69f2709 
> >>>>>> "x86/HVM:
> >>>>>> reduce hvm.h include dependencies".
> >>>>>>
> >>>>>> Fixes: 41548c5472a "mem_sharing: VM forking"
> >>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >>>>> So staging definitely compiles for me both with and without
> >>>>> CONFIG_MEM_SHARING enabled. By default its off, so this shouldn't even
> >>>>> be compiled so I'm curious what's happening in your build. That said,
> >>>>> I have no objection to the extra include if it turns out to be
> >>>>> actually necessary.
> >>>> Exact config attached.  I've just double checked that staging fails.
> >>>>
> >>>> We should get  to the bottom of exactly what is going on here, if it
> >>>> isn't the obvious thing.
> >>> Strange, with your config it does produce the error. With "echo
> >>> CONFIG_MEM_SHARING=y > .config && XEN_CONFIG_EXPERT=y make
> >>> olddefconfig && make" it does compile.
> >> You lose XEN_CONFIG_EXPERT=y in the second make, so it rewrites your
> >> .config behind your back.  (This behaviour is totally obnoxious).
> > I also compiled with export XEN_CONFIG_EXPERT=y and it compiles fine.
> >
> >> Diff the current config against original?
> > drt@t0:~/workspace/xen/xen$ diff .config .config-debug
> > 6c6
> > < CONFIG_GCC_VERSION=80300
> > ---
> >> CONFIG_GCC_VERSION=60300
>
> ;-(
>
> I occasionally forget that `diff` defaults to unreadable.
>
> >> # CONFIG_XEN_ALIGN_DEFAULT is not set
> >> CONFIG_XEN_ALIGN_2M=y
> >> CONFIG_GUEST=y
> >> CONFIG_XEN_GUEST=y
> >> CONFIG_PVH_GUEST=y
> >> CONFIG_PV_SHIM=y
> >> # CONFIG_PV_SHIM_EXCLUSIVE is not set
> >> CONFIG_HYPERV_GUEST=y
> > 61,62c65,74
> > < # CONFIG_XSM is not set
>
> With XSM compiled out, xsm/dummy.h is used, and has to include
> public/hvm/params.h to get XEN_ALTP2M_* for xsm_hvm_altp2mhvm_op(), and
> this bleeds through into everything which includes xsm/xsm.h, which is
> basically everything.
>
> Are you happy for me to fix up the commit message to this effect,
> replacing the previous guesswork?

Of course. Thanks for deciphering this.

Acked-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>