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' and
including "<xen/console.h>" respectively.
This also resolves violations of MISRA C:2012 Rule 8.4.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Fixes: ebb26b509f1a ("xen/x86: make VGA support selectable")
Fixes: 6d9199bd0f22 ("x86-64: enable hypervisor output on VESA frame buffer")
---
xen/arch/x86/include/asm/setup.h | 6 ------
xen/drivers/video/vga.c | 9 +--------
xen/include/xen/vga.h | 14 ++++++++++++++
3 files changed, 15 insertions(+), 14 deletions(-)
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/drivers/video/vga.c b/xen/drivers/video/vga.c
index 0a03508bee60..b62a47e000e7 100644
--- a/xen/drivers/video/vga.c
+++ b/xen/drivers/video/vga.c
@@ -9,6 +9,7 @@
#include <xen/mm.h>
#include <xen/param.h>
#include <xen/vga.h>
+#include <xen/console.h>
#include <xen/pci.h>
#include <asm/io.h>
@@ -54,14 +55,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/vga.h b/xen/include/xen/vga.h
index f72b63d446b1..ffd73c9db135 100644
--- a/xen/include/xen/vga.h
+++ b/xen/include/xen/vga.h
@@ -15,4 +15,18 @@
extern struct xen_vga_console_info vga_console_info;
#endif
+#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 */
--
2.34.1On 11.08.2023 09:19, Nicola Vetrini wrote: > --- a/xen/drivers/video/vga.c > +++ b/xen/drivers/video/vga.c > @@ -9,6 +9,7 @@ > #include <xen/mm.h> > #include <xen/param.h> > #include <xen/vga.h> > +#include <xen/console.h> xen/vga.h, which you move the declarations to, is already included here. Why the need for xen/console.h? Jan
On 14/08/2023 09:47, Jan Beulich wrote: > On 11.08.2023 09:19, Nicola Vetrini wrote: >> --- a/xen/drivers/video/vga.c >> +++ b/xen/drivers/video/vga.c >> @@ -9,6 +9,7 @@ >> #include <xen/mm.h> >> #include <xen/param.h> >> #include <xen/vga.h> >> +#include <xen/console.h> > > xen/vga.h, which you move the declarations to, is already included > here. > Why the need for xen/console.h? > > Jan vga.c needs a declaration for fill_console_start_info, which is declared in console.h, as stated in the commit message (it could be clarified perhaps). -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
On 14.08.2023 09:56, Nicola Vetrini wrote: > On 14/08/2023 09:47, Jan Beulich wrote: >> On 11.08.2023 09:19, Nicola Vetrini wrote: >>> --- a/xen/drivers/video/vga.c >>> +++ b/xen/drivers/video/vga.c >>> @@ -9,6 +9,7 @@ >>> #include <xen/mm.h> >>> #include <xen/param.h> >>> #include <xen/vga.h> >>> +#include <xen/console.h> >> >> xen/vga.h, which you move the declarations to, is already included >> here. >> Why the need for xen/console.h? > > vga.c needs a declaration for fill_console_start_info, which is declared > in console.h, as > stated in the commit message (it could be clarified perhaps). Ah, I see. It's not very fortunate mixing of two separate adjustments. But then I'm inclined to ask: When fill_console_start_info() is defined in vga.c, wouldn't it make sense to move its declaration to vga.h? The more considering the type of its parameter? Jan
On 14/08/2023 10:12, Jan Beulich wrote: > On 14.08.2023 09:56, Nicola Vetrini wrote: >> On 14/08/2023 09:47, Jan Beulich wrote: >>> On 11.08.2023 09:19, Nicola Vetrini wrote: >>>> --- a/xen/drivers/video/vga.c >>>> +++ b/xen/drivers/video/vga.c >>>> @@ -9,6 +9,7 @@ >>>> #include <xen/mm.h> >>>> #include <xen/param.h> >>>> #include <xen/vga.h> >>>> +#include <xen/console.h> >>> >>> xen/vga.h, which you move the declarations to, is already included >>> here. >>> Why the need for xen/console.h? >> >> vga.c needs a declaration for fill_console_start_info, which is >> declared >> in console.h, as >> stated in the commit message (it could be clarified perhaps). > > Ah, I see. It's not very fortunate mixing of two separate adjustments. Well, they are not so separate, because they both deal with what the functions in vga.c need to be compliant with Rule 8.4. > But then I'm inclined to ask: When fill_console_start_info() is > defined in vga.c, wouldn't it make sense to move its declaration to > vga.h? The more considering the type of its parameter? > > Jan I see no downside. This change would imply having to include <xen/vga.h> in both - x86/platform_hypercall.c - x86/pv/dom0_build.c which seems okay. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
On Fri, 11 Aug 2023, Nicola Vetrini wrote:
> 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' and
> including "<xen/console.h>" respectively.
> This also resolves violations of MISRA C:2012 Rule 8.4.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Fixes: ebb26b509f1a ("xen/x86: make VGA support selectable")
> Fixes: 6d9199bd0f22 ("x86-64: enable hypervisor output on VESA frame buffer")
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> xen/arch/x86/include/asm/setup.h | 6 ------
> xen/drivers/video/vga.c | 9 +--------
> xen/include/xen/vga.h | 14 ++++++++++++++
> 3 files changed, 15 insertions(+), 14 deletions(-)
>
> 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/drivers/video/vga.c b/xen/drivers/video/vga.c
> index 0a03508bee60..b62a47e000e7 100644
> --- a/xen/drivers/video/vga.c
> +++ b/xen/drivers/video/vga.c
> @@ -9,6 +9,7 @@
> #include <xen/mm.h>
> #include <xen/param.h>
> #include <xen/vga.h>
> +#include <xen/console.h>
> #include <xen/pci.h>
> #include <asm/io.h>
>
> @@ -54,14 +55,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/vga.h b/xen/include/xen/vga.h
> index f72b63d446b1..ffd73c9db135 100644
> --- a/xen/include/xen/vga.h
> +++ b/xen/include/xen/vga.h
> @@ -15,4 +15,18 @@
> extern struct xen_vga_console_info vga_console_info;
> #endif
>
> +#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 */
> --
> 2.34.1
>
© 2016 - 2024 Red Hat, Inc.