| Message ID | 97e02ced-a5e4-a0d7-0435-124fff9f5dca@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: reduce include dependencies | expand |
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 10 March 2020 15:48 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné > <roger.pau@citrix.com>; Paul Durrant <paul@xen.org>; Daniel de Graaf <dgdegra@tycho.nsa.gov>; Tamas K > Lengyel <tamas@tklengyel.com>; Petre Pircalabu <ppircalabu@bitdefender.com>; Alexandru Isaila > <aisaila@bitdefender.com> > Subject: [PATCH v2 1/9] x86/HVM: reduce domain.h include dependencies > > Drop #include-s not needed by the header itself. Put the ones needed > into whichever other files actually need them. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Paul Durrant <paul@xen.org>
On 10/03/2020 15:48, Jan Beulich wrote: > Drop #include-s not needed by the header itself. Put the ones needed > into whichever other files actually need them. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > v2: Also make things build with XSM=y. Looking better, but still got problems. xen_pv_console.c: In function ‘pv_console_init’: xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first use in this function) r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, &raw_pfn); ^~~~~~~~~~~~~~~~~~~~~ and shim.c: In function ‘pv_shim_fixup_e820’: shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in this function) MARK_PARAM_RAM(HVM_PARAM_STORE_PFN); ^ ~Andrew
On 11.03.2020 14:09, Andrew Cooper wrote: > On 10/03/2020 15:48, Jan Beulich wrote: >> Drop #include-s not needed by the header itself. Put the ones needed >> into whichever other files actually need them. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> v2: Also make things build with XSM=y. > > Looking better, but still got problems. > > xen_pv_console.c: In function ‘pv_console_init’: > xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first > use in this function) > r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, &raw_pfn); > ^~~~~~~~~~~~~~~~~~~~~ > > and > > shim.c: In function ‘pv_shim_fixup_e820’: > shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in > this function) > MARK_PARAM_RAM(HVM_PARAM_STORE_PFN); Oh, so that's an XSM+shim config aiui; Among the sets of what I regularly test I have only an XSM one and a shim one. The funs of allowing too wide a variety of different configs ... In cases like this series here I really don't see how one is supposed to cover _all_ possible configs; randconfig builds won't help with this unless one would let them run until they've covered all possible ones. Jan
On 11/03/2020 15:22, Jan Beulich wrote: > On 11.03.2020 14:09, Andrew Cooper wrote: >> On 10/03/2020 15:48, Jan Beulich wrote: >>> Drop #include-s not needed by the header itself. Put the ones needed >>> into whichever other files actually need them. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> --- >>> v2: Also make things build with XSM=y. >> Looking better, but still got problems. >> >> xen_pv_console.c: In function ‘pv_console_init’: >> xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first >> use in this function) >> r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, &raw_pfn); >> ^~~~~~~~~~~~~~~~~~~~~ >> >> and >> >> shim.c: In function ‘pv_shim_fixup_e820’: >> shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in >> this function) >> MARK_PARAM_RAM(HVM_PARAM_STORE_PFN); > Oh, so that's an XSM+shim config aiui; Among the sets of what > I regularly test I have only an XSM one and a shim one. The funs > of allowing too wide a variety of different configs ... In cases > like this series here I really don't see how one is supposed to > cover _all_ possible configs; randconfig builds won't help with > this unless one would let them run until they've covered all > possible ones. This happens to be my default config. I wouldn't expect to be perfect even under weird combinations, but these aren't weird, and this is the kind of change that `make allyesconfig` is intended for. ~Andrew
On 11.03.2020 14:09, Andrew Cooper wrote: > On 10/03/2020 15:48, Jan Beulich wrote: >> Drop #include-s not needed by the header itself. Put the ones needed >> into whichever other files actually need them. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> v2: Also make things build with XSM=y. > > Looking better, but still got problems. > > xen_pv_console.c: In function ‘pv_console_init’: > xen_pv_console.c:51:37: error: ‘HVM_PARAM_CONSOLE_PFN’ undeclared (first > use in this function) > r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, &raw_pfn); > ^~~~~~~~~~~~~~~~~~~~~ > > and > > shim.c: In function ‘pv_shim_fixup_e820’: > shim.c:148:20: error: ‘HVM_PARAM_STORE_PFN’ undeclared (first use in > this function) > MARK_PARAM_RAM(HVM_PARAM_STORE_PFN); In a later reply (which I've lost due to mailbox problems) you've been suggesting allyesconfig. I'd been considering to use it indeed, but then forgot. Together with choices though I'm unconvinced this would provide broad enough coverage. I'd also similarly wonder whether allnoconfig might not be more telling, as it might result in fewer things getting included here and there. I'll make sure both build fine before sending v3, but I'm having trouble seeing how I would invoke these - neither the top level Makefile nor xen/Makefile look to permit its use right now. Have you found a way to successfully use these without first patching the tree? Jan
--- a/xen/arch/x86/cpu/mcheck/vmce.c +++ b/xen/arch/x86/cpu/mcheck/vmce.c @@ -28,6 +28,7 @@ #include <xen/mm.h> #include <asm/hvm/save.h> #include <asm/processor.h> +#include <public/hvm/params.h> #include <public/sysctl.h> #include <asm/system.h> #include <asm/msr.h> --- a/xen/arch/x86/hvm/dm.c +++ b/xen/arch/x86/hvm/dm.c @@ -27,6 +27,8 @@ #include <xsm/xsm.h> +#include <public/hvm/hvm_op.h> + struct dmop_args { domid_t domid; unsigned int nr_bufs; --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -38,6 +38,7 @@ #include <public/arch-x86/hvm/start_info.h> #include <public/hvm/hvm_info_table.h> #include <public/hvm/hvm_vcpu.h> +#include <public/hvm/params.h> /* * Have the TSS cover the ISA port range, which makes it --- a/xen/arch/x86/hvm/hpet.c +++ b/xen/arch/x86/hvm/hpet.c @@ -26,6 +26,7 @@ #include <xen/event.h> #include <xen/trace.h> #include <xen/nospec.h> +#include <public/hvm/params.h> #define domain_vhpet(x) (&(x)->arch.hvm.pl_time->vhpet) #define vcpu_vhpet(x) (domain_vhpet((x)->domain)) --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -24,6 +24,9 @@ #include <asm/hvm/support.h> +#include <public/hvm/hvm_op.h> +#include <public/hvm/params.h> + static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) { const struct vcpu *curr = current; --- a/xen/arch/x86/hvm/ioreq.c +++ b/xen/arch/x86/hvm/ioreq.c @@ -34,6 +34,7 @@ #include <asm/hvm/vmx/vmx.h> #include <public/hvm/ioreq.h> +#include <public/hvm/params.h> static void set_ioreq_server(struct domain *d, unsigned int id, struct hvm_ioreq_server *s) --- a/xen/arch/x86/hvm/irq.c +++ b/xen/arch/x86/hvm/irq.c @@ -26,6 +26,7 @@ #include <asm/hvm/domain.h> #include <asm/hvm/support.h> #include <asm/msi.h> +#include <public/hvm/params.h> bool hvm_domain_use_pirq(const struct domain *d, const struct pirq *pirq) { --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -28,6 +28,7 @@ #include <asm/hvm/support.h> #include <asm/current.h> #include <xen/trace.h> +#include <public/hvm/params.h> #define USEC_PER_SEC 1000000UL #define NS_PER_USEC 1000UL --- a/xen/arch/x86/hvm/viridian/private.h +++ b/xen/arch/x86/hvm/viridian/private.h @@ -4,6 +4,7 @@ #define X86_HVM_VIRIDIAN_PRIVATE_H #include <asm/hvm/save.h> +#include <public/hvm/params.h> int viridian_synic_wrmsr(struct vcpu *v, uint32_t idx, uint64_t val); int viridian_synic_rdmsr(const struct vcpu *v, uint32_t idx, uint64_t *val); --- a/xen/arch/x86/hvm/vpt.c +++ b/xen/arch/x86/hvm/vpt.c @@ -22,6 +22,7 @@ #include <asm/event.h> #include <asm/apic.h> #include <asm/mc146818rtc.h> +#include <public/hvm/params.h> #define mode_is(d, name) \ ((d)->arch.hvm.params[HVM_PARAM_TIMER_MODE] == HVMPTM_##name) --- a/xen/arch/x86/msr.c +++ b/xen/arch/x86/msr.c @@ -28,6 +28,8 @@ #include <asm/msr.h> #include <asm/setup.h> +#include <public/hvm/params.h> + DEFINE_PER_CPU(uint32_t, tsc_aux); struct msr_policy __read_mostly raw_msr_policy, --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -76,6 +76,7 @@ #include <asm/hpet.h> #include <asm/vpmu.h> #include <public/arch-x86/cpuid.h> +#include <public/hvm/params.h> #include <asm/cpuid.h> #include <xsm/xsm.h> #include <asm/pv/traps.h> --- a/xen/common/vm_event.c +++ b/xen/common/vm_event.c @@ -29,6 +29,7 @@ #include <asm/monitor.h> #include <asm/vm_event.h> #include <xsm/xsm.h> +#include <public/hvm/params.h> /* for public/io/ring.h macros */ #define xen_mb() smp_mb() --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -20,20 +20,14 @@ #ifndef __ASM_X86_HVM_DOMAIN_H__ #define __ASM_X86_HVM_DOMAIN_H__ -#include <xen/iommu.h> -#include <asm/hvm/irq.h> -#include <asm/hvm/vpt.h> -#include <asm/hvm/vlapic.h> -#include <asm/hvm/vioapic.h> +#include <xen/list.h> +#include <xen/mm.h> +#include <xen/radix-tree.h> + #include <asm/hvm/io.h> -#include <asm/hvm/viridian.h> #include <asm/hvm/vmx/vmcs.h> #include <asm/hvm/svm/vmcb.h> -#include <asm/mem_sharing.h> -#include <public/grant_table.h> -#include <public/hvm/params.h> -#include <public/hvm/save.h> -#include <public/hvm/hvm_op.h> + #include <public/hvm/dm_op.h> struct hvm_ioreq_page { --- a/xen/include/asm-x86/hvm/nestedhvm.h +++ b/xen/include/asm-x86/hvm/nestedhvm.h @@ -22,6 +22,7 @@ #include <xen/types.h> /* for uintNN_t */ #include <xen/sched.h> /* for struct vcpu, struct domain */ #include <asm/hvm/vcpu.h> /* for vcpu_nestedhvm */ +#include <public/hvm/params.h> enum nestedhvm_vmexits { NESTEDHVM_VMEXIT_ERROR = 0, /* inject VMEXIT w/ invalid VMCB */ --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -17,6 +17,7 @@ #include <xen/sched.h> #include <xsm/xsm.h> +#include <public/hvm/params.h> /* Cannot use BUILD_BUG_ON here because the expressions we check are not * considered constant at compile time. Instead, rely on constant propagation to --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -28,7 +28,7 @@ #include <public/physdev.h> #include <public/platform.h> #include <public/version.h> - +#include <public/hvm/params.h> #include <public/xsm/flask_op.h> #include <avc.h>
Drop #include-s not needed by the header itself. Put the ones needed into whichever other files actually need them. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: Also make things build with XSM=y.