Message ID | 20200409204837.7017-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/mem_sharing: Fix build folloing VM Fork work | expand |
On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> 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. Tamas
On 09/04/2020 22:24, Tamas K Lengyel wrote: > On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> > 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. ~Andrew # # Automatically generated file; DO NOT EDIT. # Xen/x86 4.14-unstable Configuration # CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=60300 CONFIG_CLANG_VERSION=0 CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_INDIRECT_THUNK=y # # Architecture Features # CONFIG_NR_CPUS=256 CONFIG_PV=y CONFIG_PV_LINEAR_PT=y CONFIG_HVM=y CONFIG_SHADOW_PAGING=y # CONFIG_BIGMEM is not set CONFIG_HVM_FEP=y CONFIG_TBOOT=y # 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 CONFIG_MEM_SHARING=y # end of Architecture Features # # Common Features # CONFIG_COMPAT=y CONFIG_CORE_PARKING=y CONFIG_GRANT_TABLE=y CONFIG_HAS_ALTERNATIVE=y CONFIG_HAS_EX_TABLE=y CONFIG_HAS_FAST_MULTIPLY=y CONFIG_MEM_ACCESS_ALWAYS_ON=y CONFIG_MEM_ACCESS=y CONFIG_HAS_MEM_PAGING=y CONFIG_HAS_PDX=y CONFIG_HAS_UBSAN=y CONFIG_HAS_KEXEC=y CONFIG_HAS_IOPORTS=y CONFIG_HAS_SCHED_GRANULARITY=y CONFIG_NEEDS_LIBELF=y # # Speculative hardening # CONFIG_SPECULATIVE_HARDEN_ARRAY=y CONFIG_SPECULATIVE_HARDEN_BRANCH=y # end of Speculative hardening CONFIG_KEXEC=y # CONFIG_EFI_SET_VIRTUAL_ADDRESS_MAP is not set CONFIG_XENOPROF=y CONFIG_XSM=y CONFIG_XSM_FLASK=y CONFIG_XSM_FLASK_AVC_STATS=y CONFIG_XSM_FLASK_POLICY=y CONFIG_XSM_SILO=y # CONFIG_XSM_DUMMY_DEFAULT is not set # CONFIG_XSM_FLASK_DEFAULT is not set CONFIG_XSM_SILO_DEFAULT=y # CONFIG_LATE_HWDOM is not set CONFIG_ARGO=y # # Schedulers # CONFIG_SCHED_CREDIT=y CONFIG_SCHED_CREDIT2=y CONFIG_SCHED_RTDS=y CONFIG_SCHED_ARINC653=y CONFIG_SCHED_NULL=y CONFIG_SCHED_CREDIT_DEFAULT=y # CONFIG_SCHED_CREDIT2_DEFAULT is not set # CONFIG_SCHED_RTDS_DEFAULT is not set # CONFIG_SCHED_ARINC653_DEFAULT is not set # CONFIG_SCHED_NULL_DEFAULT is not set CONFIG_SCHED_DEFAULT="credit" # end of Schedulers CONFIG_CRYPTO=y CONFIG_LIVEPATCH=y CONFIG_FAST_SYMBOL_LOOKUP=y CONFIG_ENFORCE_UNIQUE_SYMBOLS=y CONFIG_CMDLINE="" CONFIG_DOM0_MEM="" CONFIG_TRACEBUFFER=y # end of Common Features # # Device Drivers # CONFIG_ACPI=y CONFIG_ACPI_LEGACY_TABLES_LOOKUP=y CONFIG_NUMA=y CONFIG_HAS_NS16550=y CONFIG_HAS_EHCI=y CONFIG_HAS_CPUFREQ=y CONFIG_HAS_PASSTHROUGH=y CONFIG_HAS_PCI=y CONFIG_VIDEO=y CONFIG_VGA=y CONFIG_HAS_VPCI=y # end of Device Drivers # # Deprecated Functionality # # CONFIG_PV_LDT_PAGING is not set # end of Deprecated Functionality CONFIG_EXPERT="y" CONFIG_ARCH_SUPPORTS_INT128=y # # Debugging Options # CONFIG_DEBUG=y # CONFIG_CRASH_DEBUG is not set CONFIG_GDBSX=y CONFIG_DEBUG_INFO=y CONFIG_FRAME_POINTER=y # CONFIG_DEBUG_LOCK_PROFILE is not set CONFIG_DEBUG_LOCKS=y # CONFIG_PERF_COUNTERS is not set CONFIG_VERBOSE_DEBUG=y CONFIG_SCRUB_DEBUG=y # CONFIG_UBSAN is not set # CONFIG_DEBUG_TRACE is not set CONFIG_XMEM_POOL_POISON=y # end of Debugging Options
On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 09/04/2020 22:24, Tamas K Lengyel wrote: > > On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> > > 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. Tamas
On 09/04/2020 23:38, Tamas K Lengyel wrote: > On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> On 09/04/2020 22:24, Tamas K Lengyel wrote: >>> On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> >>> 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). Diff the current config against original? ~Andrew
On Thu, Apr 9, 2020 at 5:00 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 09/04/2020 23:38, Tamas K Lengyel wrote: > > On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >> On 09/04/2020 22:24, Tamas K Lengyel wrote: > >>> On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> > >>> 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 25,28c25,32 < CONFIG_XEN_ALIGN_DEFAULT=y < # CONFIG_XEN_ALIGN_2M is not set < # CONFIG_XEN_GUEST is not set < # CONFIG_HYPERV_GUEST is not set --- > # 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 < # CONFIG_ARGO is not set --- > CONFIG_XSM=y > CONFIG_XSM_FLASK=y > CONFIG_XSM_FLASK_AVC_STATS=y > CONFIG_XSM_FLASK_POLICY=y > CONFIG_XSM_SILO=y > # CONFIG_XSM_DUMMY_DEFAULT is not set > # CONFIG_XSM_FLASK_DEFAULT is not set > CONFIG_XSM_SILO_DEFAULT=y > # CONFIG_LATE_HWDOM is not set > CONFIG_ARGO=y 72,73c84,85 < # CONFIG_SCHED_CREDIT_DEFAULT is not set < CONFIG_SCHED_CREDIT2_DEFAULT=y --- > CONFIG_SCHED_CREDIT_DEFAULT=y > # CONFIG_SCHED_CREDIT2_DEFAULT is not set 77c89 < CONFIG_SCHED_DEFAULT="credit2" --- > CONFIG_SCHED_DEFAULT="credit" Tamas
On 10/04/2020 00:23, Tamas K Lengyel wrote: > On Thu, Apr 9, 2020 at 5:00 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> On 09/04/2020 23:38, Tamas K Lengyel wrote: >>> On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >>>> On 09/04/2020 22:24, Tamas K Lengyel wrote: >>>>> On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> >>>>> 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? ~Andrew
On Thu, Apr 9, 2020 at 6:58 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 10/04/2020 00:23, Tamas K Lengyel wrote: > > On Thu, Apr 9, 2020 at 5:00 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >> On 09/04/2020 23:38, Tamas K Lengyel wrote: > >>> On Thu, Apr 9, 2020 at 4:05 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >>>> On 09/04/2020 22:24, Tamas K Lengyel wrote: > >>>>> On Thu, Apr 9, 2020 at 2:48 PM Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com> > >>>>> 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@tklengyel.com>
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index 85951b7bea..e572e9e39d 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -41,6 +41,8 @@ #include <asm/hvm/hvm.h> #include <xsm/xsm.h> +#include <public/hvm/params.h> + #include "mm-locks.h" static shr_handle_t next_handle = 1;
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@citrix.com> --- CC: Tamas K Lengyel <tamas@tklengyel.com> CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/mm/mem_sharing.c | 2 ++ 1 file changed, 2 insertions(+)