Re: [PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
On Mon, Feb 07, 2022 at 07:24:12PM +, Julien Grall wrote:
> Hi Roger,
>
> On 07/02/2022 11:58, Roger Pau Monné wrote:
> > On Mon, Feb 07, 2022 at 09:57:55AM +0100, Jan Beulich wrote:
> > > On 06.02.2022 20:28, Julien Grall wrote:
> > > > From: Julien Grall
> > > >
> > > > When using EFI, the VGA information is fetched using the EFI
> > > > boot services. However, Xen will have exited the boot services.
> > > > Therefore, we need to find a different way to pass the information
> > > > to dom0.
> > > >
> > > > For PV dom0, they are part of the start_info. But this is not
> > > > something that exists on Arm. So the best way would to be to
> > > > use a hypercall.
> > > >
> > > > For now the structure layout is based on dom0_vga_console_info
> > > > for convenience. I am open on another proposal.
> > > >
> > > > Signed-off-by: Julien Grall
> > >
> > > Cc-ing Roger as this may want using for PVH Dom0 also on x86; my
> > > first attempt to propagate this information was rejected.
> >
> > I think it's easier to use a Xen specific layout in XENPF, as that's
> > already a Xen specific interface.
> >
> > I wonder however if passing the information here (instead of doing it
> > in the start info or equivalent) could cause a delay in the
> > initialization of the video console.
>
> My current plan for Arm is to issue the hypercall as part of an earlyinit
> call. But we can do much earlier (i.e. xen_early_init() which is called from
> setup_arch()) if necessary.
>
> This should be early enough for Arm. How about x86?
Yes, I think that's fine for x86 also.
> > I guess the same happens when
> > using the Xen consoles (either the hypercall one or the shared ring),
> > so it's fine.
> >
> > > > --- a/xen/include/public/platform.h
> > > > +++ b/xen/include/public/platform.h
> > > > @@ -244,6 +244,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_efi_runtime_call_t);
> > > > #define XEN_FW_EFI_PCI_ROM5
> > > > #define XEN_FW_EFI_APPLE_PROPERTIES 6
> > > > #define XEN_FW_KBD_SHIFT_FLAGS5
> > > > +#define XEN_FW_VGA_INFO 6
> > >
> > > Perhaps s/VGA/VIDEO/, despite ...
> > >
> > > > struct xenpf_firmware_info {
> > > > /* IN variables. */
> > > > uint32_t type;
> > > > @@ -311,6 +312,7 @@ struct xenpf_firmware_info {
> > > > /* Int16, Fn02: Get keyboard shift flags. */
> > > > uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
> > > > +struct dom0_vga_console_info vga;
> > >
> > > ... the structure name including "vga" (but if the #define is adjusted,
> > > the field name would want to become "video" as well).
> >
>
> [...]
>
> (Re-ordered the quote as it makes more sense for my reply)
>
> > There's no need to propagate XEN_VGATYPE_TEXT_MODE_3 into this
> > interface.
>
> So for Arm, we are only caring about XEN_VGATYPE_EFI_LFB. I wasn't sure what
> would be your need on x86. Hence, why I keep it.
>
> If you don't need then, then I am happy to trim the structure for the new
> hypercall.
Oh, so I was slightly confused. You are adding a top level
XEN_FW_VIDEO_INFO, not a EFI sub-op one. In which case, yes, we would
need to keep the MODE_3 as part of the interface.
> > It's my understanding that this will forcefully be
> > XEN_VGATYPE_EFI_LFB, at which point we could consider giving a type
> > name to the vesa_lfb field of dom0_vga_console_info (video_lfb) and
> > use the same struct here?>
>
> Just to clarify, are you suggesting to only pass video_lfb? IOW, we will
> always assume it is an EFI framebuffer and not pass the video_type.
That would be the case if we add a XEN_FW_EFI_VIDEO sub option, if
instead we add a top level one (XEN_FW_VIDEO_INFO) we need to keep the
mode 3 support.
It might be best for x86 to introduce a global XEN_FW_VIDEO_INFO, as
we can then convey all the video information regardless of the boot
mode.
FWIW, I'm not a huge fan of the struct name (I would rather prefer
video_info or some such), but that ship sailed long time ago.
Thanks, Roger.
Re: [PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
Hi Roger,
On 07/02/2022 11:58, Roger Pau Monné wrote:
On Mon, Feb 07, 2022 at 09:57:55AM +0100, Jan Beulich wrote:
On 06.02.2022 20:28, Julien Grall wrote:
From: Julien Grall
When using EFI, the VGA information is fetched using the EFI
boot services. However, Xen will have exited the boot services.
Therefore, we need to find a different way to pass the information
to dom0.
For PV dom0, they are part of the start_info. But this is not
something that exists on Arm. So the best way would to be to
use a hypercall.
For now the structure layout is based on dom0_vga_console_info
for convenience. I am open on another proposal.
Signed-off-by: Julien Grall
Cc-ing Roger as this may want using for PVH Dom0 also on x86; my
first attempt to propagate this information was rejected.
I think it's easier to use a Xen specific layout in XENPF, as that's
already a Xen specific interface.
I wonder however if passing the information here (instead of doing it
in the start info or equivalent) could cause a delay in the
initialization of the video console.
My current plan for Arm is to issue the hypercall as part of an
earlyinit call. But we can do much earlier (i.e. xen_early_init() which
is called from setup_arch()) if necessary.
This should be early enough for Arm. How about x86?
I guess the same happens when
using the Xen consoles (either the hypercall one or the shared ring),
so it's fine.
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -244,6 +244,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_efi_runtime_call_t);
#define XEN_FW_EFI_PCI_ROM5
#define XEN_FW_EFI_APPLE_PROPERTIES 6
#define XEN_FW_KBD_SHIFT_FLAGS5
+#define XEN_FW_VGA_INFO 6
Perhaps s/VGA/VIDEO/, despite ...
struct xenpf_firmware_info {
/* IN variables. */
uint32_t type;
@@ -311,6 +312,7 @@ struct xenpf_firmware_info {
/* Int16, Fn02: Get keyboard shift flags. */
uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
+struct dom0_vga_console_info vga;
... the structure name including "vga" (but if the #define is adjusted,
the field name would want to become "video" as well).
[...]
(Re-ordered the quote as it makes more sense for my reply)
There's no need to propagate XEN_VGATYPE_TEXT_MODE_3 into this
interface.
So for Arm, we are only caring about XEN_VGATYPE_EFI_LFB. I wasn't sure
what would be your need on x86. Hence, why I keep it.
If you don't need then, then I am happy to trim the structure for the
new hypercall.
> It's my understanding that this will forcefully be
> XEN_VGATYPE_EFI_LFB, at which point we could consider giving a type
> name to the vesa_lfb field of dom0_vga_console_info (video_lfb) and
> use the same struct here?>
Just to clarify, are you suggesting to only pass video_lfb? IOW, we will
always assume it is an EFI framebuffer and not pass the video_type.
Cheers,
--
Julien Grall
Re: [PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
On 07.02.2022 12:58, Roger Pau Monné wrote:
> On Mon, Feb 07, 2022 at 09:57:55AM +0100, Jan Beulich wrote:
>> On 06.02.2022 20:28, Julien Grall wrote:
>>> @@ -311,6 +312,7 @@ struct xenpf_firmware_info {
>>>
>>> /* Int16, Fn02: Get keyboard shift flags. */
>>> uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
>>> +struct dom0_vga_console_info vga;
>>
>> ... the structure name including "vga" (but if the #define is adjusted,
>> the field name would want to become "video" as well).
>
> It's my understanding that this will forcefully be
> XEN_VGATYPE_EFI_LFB, at which point we could consider giving a type
> name to the vesa_lfb field of dom0_vga_console_info (video_lfb) and
> use the same struct here?
>
> There's no need to propagate XEN_VGATYPE_TEXT_MODE_3 into this
> interface.
Hmm, yes, this is probably better / more clean. Julien, thoughts?
Jan
Re: [PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
On Mon, Feb 07, 2022 at 09:57:55AM +0100, Jan Beulich wrote:
> On 06.02.2022 20:28, Julien Grall wrote:
> > From: Julien Grall
> >
> > When using EFI, the VGA information is fetched using the EFI
> > boot services. However, Xen will have exited the boot services.
> > Therefore, we need to find a different way to pass the information
> > to dom0.
> >
> > For PV dom0, they are part of the start_info. But this is not
> > something that exists on Arm. So the best way would to be to
> > use a hypercall.
> >
> > For now the structure layout is based on dom0_vga_console_info
> > for convenience. I am open on another proposal.
> >
> > Signed-off-by: Julien Grall
>
> Cc-ing Roger as this may want using for PVH Dom0 also on x86; my
> first attempt to propagate this information was rejected.
I think it's easier to use a Xen specific layout in XENPF, as that's
already a Xen specific interface.
I wonder however if passing the information here (instead of doing it
in the start info or equivalent) could cause a delay in the
initialization of the video console. I guess the same happens when
using the Xen consoles (either the hypercall one or the shared ring),
so it's fine.
> > --- a/xen/include/public/platform.h
> > +++ b/xen/include/public/platform.h
> > @@ -244,6 +244,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_efi_runtime_call_t);
> > #define XEN_FW_EFI_PCI_ROM5
> > #define XEN_FW_EFI_APPLE_PROPERTIES 6
> > #define XEN_FW_KBD_SHIFT_FLAGS5
> > +#define XEN_FW_VGA_INFO 6
>
> Perhaps s/VGA/VIDEO/, despite ...
>
> > struct xenpf_firmware_info {
> > /* IN variables. */
> > uint32_t type;
> > @@ -311,6 +312,7 @@ struct xenpf_firmware_info {
> >
> > /* Int16, Fn02: Get keyboard shift flags. */
> > uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
> > +struct dom0_vga_console_info vga;
>
> ... the structure name including "vga" (but if the #define is adjusted,
> the field name would want to become "video" as well).
It's my understanding that this will forcefully be
XEN_VGATYPE_EFI_LFB, at which point we could consider giving a type
name to the vesa_lfb field of dom0_vga_console_info (video_lfb) and
use the same struct here?
There's no need to propagate XEN_VGATYPE_TEXT_MODE_3 into this
interface.
Thanks, Roger.
Re: [PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
On 06.02.2022 20:28, Julien Grall wrote:
> From: Julien Grall
>
> When using EFI, the VGA information is fetched using the EFI
> boot services. However, Xen will have exited the boot services.
> Therefore, we need to find a different way to pass the information
> to dom0.
>
> For PV dom0, they are part of the start_info. But this is not
> something that exists on Arm. So the best way would to be to
> use a hypercall.
>
> For now the structure layout is based on dom0_vga_console_info
> for convenience. I am open on another proposal.
>
> Signed-off-by: Julien Grall
Cc-ing Roger as this may want using for PVH Dom0 also on x86; my
first attempt to propagate this information was rejected.
> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -244,6 +244,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_efi_runtime_call_t);
> #define XEN_FW_EFI_PCI_ROM5
> #define XEN_FW_EFI_APPLE_PROPERTIES 6
> #define XEN_FW_KBD_SHIFT_FLAGS5
> +#define XEN_FW_VGA_INFO 6
Perhaps s/VGA/VIDEO/, despite ...
> struct xenpf_firmware_info {
> /* IN variables. */
> uint32_t type;
> @@ -311,6 +312,7 @@ struct xenpf_firmware_info {
>
> /* Int16, Fn02: Get keyboard shift flags. */
> uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
> +struct dom0_vga_console_info vga;
... the structure name including "vga" (but if the #define is adjusted,
the field name would want to become "video" as well).
Jan
[PATCH RFC 3/3] xen: Introduce a platform sub-op to retrieve the VGA information
From: Julien Grall
When using EFI, the VGA information is fetched using the EFI
boot services. However, Xen will have exited the boot services.
Therefore, we need to find a different way to pass the information
to dom0.
For PV dom0, they are part of the start_info. But this is not
something that exists on Arm. So the best way would to be to
use a hypercall.
For now the structure layout is based on dom0_vga_console_info
for convenience. I am open on another proposal.
Signed-off-by: Julien Grall
TODO:
- Check the structure size has not changed (I would like to
avoid bumping the platform interface).
---
xen/arch/arm/platform_hypercall.c | 15 +++
xen/include/public/platform.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/xen/arch/arm/platform_hypercall.c
b/xen/arch/arm/platform_hypercall.c
index 8efac7ee602a..78ad328e2ab8 100644
--- a/xen/arch/arm/platform_hypercall.c
+++ b/xen/arch/arm/platform_hypercall.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -58,6 +59,20 @@ long
do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
ret = -EINVAL;
break;
+case XENPF_firmware_info:
+switch ( op->u.firmware_info.type )
+{
+case XEN_FW_VGA_INFO:
+BUILD_BUG_ON(sizeof(op->u.firmware_info.u.vga) !=
+ sizeof(vga_console_info));
+memcpy(>u.firmware_info.u.vga, _console_info,
+ sizeof(vga_console_info));
+if ( __copy_to_guest(u_xenpf_op, op, 1) )
+ret = -EFAULT;
+break;
+}
+break;
+
default:
ret = -ENOSYS;
break;
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index a4c0eb62249a..4de42ce6cbc5 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -244,6 +244,7 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_efi_runtime_call_t);
#define XEN_FW_EFI_PCI_ROM5
#define XEN_FW_EFI_APPLE_PROPERTIES 6
#define XEN_FW_KBD_SHIFT_FLAGS5
+#define XEN_FW_VGA_INFO 6
struct xenpf_firmware_info {
/* IN variables. */
uint32_t type;
@@ -311,6 +312,7 @@ struct xenpf_firmware_info {
/* Int16, Fn02: Get keyboard shift flags. */
uint8_t kbd_shift_flags; /* XEN_FW_KBD_SHIFT_FLAGS */
+struct dom0_vga_console_info vga;
} u;
};
typedef struct xenpf_firmware_info xenpf_firmware_info_t;
--
2.32.0
