| Message ID | cc46049dbdf86cad33f6a9f7ae79851b54cecea1.1692275359.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: fix inclusions and static storage duration | expand |
On 17.08.2023 14:39, Nicola Vetrini wrote: > --- a/xen/include/xen/vga.h > +++ b/xen/include/xen/vga.h > @@ -15,4 +15,20 @@ > extern struct xen_vga_console_info vga_console_info; > #endif > > +int fill_console_start_info(struct dom0_vga_console_info *); > + > +#ifdef CONFIG_X86 > +void vesa_early_init(void); > +void vesa_endboot(bool_t keep); > +#else > +#define vesa_early_init() ((void)0) > +#define vesa_endboot(x) ((void)0) > +#endif > + > +#ifdef CONFIG_VIDEO > +void vesa_init(void); > +#else > +static inline void vesa_init(void) {}; > +#endif Hmm, on one hand you simply move existing code here. But then why don't you leverage the existing #ifdef? The more that it's more specific and in line with drivers/video/Makefile having obj-$(CONFIG_VGA) := vga.o and obj-$(CONFIG_VGA) += vesa.o Jan
On 17/08/2023 15:28, Jan Beulich wrote: > On 17.08.2023 14:39, Nicola Vetrini wrote: >> --- a/xen/include/xen/vga.h >> +++ b/xen/include/xen/vga.h >> @@ -15,4 +15,20 @@ >> extern struct xen_vga_console_info vga_console_info; >> #endif >> >> +int fill_console_start_info(struct dom0_vga_console_info *); >> + >> +#ifdef CONFIG_X86 >> +void vesa_early_init(void); >> +void vesa_endboot(bool_t keep); >> +#else >> +#define vesa_early_init() ((void)0) >> +#define vesa_endboot(x) ((void)0) >> +#endif >> + >> +#ifdef CONFIG_VIDEO >> +void vesa_init(void); >> +#else >> +static inline void vesa_init(void) {}; >> +#endif > > Hmm, on one hand you simply move existing code here. But then why don't > you leverage the existing #ifdef? The more that it's more specific and > in line with drivers/video/Makefile having > > obj-$(CONFIG_VGA) := vga.o > > and > > obj-$(CONFIG_VGA) += vesa.o > > Jan Are you saying that CONFIG_VGA implies CONFIG_VIDEO and therefore "#ifdef CONFIG_VGA" at line 14 of vga.h can be used instead of the #ifdefs inherited from the original locations to wrap all the declarations that are being moved?
On 17.08.2023 16:52, Nicola Vetrini wrote: > On 17/08/2023 15:28, Jan Beulich wrote: >> On 17.08.2023 14:39, Nicola Vetrini wrote: >>> --- a/xen/include/xen/vga.h >>> +++ b/xen/include/xen/vga.h >>> @@ -15,4 +15,20 @@ >>> extern struct xen_vga_console_info vga_console_info; >>> #endif >>> >>> +int fill_console_start_info(struct dom0_vga_console_info *); >>> + >>> +#ifdef CONFIG_X86 >>> +void vesa_early_init(void); >>> +void vesa_endboot(bool_t keep); >>> +#else >>> +#define vesa_early_init() ((void)0) >>> +#define vesa_endboot(x) ((void)0) >>> +#endif >>> + >>> +#ifdef CONFIG_VIDEO >>> +void vesa_init(void); >>> +#else >>> +static inline void vesa_init(void) {}; >>> +#endif >> >> Hmm, on one hand you simply move existing code here. But then why don't >> you leverage the existing #ifdef? The more that it's more specific and >> in line with drivers/video/Makefile having >> >> obj-$(CONFIG_VGA) := vga.o >> >> and >> >> obj-$(CONFIG_VGA) += vesa.o > > Are you saying that CONFIG_VGA implies CONFIG_VIDEO and therefore > "#ifdef CONFIG_VGA" > at line 14 of vga.h can be used instead of the #ifdefs inherited from > the original locations > to wrap all the declarations that are being moved? Yes - see drivers/video/Kconfig. Jan
>> >> Are you saying that CONFIG_VGA implies CONFIG_VIDEO and therefore >> "#ifdef CONFIG_VGA" >> at line 14 of vga.h can be used instead of the #ifdefs inherited from >> the original locations >> to wrap all the declarations that are being moved? > > Yes - see drivers/video/Kconfig. > > Jan Ok then. I guess I can make a standalone patch with this modification if the other patches of this series go in as is.
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h index b0e6a39e2365..dfdd9e555149 100644 --- a/xen/arch/x86/include/asm/setup.h +++ b/xen/arch/x86/include/asm/setup.h @@ -25,12 +25,6 @@ void subarch_init_memory(void); void init_IRQ(void); -#ifdef CONFIG_VIDEO -void vesa_init(void); -#else -static inline void vesa_init(void) {}; -#endif - int construct_dom0( struct domain *d, const module_t *image, unsigned long image_headroom, diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c index 9ff2da8fc324..9469de9045c7 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -14,7 +14,6 @@ #include <xen/event.h> #include <xen/domain_page.h> #include <xen/trace.h> -#include <xen/console.h> #include <xen/iocap.h> #include <xen/guest_access.h> #include <xen/hypercall.h> @@ -24,6 +23,7 @@ #include <xen/pmstat.h> #include <xen/irq.h> #include <xen/symbols.h> +#include <xen/vga.h> #include <asm/current.h> #include <public/platform.h> #include <acpi/cpufreq/processor_perf.h> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index 909ee9a899a4..5bbed3a36a21 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -4,7 +4,6 @@ * Copyright (c) 2002-2005, K A Fraser */ -#include <xen/console.h> #include <xen/domain.h> #include <xen/domain_page.h> #include <xen/init.h> @@ -13,6 +12,7 @@ #include <xen/pfn.h> #include <xen/sched.h> #include <xen/softirq.h> +#include <xen/vga.h> #include <asm/bzimage.h> #include <asm/dom0_build.h> diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c index 0a03508bee60..18b590cdf072 100644 --- a/xen/drivers/video/vga.c +++ b/xen/drivers/video/vga.c @@ -54,14 +54,6 @@ string_param("vga", opt_vga); static unsigned int columns, lines; #define ATTRIBUTE 7 -#ifdef CONFIG_X86 -void vesa_early_init(void); -void vesa_endboot(bool_t keep); -#else -#define vesa_early_init() ((void)0) -#define vesa_endboot(x) ((void)0) -#endif - void __init video_init(void) { char *p; diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h index 53c56191ba9e..ab5c30c0daf2 100644 --- a/xen/include/xen/console.h +++ b/xen/include/xen/console.h @@ -20,8 +20,6 @@ void console_init_postirq(void); void console_endboot(void); int console_has(const char *device); -int fill_console_start_info(struct dom0_vga_console_info *); - unsigned long console_lock_recursive_irqsave(void); void console_unlock_recursive_irqrestore(unsigned long flags); void console_force_unlock(void); diff --git a/xen/include/xen/vga.h b/xen/include/xen/vga.h index f72b63d446b1..1d53f0149433 100644 --- a/xen/include/xen/vga.h +++ b/xen/include/xen/vga.h @@ -15,4 +15,20 @@ extern struct xen_vga_console_info vga_console_info; #endif +int fill_console_start_info(struct dom0_vga_console_info *); + +#ifdef CONFIG_X86 +void vesa_early_init(void); +void vesa_endboot(bool_t keep); +#else +#define vesa_early_init() ((void)0) +#define vesa_endboot(x) ((void)0) +#endif + +#ifdef CONFIG_VIDEO +void vesa_init(void); +#else +static inline void vesa_init(void) {}; +#endif + #endif /* _XEN_VGA_H */
The declarations for 'vesa_{init,early_init,endboot}' needed by 'xen/drivers/video/vesa.c' and 'fill_console_start_info' in 'vga.c' are now available by moving the relative code inside 'vga.h'. The latter is moved from 'xen/console.h' because of its close relation with vga. This also resolves violations of MISRA C:2012 Rule 8.4. Fixes: 6d9199bd0f22 ("x86-64: enable hypervisor output on VESA frame buffer") Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- Changes in v2: - Moved fill_console_start_info to vga.h (21bee1787021 introduced this function) --- xen/arch/x86/include/asm/setup.h | 6 ------ xen/arch/x86/platform_hypercall.c | 2 +- xen/arch/x86/pv/dom0_build.c | 2 +- xen/drivers/video/vga.c | 8 -------- xen/include/xen/console.h | 2 -- xen/include/xen/vga.h | 16 ++++++++++++++++ 6 files changed, 18 insertions(+), 18 deletions(-) -- 2.34.1