* [PATCH 0/2] x86/video: improve early video detection @ 2024-03-28 15:35 Roger Pau Monne 2024-03-28 15:35 ` [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets Roger Pau Monne 2024-03-28 15:35 ` [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present Roger Pau Monne 0 siblings, 2 replies; 13+ messages in thread From: Roger Pau Monne @ 2024-03-28 15:35 UTC (permalink / raw) To: xen-devel; +Cc: Roger Pau Monne, Jan Beulich, Andrew Cooper Hello, The current video logic reports wrong values when booted in PVH mode, as that boot path explicitly skips real-mode logic, and thus avoids any VGA video detection. First patch is cleanup of the offset declarations in boot_video_info. Second patch attempts to fix Xen in PVH mode reporting VGA support. Roger Pau Monne (2): x86/video: add boot_video_info offset generation to asm-offsets x86/video: do not assume a video mode to be unconditionally present xen/arch/x86/boot/video.S | 88 +++++++++++-------------------- xen/arch/x86/x86_64/asm-offsets.c | 26 +++++++++ 2 files changed, 58 insertions(+), 56 deletions(-) -- 2.44.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets 2024-03-28 15:35 [PATCH 0/2] x86/video: improve early video detection Roger Pau Monne @ 2024-03-28 15:35 ` Roger Pau Monne 2024-03-28 19:55 ` Andrew Cooper 2024-04-02 9:38 ` Jan Beulich 2024-03-28 15:35 ` [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present Roger Pau Monne 1 sibling, 2 replies; 13+ messages in thread From: Roger Pau Monne @ 2024-03-28 15:35 UTC (permalink / raw) To: xen-devel; +Cc: Roger Pau Monne, Jan Beulich, Andrew Cooper Currently the offsets into the boot_video_info struct are manually encoded in video.S, which is fragile. Generate them in asm-offsets.c and switch the current code to use those instead. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/boot/video.S | 83 ++++++++++++------------------- xen/arch/x86/x86_64/asm-offsets.c | 26 ++++++++++ 2 files changed, 58 insertions(+), 51 deletions(-) diff --git a/xen/arch/x86/boot/video.S b/xen/arch/x86/boot/video.S index 0ae04f270f8c..a4b25a3b34d1 100644 --- a/xen/arch/x86/boot/video.S +++ b/xen/arch/x86/boot/video.S @@ -26,32 +26,13 @@ /* Force 400 scan lines for standard modes (hack to fix bad BIOS behaviour */ #undef CONFIG_VIDEO_400_HACK -/* Positions of various video parameters passed to the kernel */ -/* (see also include/linux/tty.h) */ -#define PARAM_CURSOR_POS 0x00 -#define PARAM_VIDEO_MODE 0x02 -#define PARAM_VIDEO_COLS 0x03 -#define PARAM_VIDEO_LINES 0x04 -#define PARAM_HAVE_VGA 0x05 -#define PARAM_FONT_POINTS 0x06 -#define PARAM_CAPABILITIES 0x08 -#define PARAM_LFB_LINELENGTH 0x0c -#define PARAM_LFB_WIDTH 0x0e -#define PARAM_LFB_HEIGHT 0x10 -#define PARAM_LFB_DEPTH 0x12 -#define PARAM_LFB_BASE 0x14 -#define PARAM_LFB_SIZE 0x18 -#define PARAM_LFB_COLORS 0x1c -#define PARAM_VESAPM_SEG 0x24 -#define PARAM_VESAPM_OFF 0x26 -#define PARAM_VESA_ATTRIB 0x28 #define _param(param) bootsym(boot_vid_info)+(param) video: xorw %ax, %ax movw %ax, %gs # GS is zero cld call basic_detect # Basic adapter type testing (EGA/VGA/MDA/CGA) - cmpb $0,_param(PARAM_HAVE_VGA) + cmpb $0,_param(BVI_have_vga) je 1f # Bail if there's no VGA movw bootsym(boot_vid_mode), %ax # User selected video mode cmpw $ASK_VGA, %ax # Bring up the menu @@ -69,7 +50,7 @@ vid1: call store_edid # Detect if we have CGA, MDA, EGA or VGA and pass it to the kernel. basic_detect: - movb $0, _param(PARAM_HAVE_VGA) + movb $0, _param(BVI_have_vga) movb $0x12, %ah # Check EGA/VGA movb $0x10, %bl int $0x10 @@ -79,7 +60,7 @@ basic_detect: int $0x10 cmpb $0x1a, %al # 1a means VGA... jne basret # anything else is EGA. - incb _param(PARAM_HAVE_VGA) # We've detected a VGA + incb _param(BVI_have_vga) # We've detected a VGA basret: ret # Store the video mode parameters for later usage by the kernel. @@ -92,57 +73,57 @@ mode_params: movb $0x03, %ah # Read cursor position xorb %bh, %bh int $0x10 - movw %dx, _param(PARAM_CURSOR_POS) + movw %dx, _param(BVI_cursor_pos) movb $0x0f, %ah # Read page/mode/width int $0x10 - movw %ax, _param(PARAM_VIDEO_MODE) # Video mode and screen width + movw %ax, _param(BVI_video_mode) # Video mode and screen width movw %gs:(0x485), %ax # Font size - movw %ax, _param(PARAM_FONT_POINTS) # (valid only on EGA/VGA) + movw %ax, _param(BVI_font_points) # (valid only on EGA/VGA) movw bootsym(force_size), %ax # Forced size? orw %ax, %ax jz mopar1 - movb %ah, _param(PARAM_VIDEO_COLS) - movb %al, _param(PARAM_VIDEO_LINES) + movb %ah, _param(BVI_video_cols) + movb %al, _param(BVI_video_lines) ret mopar1: movb %gs:(0x484), %al # On EGA/VGA, use the EGA+ BIOS incb %al # location of max lines. -mopar2: movb %al, _param(PARAM_VIDEO_LINES) +mopar2: movb %al, _param(BVI_video_lines) ret # Fetching of VESA frame buffer parameters mopar_gr: movw $vesa_mode_info, %di - movb $0x23, _param(PARAM_HAVE_VGA) + movb $0x23, _param(BVI_have_vga) movw 16(%di), %ax - movw %ax, _param(PARAM_LFB_LINELENGTH) + movw %ax, _param(BVI_lfb_linelength) movw 18(%di), %ax - movw %ax, _param(PARAM_LFB_WIDTH) + movw %ax, _param(BVI_lfb_width) movw 20(%di), %ax - movw %ax, _param(PARAM_LFB_HEIGHT) + movw %ax, _param(BVI_lfb_height) movzbw 25(%di), %ax - movw %ax, _param(PARAM_LFB_DEPTH) + movw %ax, _param(BVI_lfb_depth) movl 40(%di), %eax - movl %eax, _param(PARAM_LFB_BASE) + movl %eax, _param(BVI_lfb_base) movl 31(%di), %eax - movl %eax, _param(PARAM_LFB_COLORS) + movl %eax, _param(BVI_lfb_colors) movl 35(%di), %eax - movl %eax, _param(PARAM_LFB_COLORS+4) + movl %eax, _param(BVI_lfb_colors+4) movw 0(%di), %ax - movw %ax, _param(PARAM_VESA_ATTRIB) + movw %ax, _param(BVI_vesa_attrib) # get video mem size movw $vesa_glob_info, %di movzwl 18(%di), %eax - movl %eax, _param(PARAM_LFB_SIZE) + movl %eax, _param(BVI_lfb_size) # store mode capabilities movl 10(%di), %eax - movl %eax, _param(PARAM_CAPABILITIES) + movl %eax, _param(BVI_capabilities) # switching the DAC to 8-bit is for <= 8 bpp only - cmpw $8, _param(PARAM_LFB_DEPTH) + cmpw $8, _param(BVI_lfb_depth) jg dac_done # get DAC switching capability @@ -160,16 +141,16 @@ mopar_gr: dac_set: # set color size to DAC size movzbw bootsym(dac_size), %ax - movb %al, _param(PARAM_LFB_COLORS+0) - movb %al, _param(PARAM_LFB_COLORS+2) - movb %al, _param(PARAM_LFB_COLORS+4) - movb %al, _param(PARAM_LFB_COLORS+6) + movb %al, _param(BVI_lfb_colors+0) + movb %al, _param(BVI_lfb_colors+2) + movb %al, _param(BVI_lfb_colors+4) + movb %al, _param(BVI_lfb_colors+6) # set color offsets to 0 - movb %ah, _param(PARAM_LFB_COLORS+1) - movb %ah, _param(PARAM_LFB_COLORS+3) - movb %ah, _param(PARAM_LFB_COLORS+5) - movb %ah, _param(PARAM_LFB_COLORS+7) + movb %ah, _param(BVI_lfb_colors+1) + movb %ah, _param(BVI_lfb_colors+3) + movb %ah, _param(BVI_lfb_colors+5) + movb %ah, _param(BVI_lfb_colors+7) dac_done: # get protected mode interface information @@ -180,8 +161,8 @@ dac_done: cmp $0x004f, %ax jnz no_pm - movw %es, _param(PARAM_VESAPM_SEG) - movw %di, _param(PARAM_VESAPM_OFF) + movw %es, _param(BVI_vesapm_seg) + movw %di, _param(BVI_vesapm_off) no_pm: pushw %ds popw %es @@ -1018,7 +999,7 @@ GLOBAL(boot_vid_info) .byte 80, 25 /* 80x25 */ .byte 1 /* isVGA */ .word 16 /* 8x16 font */ - .fill 0x28,1,0 + .space BVI_size - BVI_capabilities GLOBAL(boot_edid_info) .fill 128,1,0x13 GLOBAL(boot_edid_caps) diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index d8903a3ce9c7..91da6b9d3885 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -16,6 +16,10 @@ #include <xen/multiboot.h> #include <xen/multiboot2.h> +#ifdef CONFIG_VIDEO +# include "../boot/video.h" +#endif + #define DEFINE(_sym, _val) \ asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\""\ :: "i" (_val) ) @@ -208,4 +212,26 @@ void __dummy__(void) OFFSET(DOMAIN_vm_assist, struct domain, vm_assist); BLANK(); + +#ifdef CONFIG_VIDEO + DEFINE(BVI_size, sizeof(struct boot_video_info)); + OFFSET(BVI_cursor_pos, struct boot_video_info, orig_x); + OFFSET(BVI_video_mode, struct boot_video_info, orig_video_mode); + OFFSET(BVI_video_cols, struct boot_video_info, orig_video_cols); + OFFSET(BVI_video_lines, struct boot_video_info, orig_video_lines); + OFFSET(BVI_have_vga, struct boot_video_info, orig_video_isVGA); + OFFSET(BVI_font_points, struct boot_video_info, orig_video_points); + OFFSET(BVI_capabilities, struct boot_video_info, capabilities); + OFFSET(BVI_lfb_linelength, struct boot_video_info, lfb_linelength); + OFFSET(BVI_lfb_width, struct boot_video_info, lfb_width); + OFFSET(BVI_lfb_height, struct boot_video_info, lfb_height); + OFFSET(BVI_lfb_depth, struct boot_video_info, lfb_depth); + OFFSET(BVI_lfb_base, struct boot_video_info, lfb_base); + OFFSET(BVI_lfb_size, struct boot_video_info, lfb_size); + OFFSET(BVI_lfb_colors, struct boot_video_info, colors); + OFFSET(BVI_vesapm_seg, struct boot_video_info, vesapm.seg); + OFFSET(BVI_vesapm_off, struct boot_video_info, vesapm.off); + OFFSET(BVI_vesa_attrib, struct boot_video_info, vesa_attrib); + BLANK(); +#endif /* CONFIG_VIDEO */ } -- 2.44.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets 2024-03-28 15:35 ` [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets Roger Pau Monne @ 2024-03-28 19:55 ` Andrew Cooper 2024-04-02 9:38 ` Jan Beulich 1 sibling, 0 replies; 13+ messages in thread From: Andrew Cooper @ 2024-03-28 19:55 UTC (permalink / raw) To: Roger Pau Monne, xen-devel; +Cc: Jan Beulich On 28/03/2024 3:35 pm, Roger Pau Monne wrote: > Currently the offsets into the boot_video_info struct are manually encoded in > video.S, which is fragile. Generate them in asm-offsets.c and switch the > current code to use those instead. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> R-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks for looking at this. As you're touching these lines, there are a few style fixes. > diff --git a/xen/arch/x86/boot/video.S b/xen/arch/x86/boot/video.S > index 0ae04f270f8c..a4b25a3b34d1 100644 > --- a/xen/arch/x86/boot/video.S > +++ b/xen/arch/x86/boot/video.S > @@ -26,32 +26,13 @@ > /* Force 400 scan lines for standard modes (hack to fix bad BIOS behaviour */ > #undef CONFIG_VIDEO_400_HACK > > -/* Positions of various video parameters passed to the kernel */ > -/* (see also include/linux/tty.h) */ > -#define PARAM_CURSOR_POS 0x00 > -#define PARAM_VIDEO_MODE 0x02 > -#define PARAM_VIDEO_COLS 0x03 > -#define PARAM_VIDEO_LINES 0x04 > -#define PARAM_HAVE_VGA 0x05 > -#define PARAM_FONT_POINTS 0x06 > -#define PARAM_CAPABILITIES 0x08 > -#define PARAM_LFB_LINELENGTH 0x0c > -#define PARAM_LFB_WIDTH 0x0e > -#define PARAM_LFB_HEIGHT 0x10 > -#define PARAM_LFB_DEPTH 0x12 > -#define PARAM_LFB_BASE 0x14 > -#define PARAM_LFB_SIZE 0x18 > -#define PARAM_LFB_COLORS 0x1c > -#define PARAM_VESAPM_SEG 0x24 > -#define PARAM_VESAPM_OFF 0x26 > -#define PARAM_VESA_ATTRIB 0x28 > #define _param(param) bootsym(boot_vid_info)+(param) > > video: xorw %ax, %ax > movw %ax, %gs # GS is zero > cld > call basic_detect # Basic adapter type testing (EGA/VGA/MDA/CGA) > - cmpb $0,_param(PARAM_HAVE_VGA) > + cmpb $0,_param(BVI_have_vga) Space > @@ -160,16 +141,16 @@ mopar_gr: > dac_set: > # set color size to DAC size > movzbw bootsym(dac_size), %ax > - movb %al, _param(PARAM_LFB_COLORS+0) > - movb %al, _param(PARAM_LFB_COLORS+2) > - movb %al, _param(PARAM_LFB_COLORS+4) > - movb %al, _param(PARAM_LFB_COLORS+6) > + movb %al, _param(BVI_lfb_colors+0) > + movb %al, _param(BVI_lfb_colors+2) > + movb %al, _param(BVI_lfb_colors+4) > + movb %al, _param(BVI_lfb_colors+6) Spaces > diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c > index d8903a3ce9c7..91da6b9d3885 100644 > --- a/xen/arch/x86/x86_64/asm-offsets.c > +++ b/xen/arch/x86/x86_64/asm-offsets.c > @@ -16,6 +16,10 @@ > #include <xen/multiboot.h> > #include <xen/multiboot2.h> > > +#ifdef CONFIG_VIDEO > +# include "../boot/video.h" > +#endif > + > #define DEFINE(_sym, _val) \ > asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\""\ > :: "i" (_val) ) > @@ -208,4 +212,26 @@ void __dummy__(void) > > OFFSET(DOMAIN_vm_assist, struct domain, vm_assist); > BLANK(); > + > +#ifdef CONFIG_VIDEO > + DEFINE(BVI_size, sizeof(struct boot_video_info)); > + OFFSET(BVI_cursor_pos, struct boot_video_info, orig_x); > + OFFSET(BVI_video_mode, struct boot_video_info, orig_video_mode); > + OFFSET(BVI_video_cols, struct boot_video_info, orig_video_cols); > + OFFSET(BVI_video_lines, struct boot_video_info, orig_video_lines); > + OFFSET(BVI_have_vga, struct boot_video_info, orig_video_isVGA); > + OFFSET(BVI_font_points, struct boot_video_info, orig_video_points); > + OFFSET(BVI_capabilities, struct boot_video_info, capabilities); > + OFFSET(BVI_lfb_linelength, struct boot_video_info, lfb_linelength); > + OFFSET(BVI_lfb_width, struct boot_video_info, lfb_width); > + OFFSET(BVI_lfb_height, struct boot_video_info, lfb_height); > + OFFSET(BVI_lfb_depth, struct boot_video_info, lfb_depth); > + OFFSET(BVI_lfb_base, struct boot_video_info, lfb_base); > + OFFSET(BVI_lfb_size, struct boot_video_info, lfb_size); > + OFFSET(BVI_lfb_colors, struct boot_video_info, colors); > + OFFSET(BVI_vesapm_seg, struct boot_video_info, vesapm.seg); > + OFFSET(BVI_vesapm_off, struct boot_video_info, vesapm.off); > + OFFSET(BVI_vesa_attrib, struct boot_video_info, vesa_attrib); > + BLANK(); > +#endif /* CONFIG_VIDEO */ BVI_size traditionally goes last. MB2 (which I guess you copied?) is a little odd. I'd like to start vertically aligning this for readability, like I started with EFRAME_*. Happy to fold of all of these tweaks on commit. ~Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets 2024-03-28 15:35 ` [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets Roger Pau Monne 2024-03-28 19:55 ` Andrew Cooper @ 2024-04-02 9:38 ` Jan Beulich 2024-04-02 9:43 ` Jan Beulich 1 sibling, 1 reply; 13+ messages in thread From: Jan Beulich @ 2024-04-02 9:38 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, xen-devel On 28.03.2024 16:35, Roger Pau Monne wrote: > Currently the offsets into the boot_video_info struct are manually encoded in > video.S, which is fragile. Generate them in asm-offsets.c and switch the > current code to use those instead. Just to mention it (without asking for immediate action): Defining boot_vid_info in assembly code then is as fragile. Moving to C would likely be problematic because it needs to be in the trampoline range. But at least its size should (at some point) perhaps better be tied to the C struct's sizeof(). The fields, with some effort, could also be converted using the new BVI_* constants. That would still leave the field sizes; maybe those could at least be cross-checked by some BUILD_BUG_ONs. Jan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets 2024-04-02 9:38 ` Jan Beulich @ 2024-04-02 9:43 ` Jan Beulich 2024-04-19 7:25 ` Roger Pau Monné 0 siblings, 1 reply; 13+ messages in thread From: Jan Beulich @ 2024-04-02 9:43 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, xen-devel On 02.04.2024 11:38, Jan Beulich wrote: > On 28.03.2024 16:35, Roger Pau Monne wrote: >> Currently the offsets into the boot_video_info struct are manually encoded in >> video.S, which is fragile. Generate them in asm-offsets.c and switch the >> current code to use those instead. > > Just to mention it (without asking for immediate action): Defining boot_vid_info > in assembly code then is as fragile. Moving to C would likely be problematic > because it needs to be in the trampoline range. But at least its size should (at > some point) perhaps better be tied to the C struct's sizeof(). Actually I overlooked that you partly do this. The use of BVI_capabilities there looks odd to me, though. Why not .space BVI_size - (. - boot_vid_info) ? I realize it becomes just BVI_size in patch 2, but I have some question there, too. Jan > The fields, with > some effort, could also be converted using the new BVI_* constants. That would > still leave the field sizes; maybe those could at least be cross-checked by some > BUILD_BUG_ONs. > > Jan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets 2024-04-02 9:43 ` Jan Beulich @ 2024-04-19 7:25 ` Roger Pau Monné 0 siblings, 0 replies; 13+ messages in thread From: Roger Pau Monné @ 2024-04-19 7:25 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, xen-devel On Tue, Apr 02, 2024 at 11:43:49AM +0200, Jan Beulich wrote: > On 02.04.2024 11:38, Jan Beulich wrote: > > On 28.03.2024 16:35, Roger Pau Monne wrote: > >> Currently the offsets into the boot_video_info struct are manually encoded in > >> video.S, which is fragile. Generate them in asm-offsets.c and switch the > >> current code to use those instead. > > > > Just to mention it (without asking for immediate action): Defining boot_vid_info > > in assembly code then is as fragile. Moving to C would likely be problematic > > because it needs to be in the trampoline range. But at least its size should (at > > some point) perhaps better be tied to the C struct's sizeof(). > > Actually I overlooked that you partly do this. The use of BVI_capabilities there > looks odd to me, though. Why not > > .space BVI_size - (. - boot_vid_info) > > ? I realize it becomes just BVI_size in patch 2, but I have some question there, > too. I didn't think much about this because it was going away in the next patch, but let me adjust. In fact I was tempted to just use BVI_size and allocate more than required. Thanks, Roger. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-03-28 15:35 [PATCH 0/2] x86/video: improve early video detection Roger Pau Monne 2024-03-28 15:35 ` [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets Roger Pau Monne @ 2024-03-28 15:35 ` Roger Pau Monne 2024-03-28 19:58 ` Andrew Cooper 2024-04-02 9:49 ` Jan Beulich 1 sibling, 2 replies; 13+ messages in thread From: Roger Pau Monne @ 2024-03-28 15:35 UTC (permalink / raw) To: xen-devel; +Cc: Roger Pau Monne, Jan Beulich, Andrew Cooper There's no reason to assume VGA text mode 3 to be unconditionally available. With the addition of booting Xen itself in PVH mode there's a boot path that explicitly short-circuits all the real-mode logic, including the VGA detection. Leave the default user selected mode as text mode 3 in boot_vid_mode, but do not populate boot_vid_info with any default settings. It will either be populated by the real-mode video detection code, or left zeroed in case real-mode code is skipped. Note that only PVH skips the real-mode portion of the boot trampoline, otherwise the only way to skip it is to set `no-real-mode` on the command line, and the description for the option already notes that VGA would be disabled as a result of skipping real-mode bootstrap. This fixes Xen incorrectly reporting: (XEN) Video information: (XEN) VGA is text mode 80x25, font 8x16 When booted as a PVH guest. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/boot/video.S | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/xen/arch/x86/boot/video.S b/xen/arch/x86/boot/video.S index a4b25a3b34d1..a51de04a024e 100644 --- a/xen/arch/x86/boot/video.S +++ b/xen/arch/x86/boot/video.S @@ -994,12 +994,7 @@ name_bann: .asciz "Video adapter: " force_size: .word 0 # Use this size instead of the one in BIOS vars GLOBAL(boot_vid_info) - .byte 0, 0 /* orig_x, orig_y */ - .byte 3 /* text mode 3 */ - .byte 80, 25 /* 80x25 */ - .byte 1 /* isVGA */ - .word 16 /* 8x16 font */ - .space BVI_size - BVI_capabilities + .space BVI_size GLOBAL(boot_edid_info) .fill 128,1,0x13 GLOBAL(boot_edid_caps) -- 2.44.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-03-28 15:35 ` [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present Roger Pau Monne @ 2024-03-28 19:58 ` Andrew Cooper 2024-04-02 9:54 ` Jan Beulich 2024-04-02 9:49 ` Jan Beulich 1 sibling, 1 reply; 13+ messages in thread From: Andrew Cooper @ 2024-03-28 19:58 UTC (permalink / raw) To: Roger Pau Monne, xen-devel; +Cc: Jan Beulich On 28/03/2024 3:35 pm, Roger Pau Monne wrote: > There's no reason to assume VGA text mode 3 to be unconditionally available. > With the addition of booting Xen itself in PVH mode there's a boot path that > explicitly short-circuits all the real-mode logic, including the VGA detection. > > Leave the default user selected mode as text mode 3 in boot_vid_mode, but do > not populate boot_vid_info with any default settings. It will either be > populated by the real-mode video detection code, or left zeroed in case > real-mode code is skipped. > > Note that only PVH skips the real-mode portion of the boot trampoline, > otherwise the only way to skip it is to set `no-real-mode` on the command line, > and the description for the option already notes that VGA would be disabled as > a result of skipping real-mode bootstrap. IIRC, Grub defaults to using no-real-mode for xen.efi. It's definitely a common option to find used in practice. > > This fixes Xen incorrectly reporting: > > (XEN) Video information: > (XEN) VGA is text mode 80x25, font 8x16 > > When booted as a PVH guest. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reivewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-03-28 19:58 ` Andrew Cooper @ 2024-04-02 9:54 ` Jan Beulich 2024-04-02 10:06 ` Andrew Cooper 0 siblings, 1 reply; 13+ messages in thread From: Jan Beulich @ 2024-04-02 9:54 UTC (permalink / raw) To: Andrew Cooper, Roger Pau Monne; +Cc: xen-devel On 28.03.2024 20:58, Andrew Cooper wrote: > On 28/03/2024 3:35 pm, Roger Pau Monne wrote: >> There's no reason to assume VGA text mode 3 to be unconditionally available. >> With the addition of booting Xen itself in PVH mode there's a boot path that >> explicitly short-circuits all the real-mode logic, including the VGA detection. >> >> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do >> not populate boot_vid_info with any default settings. It will either be >> populated by the real-mode video detection code, or left zeroed in case >> real-mode code is skipped. >> >> Note that only PVH skips the real-mode portion of the boot trampoline, >> otherwise the only way to skip it is to set `no-real-mode` on the command line, >> and the description for the option already notes that VGA would be disabled as >> a result of skipping real-mode bootstrap. > > IIRC, Grub defaults to using no-real-mode for xen.efi. That's our MB2 entry path which forces skip_realmode to 1. The xen.efi boot path doesn't, but similarly skips entering real mode (retrieving desired data via EFI protocols instead). Imo if to be retained, the above paragraph would want extending some, to cover all the cases. Jan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-04-02 9:54 ` Jan Beulich @ 2024-04-02 10:06 ` Andrew Cooper 0 siblings, 0 replies; 13+ messages in thread From: Andrew Cooper @ 2024-04-02 10:06 UTC (permalink / raw) To: Jan Beulich, Roger Pau Monne; +Cc: xen-devel On 02/04/2024 10:54 am, Jan Beulich wrote: > On 28.03.2024 20:58, Andrew Cooper wrote: >> On 28/03/2024 3:35 pm, Roger Pau Monne wrote: >>> There's no reason to assume VGA text mode 3 to be unconditionally available. >>> With the addition of booting Xen itself in PVH mode there's a boot path that >>> explicitly short-circuits all the real-mode logic, including the VGA detection. >>> >>> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do >>> not populate boot_vid_info with any default settings. It will either be >>> populated by the real-mode video detection code, or left zeroed in case >>> real-mode code is skipped. >>> >>> Note that only PVH skips the real-mode portion of the boot trampoline, >>> otherwise the only way to skip it is to set `no-real-mode` on the command line, >>> and the description for the option already notes that VGA would be disabled as >>> a result of skipping real-mode bootstrap. >> IIRC, Grub defaults to using no-real-mode for xen.efi. > That's our MB2 entry path which forces skip_realmode to 1. The xen.efi boot > path doesn't, but similarly skips entering real mode (retrieving desired > data via EFI protocols instead). Imo if to be retained, the above paragraph > would want extending some, to cover all the cases. What I mean is that Grub's 20_linux_xen script writes a stanza which includes "no-real-mode edd=off" for any non-PC platform, which includes EFI. ~Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-03-28 15:35 ` [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present Roger Pau Monne 2024-03-28 19:58 ` Andrew Cooper @ 2024-04-02 9:49 ` Jan Beulich 2024-04-19 7:42 ` Roger Pau Monné 1 sibling, 1 reply; 13+ messages in thread From: Jan Beulich @ 2024-04-02 9:49 UTC (permalink / raw) To: Roger Pau Monne; +Cc: Andrew Cooper, xen-devel On 28.03.2024 16:35, Roger Pau Monne wrote: > There's no reason to assume VGA text mode 3 to be unconditionally available. > With the addition of booting Xen itself in PVH mode there's a boot path that > explicitly short-circuits all the real-mode logic, including the VGA detection. > > Leave the default user selected mode as text mode 3 in boot_vid_mode, but do > not populate boot_vid_info with any default settings. It will either be > populated by the real-mode video detection code, or left zeroed in case > real-mode code is skipped. > > Note that only PVH skips the real-mode portion of the boot trampoline, > otherwise the only way to skip it is to set `no-real-mode` on the command line, > and the description for the option already notes that VGA would be disabled as > a result of skipping real-mode bootstrap. > > This fixes Xen incorrectly reporting: > > (XEN) Video information: > (XEN) VGA is text mode 80x25, font 8x16 > > When booted as a PVH guest. And what effect does this have on a bare-metal boot with no-real-mode in use? The default on x86 hardware still is that in the absence of other information, a VGA of some kind can be assumed to be there. Yes, there are headless systems, but better assume VGA is there when there's not than the other way around. What I would have expected is for the PVH boot path to clear boot_vid_info. Jan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-04-02 9:49 ` Jan Beulich @ 2024-04-19 7:42 ` Roger Pau Monné 2024-04-19 9:26 ` Jan Beulich 0 siblings, 1 reply; 13+ messages in thread From: Roger Pau Monné @ 2024-04-19 7:42 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, xen-devel On Tue, Apr 02, 2024 at 11:49:20AM +0200, Jan Beulich wrote: > On 28.03.2024 16:35, Roger Pau Monne wrote: > > There's no reason to assume VGA text mode 3 to be unconditionally available. > > With the addition of booting Xen itself in PVH mode there's a boot path that > > explicitly short-circuits all the real-mode logic, including the VGA detection. > > > > Leave the default user selected mode as text mode 3 in boot_vid_mode, but do > > not populate boot_vid_info with any default settings. It will either be > > populated by the real-mode video detection code, or left zeroed in case > > real-mode code is skipped. > > > > Note that only PVH skips the real-mode portion of the boot trampoline, > > otherwise the only way to skip it is to set `no-real-mode` on the command line, > > and the description for the option already notes that VGA would be disabled as > > a result of skipping real-mode bootstrap. > > > > This fixes Xen incorrectly reporting: > > > > (XEN) Video information: > > (XEN) VGA is text mode 80x25, font 8x16 > > > > When booted as a PVH guest. > > And what effect does this have on a bare-metal boot with no-real-mode in use? > The default on x86 hardware still is that in the absence of other information, > a VGA of some kind can be assumed to be there. Yes, there are headless > systems, but better assume VGA is there when there's not than the other way > around. But that contradicts the text of the 'no-real-mode' option, which explicitly notes: "Do not execute real-mode bootstrap code when booting Xen. This option should not be used except for debugging. It will effectively disable the vga option, which relies on real mode to set the video mode." > What I would have expected is for the PVH boot path to clear boot_vid_info. Well, my intention was to fix both PVH and also make the implementation of the 'no-real-mode' option consistent with the documentation. Thanks, Roger. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present 2024-04-19 7:42 ` Roger Pau Monné @ 2024-04-19 9:26 ` Jan Beulich 0 siblings, 0 replies; 13+ messages in thread From: Jan Beulich @ 2024-04-19 9:26 UTC (permalink / raw) To: Roger Pau Monné; +Cc: Andrew Cooper, xen-devel On 19.04.2024 09:42, Roger Pau Monné wrote: > On Tue, Apr 02, 2024 at 11:49:20AM +0200, Jan Beulich wrote: >> On 28.03.2024 16:35, Roger Pau Monne wrote: >>> There's no reason to assume VGA text mode 3 to be unconditionally available. >>> With the addition of booting Xen itself in PVH mode there's a boot path that >>> explicitly short-circuits all the real-mode logic, including the VGA detection. >>> >>> Leave the default user selected mode as text mode 3 in boot_vid_mode, but do >>> not populate boot_vid_info with any default settings. It will either be >>> populated by the real-mode video detection code, or left zeroed in case >>> real-mode code is skipped. >>> >>> Note that only PVH skips the real-mode portion of the boot trampoline, >>> otherwise the only way to skip it is to set `no-real-mode` on the command line, >>> and the description for the option already notes that VGA would be disabled as >>> a result of skipping real-mode bootstrap. >>> >>> This fixes Xen incorrectly reporting: >>> >>> (XEN) Video information: >>> (XEN) VGA is text mode 80x25, font 8x16 >>> >>> When booted as a PVH guest. >> >> And what effect does this have on a bare-metal boot with no-real-mode in use? >> The default on x86 hardware still is that in the absence of other information, >> a VGA of some kind can be assumed to be there. Yes, there are headless >> systems, but better assume VGA is there when there's not than the other way >> around. > > But that contradicts the text of the 'no-real-mode' option, which > explicitly notes: > > "Do not execute real-mode bootstrap code when booting Xen. This option > should not be used except for debugging. It will effectively disable > the vga option, which relies on real mode to set the video mode." Well. Even without setting a video mode, _some_ mode is set (by firmware) as long as there is a VGA. In the absence of a "vga=" option iirc we'd retrieve that setting, unless we're not allowed to by "no-real-mode". In which case, as indicated, we may still be better off guessing a basic mode than kind of suggesting the absence of any VGA. Besides, when booting from EFI vga= is respected despite not going through real mode. So the quoted text isn't quite right anyway. Jan >> What I would have expected is for the PVH boot path to clear boot_vid_info. > > Well, my intention was to fix both PVH and also make the > implementation of the 'no-real-mode' option consistent with the > documentation. > > Thanks, Roger. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-04-19 9:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-28 15:35 [PATCH 0/2] x86/video: improve early video detection Roger Pau Monne 2024-03-28 15:35 ` [PATCH 1/2] x86/video: add boot_video_info offset generation to asm-offsets Roger Pau Monne 2024-03-28 19:55 ` Andrew Cooper 2024-04-02 9:38 ` Jan Beulich 2024-04-02 9:43 ` Jan Beulich 2024-04-19 7:25 ` Roger Pau Monné 2024-03-28 15:35 ` [PATCH 2/2] x86/video: do not assume a video mode to be unconditionally present Roger Pau Monne 2024-03-28 19:58 ` Andrew Cooper 2024-04-02 9:54 ` Jan Beulich 2024-04-02 10:06 ` Andrew Cooper 2024-04-02 9:49 ` Jan Beulich 2024-04-19 7:42 ` Roger Pau Monné 2024-04-19 9:26 ` Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.