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(+)

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;
-- 
2.25.1

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