| Message ID | 20221207102909.17794-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/platform: Protect XENPF_get_dom0_console if CONFIG_VIDEO not set | expand |
On 07.12.2022 11:29, Michal Orzel wrote: > A build failure [1] is observed if CONFIG_VGA (and thus CONFIG_VIDEO) is > not set. This is because XENPF_get_dom0_console cmd of platform hypercall > makes a call to fill_console_start_info, which is defined in video/vga.c > and built only if CONFIG_VGA is set. Which means to use ... > --- a/xen/arch/x86/platform_hypercall.c > +++ b/xen/arch/x86/platform_hypercall.c > @@ -839,6 +839,7 @@ ret_t do_platform_op( > } > break; > > +#ifdef CONFIG_VIDEO ... CONFIG_VGA here, unless fill_console_start_info() was moved to a file that's built dependent upon CONFIG_VIDEO. Jan
Hi Jan, On 07/12/2022 11:34, Jan Beulich wrote: > > > On 07.12.2022 11:29, Michal Orzel wrote: >> A build failure [1] is observed if CONFIG_VGA (and thus CONFIG_VIDEO) is >> not set. This is because XENPF_get_dom0_console cmd of platform hypercall >> makes a call to fill_console_start_info, which is defined in video/vga.c >> and built only if CONFIG_VGA is set. > > Which means to use ... > >> --- a/xen/arch/x86/platform_hypercall.c >> +++ b/xen/arch/x86/platform_hypercall.c >> @@ -839,6 +839,7 @@ ret_t do_platform_op( >> } >> break; >> >> +#ifdef CONFIG_VIDEO > > ... CONFIG_VGA here, unless fill_console_start_info() was moved to a file > that's built dependent upon CONFIG_VIDEO. The one and only reason of using CONFIG_VIDEO and not CONFIG_VGA is that all the calls in existing codebase to fill_console_start_info are protected by CONFIG_VIDEO and not CONFIG_VGA. That is why I choose to stick to the current behavior. > > Jan ~Michal
On 07.12.2022 11:38, Michal Orzel wrote: > Hi Jan, > > On 07/12/2022 11:34, Jan Beulich wrote: >> >> >> On 07.12.2022 11:29, Michal Orzel wrote: >>> A build failure [1] is observed if CONFIG_VGA (and thus CONFIG_VIDEO) is >>> not set. This is because XENPF_get_dom0_console cmd of platform hypercall >>> makes a call to fill_console_start_info, which is defined in video/vga.c >>> and built only if CONFIG_VGA is set. >> >> Which means to use ... >> >>> --- a/xen/arch/x86/platform_hypercall.c >>> +++ b/xen/arch/x86/platform_hypercall.c >>> @@ -839,6 +839,7 @@ ret_t do_platform_op( >>> } >>> break; >>> >>> +#ifdef CONFIG_VIDEO >> >> ... CONFIG_VGA here, unless fill_console_start_info() was moved to a file >> that's built dependent upon CONFIG_VIDEO. > The one and only reason of using CONFIG_VIDEO and not CONFIG_VGA is that all the > calls in existing codebase to fill_console_start_info are protected by CONFIG_VIDEO > and not CONFIG_VGA. That is why I choose to stick to the current behavior. "All" is interesting as a statement when there's just one. But yes, I can see that the issue is broader - 2b8a95a2961b ("xen/x86: make VGA support selectable") didn't really get the #ifdef-s right. I guess the change then is okay as is: Acked-by: Jan Beulich <jbeulich@suse.com> and we'll need to touch all of that again (or move code around) if some 2nd way of selecting VIDEO appears. Jan
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c index 3f0d0389af44..08ab2fea622c 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -839,6 +839,7 @@ ret_t do_platform_op( } break; +#ifdef CONFIG_VIDEO case XENPF_get_dom0_console: if ( !fill_console_start_info(&op->u.dom0_console) ) { @@ -849,6 +850,7 @@ ret_t do_platform_op( if ( copy_field_to_guest(u_xenpf_op, op, u.dom0_console) ) ret = -EFAULT; break; +#endif default: ret = -ENOSYS;
A build failure [1] is observed if CONFIG_VGA (and thus CONFIG_VIDEO) is not set. This is because XENPF_get_dom0_console cmd of platform hypercall makes a call to fill_console_start_info, which is defined in video/vga.c and built only if CONFIG_VGA is set. To fix this issue, protect XENPF_get_dom0_console with CONFIG_VIDEO ifdefery. [1]: ld: prelink.o: in function `do_platform_op': (.text.do_platform_op+0x1a7): undefined reference to `fill_console_start_info' Fixes: 4dd160583c79 ("x86/platform: introduce hypercall to get initial video console settings") Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- CI job failure: https://gitlab.com/xen-project/xen/-/jobs/3430825994 --- xen/arch/x86/platform_hypercall.c | 2 ++ 1 file changed, 2 insertions(+)