* [PATCH] firmware/vgabios: Port PCI based VBE LFB discovery method from QEMU fork
@ 2017-05-10 14:31 Igor Druzhinin
2017-06-13 14:04 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Igor Druzhinin @ 2017-05-10 14:31 UTC (permalink / raw)
To: xen-devel
Cc: Igor Druzhinin, Ian Jackson, Wei Liu, Jan Beulich, Andrew Cooper
QEMU-traditional implements non-standard VBE registers for getting LFB
physical address from inside of VGA BIOS code. QEMU doesn't have
those registers implemented and returns 0 when an HVM guest is trying to
access them from the existing ROMBIOS code. This eventually leads to
a triple fault inside a guest which happened to use ROMBIOS instead of
SeaBIOS when in stdvga mode.
QEMU maintains its own fork of VGA BIOS where the VBE LFB discovery is
implemented through a regular PCI BAR reading. In order to support that
we need to build a PCI compliant VGA BIOS version for stdvga and include
it into ROMBIOS instead of the old one.
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/firmware/hvmloader/Makefile | 2 +-
tools/firmware/vgabios/Makefile | 29 +++++++++++++++--
tools/firmware/vgabios/vbe.c | 9 ++++++
tools/firmware/vgabios/vgabios.c | 68 +++++++++++++++++++++++++++++++++++++++
4 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 80d7b44..5f6eacd 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -45,7 +45,7 @@ CIRRUSVGA_DEBUG ?= n
ROMBIOS_DIR := ../rombios
ifeq ($(CONFIG_ROMBIOS),y)
-STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.bin
+STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.stdvga.bin
ifeq ($(CIRRUSVGA_DEBUG),y)
CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
else
diff --git a/tools/firmware/vgabios/Makefile b/tools/firmware/vgabios/Makefile
index 3284812..0f4026e 100644
--- a/tools/firmware/vgabios/Makefile
+++ b/tools/firmware/vgabios/Makefile
@@ -11,7 +11,7 @@ RELVERS = `pwd | sed "s-.*/--" | sed "s/vgabios//" | sed "s/-//"`
VGABIOS_DATE = "-DVGABIOS_DATE=\"$(VGABIOS_REL_DATE)\""
.PHONY: all
-all: bios cirrus-bios
+all: bios cirrus-bios stdvga-bios
.PHONY: bios
bios: biossums vgabios.bin vgabios.debug.bin
@@ -19,6 +19,9 @@ bios: biossums vgabios.bin vgabios.debug.bin
.PHONY: cirrus-bios
cirrus-bios: vgabios-cirrus.bin vgabios-cirrus.debug.bin
+.PHONY: stdvga-bios
+stdvga-bios: vgabios-stdvga.bin vgabios-stdvga.debug.bin
+
.PHONY: clean
clean:
rm -f biossums vbetables-gen vbetables.h *.o *.s *.ld86 \
@@ -30,13 +33,15 @@ distclean: clean
.PHONY: release
release:
- VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios
+ VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios stdvga-bios
/bin/rm -f *.o *.s *.ld86 \
temp.awk.* vgabios.*.orig _vgabios_.*.c core *.bak .#*
cp VGABIOS-lgpl-latest.bin ../$(RELEASE).bin
cp VGABIOS-lgpl-latest.debug.bin ../$(RELEASE).debug.bin
cp VGABIOS-lgpl-latest.cirrus.bin ../$(RELEASE).cirrus.bin
cp VGABIOS-lgpl-latest.cirrus.debug.bin ../$(RELEASE).cirrus.debug.bin
+ cp VGABIOS-lgpl-latest.stdvga.bin ../$(RELEASE).stdvga.bin
+ cp VGABIOS-lgpl-latest.stdvga.debug.bin ../$(RELEASE).stdvga.debug.bin
tar czvf ../$(RELEASE).tgz --exclude CVS -C .. $(RELEASE)/
vgabios.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
@@ -59,6 +64,26 @@ vgabios.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe
./biossums VGABIOS-lgpl-latest.debug.bin
ls -l VGABIOS-lgpl-latest.debug.bin
+vgabios-stdvga.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
+ $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 $(VGABIOS_DATE) > _vgabios-stdvga_.c
+ $(BCC) -o vgabios-stdvga.s -C-c -D__i86__ -S -0 _vgabios-stdvga_.c
+ sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga.s > _vgabios-stdvga_.s
+ $(AS86) _vgabios-stdvga_.s -b vgabios-stdvga.bin -u -w- -g -0 -j -O -l vgabios-stdvga.txt
+ rm -f _vgabios-stdvga_.s _vgabios-stdvga_.c vgabios-stdvga.s
+ cp vgabios-stdvga.bin VGABIOS-lgpl-latest.stdvga.bin
+ ./biossums VGABIOS-lgpl-latest.stdvga.bin
+ ls -l VGABIOS-lgpl-latest.stdvga.bin
+
+vgabios-stdvga.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
+ $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 -DDEBUG $(VGABIOS_DATE) > _vgabios-stdvga-debug_.c
+ $(BCC) -o vgabios-stdvga-debug.s -C-c -D__i86__ -S -0 _vgabios-stdvga-debug_.c
+ sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga-debug.s > _vgabios-stdvga-debug_.s
+ $(AS86) _vgabios-stdvga-debug_.s -b vgabios-stdvga-debug.bin -u -w- -g -0 -j -O -l vgabios-stdvga-debug.txt
+ rm -f _vgabios-stdvga-debug_.s _vgabios-stdvga-debug_.c vgabios-stdvga-debug.s
+ cp vgabios-stdvga-debug.bin VGABIOS-lgpl-latest.stdvga.debug.bin
+ ./biossums VGABIOS-lgpl-latest.stdvga.debug.bin
+ ls -l VGABIOS-lgpl-latest.stdvga.debug.bin
+
vgabios-cirrus.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h clext.c
$(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DCIRRUS -DPCIBIOS $(VGABIOS_DATE) > _vgabios-cirrus_.c
$(BCC) -o vgabios-cirrus.s -C-c -D__i86__ -S -0 _vgabios-cirrus_.c
diff --git a/tools/firmware/vgabios/vbe.c b/tools/firmware/vgabios/vbe.c
index c506690..d7706f5 100644
--- a/tools/firmware/vgabios/vbe.c
+++ b/tools/firmware/vgabios/vbe.c
@@ -914,6 +914,7 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
ModeInfoListItem *cur_info;
Boolean using_lfb;
ModeInfoBlockCompact info;
+ Bit16u lfb_addr=0;
#ifdef DEBUG
printf("VBE vbe_biosfn_return_mode_information ES%x DI%x CX%x\n",ES,DI,CX);
@@ -957,6 +958,14 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
outw(VBE_DISPI_IOPORT_INDEX,VBE_DISPI_INDEX_LFB_ADDRESS_L);
info.PhysBasePtr |= inw(VBE_DISPI_IOPORT_DATA);
#endif
+#ifdef PCI_VID
+ if ((Bit16u)(info.PhysBasePtr >> 16) == 0 &&
+ (Bit16u)info.PhysBasePtr == 0)
+ lfb_addr = pci_get_lfb_addr(PCI_VID);
+
+ if (lfb_addr > 0)
+ info.PhysBasePtr = ((Bit32u)lfb_addr << 16);
+#endif
result = 0x4f;
// copy updates in mode_info_block back
diff --git a/tools/firmware/vgabios/vgabios.c b/tools/firmware/vgabios/vgabios.c
index 1c75b7d..22471c5 100644
--- a/tools/firmware/vgabios/vgabios.c
+++ b/tools/firmware/vgabios/vgabios.c
@@ -209,8 +209,13 @@ vgabios_pci_data:
.word 0x1013
.word 0x00b8 // CLGD5446
#else
+#ifdef PCI_VID
+.word PCI_VID
+.word PCI_DID
+#else
#error "Unknown PCI vendor and device id"
#endif
+#endif
.word 0 // reserved
.word 0x18 // dlen
.byte 0 // revision
@@ -3829,6 +3834,69 @@ void printf(s)
}
#endif
+ASM_START
+ ; get LFB address from PCI
+ ; in - ax: PCI device vendor
+ ; out - ax: LFB address (high 16 bit)
+ ;; NOTE - may be called in protected mode
+_pci_get_lfb_addr:
+ push bx
+ push cx
+ push dx
+ push eax
+ mov bx, ax
+ xor cx, cx
+ mov dl, #0x00
+ call pci_read_reg
+ cmp ax, #0xffff
+ jz pci_get_lfb_addr_fail
+ pci_get_lfb_addr_next_dev:
+ mov dl, #0x00
+ call pci_read_reg
+ cmp ax, bx ;; check vendor
+ jz pci_get_lfb_addr_found
+ add cx, #0x8
+ cmp cx, #0x200 ;; search bus #0 and #1
+ jb pci_get_lfb_addr_next_dev
+ pci_get_lfb_addr_fail:
+ xor dx, dx ;; no LFB
+ jmp pci_get_lfb_addr_return
+ pci_get_lfb_addr_found:
+ mov dl, #0x10 ;; I/O space #0
+ call pci_read_reg
+ test ax, #0xfff1
+ jz pci_get_lfb_addr_success
+ mov dl, #0x14 ;; I/O space #1
+ call pci_read_reg
+ test ax, #0xfff1
+ jnz pci_get_lfb_addr_fail
+ pci_get_lfb_addr_success:
+ shr eax, #16
+ mov dx, ax ;; LFB address
+ pci_get_lfb_addr_return:
+ pop eax
+ mov ax, dx
+ pop dx
+ pop cx
+ pop bx
+ ret
+
+ ; read PCI register
+ ; in - cx: device/function
+ ; in - dl: register
+ ; out - eax: value
+pci_read_reg:
+ mov eax, #0x00800000
+ mov ax, cx
+ shl eax, #8
+ mov al, dl
+ mov dx, #0xcf8
+ out dx, eax
+ add dl, #4
+ in eax, dx
+ ret
+ASM_END
+
#ifdef VBE
#include "vbe.c"
#endif
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] firmware/vgabios: Port PCI based VBE LFB discovery method from QEMU fork
2017-05-10 14:31 [PATCH] firmware/vgabios: Port PCI based VBE LFB discovery method from QEMU fork Igor Druzhinin
@ 2017-06-13 14:04 ` Andrew Cooper
2017-06-13 15:35 ` Igor Druzhinin
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2017-06-13 14:04 UTC (permalink / raw)
To: Igor Druzhinin, xen-devel; +Cc: Wei Liu, Ian Jackson, Jan Beulich
On 10/05/17 15:31, Igor Druzhinin wrote:
> QEMU-traditional implements non-standard VBE registers for getting LFB
> physical address from inside of VGA BIOS code. QEMU doesn't have
> those registers implemented and returns 0 when an HVM guest is trying to
> access them from the existing ROMBIOS code. This eventually leads to
> a triple fault inside a guest which happened to use ROMBIOS instead of
> SeaBIOS when in stdvga mode.
>
> QEMU maintains its own fork of VGA BIOS where the VBE LFB discovery is
> implemented through a regular PCI BAR reading. In order to support that
> we need to build a PCI compliant VGA BIOS version for stdvga and include
> it into ROMBIOS instead of the old one.
>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
How much of this is ported from existing changes elsewhere?
> ---
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/firmware/hvmloader/Makefile | 2 +-
> tools/firmware/vgabios/Makefile | 29 +++++++++++++++--
> tools/firmware/vgabios/vbe.c | 9 ++++++
> tools/firmware/vgabios/vgabios.c | 68 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> index 80d7b44..5f6eacd 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -45,7 +45,7 @@ CIRRUSVGA_DEBUG ?= n
> ROMBIOS_DIR := ../rombios
>
> ifeq ($(CONFIG_ROMBIOS),y)
> -STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.bin
> +STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.stdvga.bin
> ifeq ($(CIRRUSVGA_DEBUG),y)
> CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
> else
> diff --git a/tools/firmware/vgabios/Makefile b/tools/firmware/vgabios/Makefile
> index 3284812..0f4026e 100644
> --- a/tools/firmware/vgabios/Makefile
> +++ b/tools/firmware/vgabios/Makefile
> @@ -11,7 +11,7 @@ RELVERS = `pwd | sed "s-.*/--" | sed "s/vgabios//" | sed "s/-//"`
> VGABIOS_DATE = "-DVGABIOS_DATE=\"$(VGABIOS_REL_DATE)\""
>
> .PHONY: all
> -all: bios cirrus-bios
> +all: bios cirrus-bios stdvga-bios
>
> .PHONY: bios
> bios: biossums vgabios.bin vgabios.debug.bin
> @@ -19,6 +19,9 @@ bios: biossums vgabios.bin vgabios.debug.bin
> .PHONY: cirrus-bios
> cirrus-bios: vgabios-cirrus.bin vgabios-cirrus.debug.bin
>
> +.PHONY: stdvga-bios
> +stdvga-bios: vgabios-stdvga.bin vgabios-stdvga.debug.bin
> +
> .PHONY: clean
> clean:
> rm -f biossums vbetables-gen vbetables.h *.o *.s *.ld86 \
> @@ -30,13 +33,15 @@ distclean: clean
>
> .PHONY: release
> release:
> - VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios
> + VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios stdvga-bios
> /bin/rm -f *.o *.s *.ld86 \
> temp.awk.* vgabios.*.orig _vgabios_.*.c core *.bak .#*
> cp VGABIOS-lgpl-latest.bin ../$(RELEASE).bin
> cp VGABIOS-lgpl-latest.debug.bin ../$(RELEASE).debug.bin
> cp VGABIOS-lgpl-latest.cirrus.bin ../$(RELEASE).cirrus.bin
> cp VGABIOS-lgpl-latest.cirrus.debug.bin ../$(RELEASE).cirrus.debug.bin
> + cp VGABIOS-lgpl-latest.stdvga.bin ../$(RELEASE).stdvga.bin
> + cp VGABIOS-lgpl-latest.stdvga.debug.bin ../$(RELEASE).stdvga.debug.bin
> tar czvf ../$(RELEASE).tgz --exclude CVS -C .. $(RELEASE)/
>
> vgabios.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
> @@ -59,6 +64,26 @@ vgabios.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe
> ./biossums VGABIOS-lgpl-latest.debug.bin
> ls -l VGABIOS-lgpl-latest.debug.bin
>
> +vgabios-stdvga.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
> + $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 $(VGABIOS_DATE) > _vgabios-stdvga_.c
The general makefile-ary around here is in serious need of improvement,
although it would be better to not merge that with a functional fix.
However, given that all the cirrus is behind -DCIRRUS, wouldn't it be
better to use -DSTDVGA here?
> + $(BCC) -o vgabios-stdvga.s -C-c -D__i86__ -S -0 _vgabios-stdvga_.c
> + sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga.s > _vgabios-stdvga_.s
> + $(AS86) _vgabios-stdvga_.s -b vgabios-stdvga.bin -u -w- -g -0 -j -O -l vgabios-stdvga.txt
> + rm -f _vgabios-stdvga_.s _vgabios-stdvga_.c vgabios-stdvga.s
> + cp vgabios-stdvga.bin VGABIOS-lgpl-latest.stdvga.bin
> + ./biossums VGABIOS-lgpl-latest.stdvga.bin
> + ls -l VGABIOS-lgpl-latest.stdvga.bin
> +
> +vgabios-stdvga.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
> + $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 -DDEBUG $(VGABIOS_DATE) > _vgabios-stdvga-debug_.c
> + $(BCC) -o vgabios-stdvga-debug.s -C-c -D__i86__ -S -0 _vgabios-stdvga-debug_.c
> + sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga-debug.s > _vgabios-stdvga-debug_.s
> + $(AS86) _vgabios-stdvga-debug_.s -b vgabios-stdvga-debug.bin -u -w- -g -0 -j -O -l vgabios-stdvga-debug.txt
> + rm -f _vgabios-stdvga-debug_.s _vgabios-stdvga-debug_.c vgabios-stdvga-debug.s
> + cp vgabios-stdvga-debug.bin VGABIOS-lgpl-latest.stdvga.debug.bin
> + ./biossums VGABIOS-lgpl-latest.stdvga.debug.bin
> + ls -l VGABIOS-lgpl-latest.stdvga.debug.bin
> +
> vgabios-cirrus.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h clext.c
> $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DCIRRUS -DPCIBIOS $(VGABIOS_DATE) > _vgabios-cirrus_.c
> $(BCC) -o vgabios-cirrus.s -C-c -D__i86__ -S -0 _vgabios-cirrus_.c
> diff --git a/tools/firmware/vgabios/vbe.c b/tools/firmware/vgabios/vbe.c
> index c506690..d7706f5 100644
> --- a/tools/firmware/vgabios/vbe.c
> +++ b/tools/firmware/vgabios/vbe.c
> @@ -914,6 +914,7 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
> ModeInfoListItem *cur_info;
> Boolean using_lfb;
> ModeInfoBlockCompact info;
> + Bit16u lfb_addr=0;
>
> #ifdef DEBUG
> printf("VBE vbe_biosfn_return_mode_information ES%x DI%x CX%x\n",ES,DI,CX);
> @@ -957,6 +958,14 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
> outw(VBE_DISPI_IOPORT_INDEX,VBE_DISPI_INDEX_LFB_ADDRESS_L);
> info.PhysBasePtr |= inw(VBE_DISPI_IOPORT_DATA);
> #endif
> +#ifdef PCI_VID
> + if ((Bit16u)(info.PhysBasePtr >> 16) == 0 &&
> + (Bit16u)info.PhysBasePtr == 0)
> + lfb_addr = pci_get_lfb_addr(PCI_VID);
> +
> + if (lfb_addr > 0)
> + info.PhysBasePtr = ((Bit32u)lfb_addr << 16);
> +#endif
> result = 0x4f;
>
> // copy updates in mode_info_block back
> diff --git a/tools/firmware/vgabios/vgabios.c b/tools/firmware/vgabios/vgabios.c
> index 1c75b7d..22471c5 100644
> --- a/tools/firmware/vgabios/vgabios.c
> +++ b/tools/firmware/vgabios/vgabios.c
> @@ -209,8 +209,13 @@ vgabios_pci_data:
> .word 0x1013
> .word 0x00b8 // CLGD5446
> #else
> +#ifdef PCI_VID
This probably wants to be an #elif defined(STDVGA) to avoid the repeated
#endif's after the error.
~Andrew
> +.word PCI_VID
> +.word PCI_DID
> +#else
> #error "Unknown PCI vendor and device id"
> #endif
> +#endif
> .word 0 // reserved
> .word 0x18 // dlen
> .byte 0 // revision
> @@ -3829,6 +3834,69 @@ void printf(s)
> }
> #endif
>
> +ASM_START
> + ; get LFB address from PCI
> + ; in - ax: PCI device vendor
> + ; out - ax: LFB address (high 16 bit)
> + ;; NOTE - may be called in protected mode
> +_pci_get_lfb_addr:
> + push bx
> + push cx
> + push dx
> + push eax
> + mov bx, ax
> + xor cx, cx
> + mov dl, #0x00
> + call pci_read_reg
> + cmp ax, #0xffff
> + jz pci_get_lfb_addr_fail
> + pci_get_lfb_addr_next_dev:
> + mov dl, #0x00
> + call pci_read_reg
> + cmp ax, bx ;; check vendor
> + jz pci_get_lfb_addr_found
> + add cx, #0x8
> + cmp cx, #0x200 ;; search bus #0 and #1
> + jb pci_get_lfb_addr_next_dev
> + pci_get_lfb_addr_fail:
> + xor dx, dx ;; no LFB
> + jmp pci_get_lfb_addr_return
> + pci_get_lfb_addr_found:
> + mov dl, #0x10 ;; I/O space #0
> + call pci_read_reg
> + test ax, #0xfff1
> + jz pci_get_lfb_addr_success
> + mov dl, #0x14 ;; I/O space #1
> + call pci_read_reg
> + test ax, #0xfff1
> + jnz pci_get_lfb_addr_fail
> + pci_get_lfb_addr_success:
> + shr eax, #16
> + mov dx, ax ;; LFB address
> + pci_get_lfb_addr_return:
> + pop eax
> + mov ax, dx
> + pop dx
> + pop cx
> + pop bx
> + ret
> +
> + ; read PCI register
> + ; in - cx: device/function
> + ; in - dl: register
> + ; out - eax: value
> +pci_read_reg:
> + mov eax, #0x00800000
> + mov ax, cx
> + shl eax, #8
> + mov al, dl
> + mov dx, #0xcf8
> + out dx, eax
> + add dl, #4
> + in eax, dx
> + ret
> +ASM_END
> +
> #ifdef VBE
> #include "vbe.c"
> #endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] firmware/vgabios: Port PCI based VBE LFB discovery method from QEMU fork
2017-06-13 14:04 ` Andrew Cooper
@ 2017-06-13 15:35 ` Igor Druzhinin
0 siblings, 0 replies; 3+ messages in thread
From: Igor Druzhinin @ 2017-06-13 15:35 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: Wei Liu, Ian Jackson, Jan Beulich
On 13/06/17 15:04, Andrew Cooper wrote:
> On 10/05/17 15:31, Igor Druzhinin wrote:
>> QEMU-traditional implements non-standard VBE registers for getting LFB
>> physical address from inside of VGA BIOS code. QEMU doesn't have
>> those registers implemented and returns 0 when an HVM guest is trying to
>> access them from the existing ROMBIOS code. This eventually leads to
>> a triple fault inside a guest which happened to use ROMBIOS instead of
>> SeaBIOS when in stdvga mode.
>>
>> QEMU maintains its own fork of VGA BIOS where the VBE LFB discovery is
>> implemented through a regular PCI BAR reading. In order to support that
>> we need to build a PCI compliant VGA BIOS version for stdvga and include
>> it into ROMBIOS instead of the old one.
>>
>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>
> How much of this is ported from existing changes elsewhere?
>
Only ASM functions below for PCI conf space accessing are ported from
vgabios fork of QEMU. If I need to incorporate this somehow into the
commit message, could you point me to an example of doing this properly?
>> ---
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Jackson <ian.jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>> tools/firmware/hvmloader/Makefile | 2 +-
>> tools/firmware/vgabios/Makefile | 29 +++++++++++++++--
>> tools/firmware/vgabios/vbe.c | 9 ++++++
>> tools/firmware/vgabios/vgabios.c | 68 +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 105 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
>> index 80d7b44..5f6eacd 100644
>> --- a/tools/firmware/hvmloader/Makefile
>> +++ b/tools/firmware/hvmloader/Makefile
>> @@ -45,7 +45,7 @@ CIRRUSVGA_DEBUG ?= n
>> ROMBIOS_DIR := ../rombios
>>
>> ifeq ($(CONFIG_ROMBIOS),y)
>> -STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.bin
>> +STDVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.stdvga.bin
>> ifeq ($(CIRRUSVGA_DEBUG),y)
>> CIRRUSVGA_ROM := ../vgabios/VGABIOS-lgpl-latest.cirrus.debug.bin
>> else
>> diff --git a/tools/firmware/vgabios/Makefile b/tools/firmware/vgabios/Makefile
>> index 3284812..0f4026e 100644
>> --- a/tools/firmware/vgabios/Makefile
>> +++ b/tools/firmware/vgabios/Makefile
>> @@ -11,7 +11,7 @@ RELVERS = `pwd | sed "s-.*/--" | sed "s/vgabios//" | sed "s/-//"`
>> VGABIOS_DATE = "-DVGABIOS_DATE=\"$(VGABIOS_REL_DATE)\""
>>
>> .PHONY: all
>> -all: bios cirrus-bios
>> +all: bios cirrus-bios stdvga-bios
>>
>> .PHONY: bios
>> bios: biossums vgabios.bin vgabios.debug.bin
>> @@ -19,6 +19,9 @@ bios: biossums vgabios.bin vgabios.debug.bin
>> .PHONY: cirrus-bios
>> cirrus-bios: vgabios-cirrus.bin vgabios-cirrus.debug.bin
>>
>> +.PHONY: stdvga-bios
>> +stdvga-bios: vgabios-stdvga.bin vgabios-stdvga.debug.bin
>> +
>> .PHONY: clean
>> clean:
>> rm -f biossums vbetables-gen vbetables.h *.o *.s *.ld86 \
>> @@ -30,13 +33,15 @@ distclean: clean
>>
>> .PHONY: release
>> release:
>> - VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios
>> + VGABIOS_VERS=\"-DVGABIOS_VERS=\\\"$(RELVERS)\\\"\" make bios cirrus-bios stdvga-bios
>> /bin/rm -f *.o *.s *.ld86 \
>> temp.awk.* vgabios.*.orig _vgabios_.*.c core *.bak .#*
>> cp VGABIOS-lgpl-latest.bin ../$(RELEASE).bin
>> cp VGABIOS-lgpl-latest.debug.bin ../$(RELEASE).debug.bin
>> cp VGABIOS-lgpl-latest.cirrus.bin ../$(RELEASE).cirrus.bin
>> cp VGABIOS-lgpl-latest.cirrus.debug.bin ../$(RELEASE).cirrus.debug.bin
>> + cp VGABIOS-lgpl-latest.stdvga.bin ../$(RELEASE).stdvga.bin
>> + cp VGABIOS-lgpl-latest.stdvga.debug.bin ../$(RELEASE).stdvga.debug.bin
>> tar czvf ../$(RELEASE).tgz --exclude CVS -C .. $(RELEASE)/
>>
>> vgabios.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
>> @@ -59,6 +64,26 @@ vgabios.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe
>> ./biossums VGABIOS-lgpl-latest.debug.bin
>> ls -l VGABIOS-lgpl-latest.debug.bin
>>
>> +vgabios-stdvga.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
>> + $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 $(VGABIOS_DATE) > _vgabios-stdvga_.c
>
> The general makefile-ary around here is in serious need of improvement,
> although it would be better to not merge that with a functional fix.
> However, given that all the cirrus is behind -DCIRRUS, wouldn't it be
> better to use -DSTDVGA here?
>
Right, probably worth it. Also, I can remove the old vgabios binary from
building since it seems we don't need this anymore.
>> + $(BCC) -o vgabios-stdvga.s -C-c -D__i86__ -S -0 _vgabios-stdvga_.c
>> + sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga.s > _vgabios-stdvga_.s
>> + $(AS86) _vgabios-stdvga_.s -b vgabios-stdvga.bin -u -w- -g -0 -j -O -l vgabios-stdvga.txt
>> + rm -f _vgabios-stdvga_.s _vgabios-stdvga_.c vgabios-stdvga.s
>> + cp vgabios-stdvga.bin VGABIOS-lgpl-latest.stdvga.bin
>> + ./biossums VGABIOS-lgpl-latest.stdvga.bin
>> + ls -l VGABIOS-lgpl-latest.stdvga.bin
>> +
>> +vgabios-stdvga.debug.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h vbe.h vbe.c vbetables.h
>> + $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DVBE -DPCIBIOS -DPCI_VID=0x1234 -DPCI_DID=0x1111 -DDEBUG $(VGABIOS_DATE) > _vgabios-stdvga-debug_.c
>> + $(BCC) -o vgabios-stdvga-debug.s -C-c -D__i86__ -S -0 _vgabios-stdvga-debug_.c
>> + sed -e 's/^\.text//' -e 's/^\.data//' vgabios-stdvga-debug.s > _vgabios-stdvga-debug_.s
>> + $(AS86) _vgabios-stdvga-debug_.s -b vgabios-stdvga-debug.bin -u -w- -g -0 -j -O -l vgabios-stdvga-debug.txt
>> + rm -f _vgabios-stdvga-debug_.s _vgabios-stdvga-debug_.c vgabios-stdvga-debug.s
>> + cp vgabios-stdvga-debug.bin VGABIOS-lgpl-latest.stdvga.debug.bin
>> + ./biossums VGABIOS-lgpl-latest.stdvga.debug.bin
>> + ls -l VGABIOS-lgpl-latest.stdvga.debug.bin
>> +
>> vgabios-cirrus.bin: biossums vgabios.c vgabios.h vgafonts.h vgatables.h clext.c
>> $(GCC) -E -P vgabios.c $(VGABIOS_VERS) -DCIRRUS -DPCIBIOS $(VGABIOS_DATE) > _vgabios-cirrus_.c
>> $(BCC) -o vgabios-cirrus.s -C-c -D__i86__ -S -0 _vgabios-cirrus_.c
>> diff --git a/tools/firmware/vgabios/vbe.c b/tools/firmware/vgabios/vbe.c
>> index c506690..d7706f5 100644
>> --- a/tools/firmware/vgabios/vbe.c
>> +++ b/tools/firmware/vgabios/vbe.c
>> @@ -914,6 +914,7 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
>> ModeInfoListItem *cur_info;
>> Boolean using_lfb;
>> ModeInfoBlockCompact info;
>> + Bit16u lfb_addr=0;
>>
>> #ifdef DEBUG
>> printf("VBE vbe_biosfn_return_mode_information ES%x DI%x CX%x\n",ES,DI,CX);
>> @@ -957,6 +958,14 @@ Bit16u *AX;Bit16u CX; Bit16u ES;Bit16u DI;
>> outw(VBE_DISPI_IOPORT_INDEX,VBE_DISPI_INDEX_LFB_ADDRESS_L);
>> info.PhysBasePtr |= inw(VBE_DISPI_IOPORT_DATA);
>> #endif
>> +#ifdef PCI_VID
>> + if ((Bit16u)(info.PhysBasePtr >> 16) == 0 &&
>> + (Bit16u)info.PhysBasePtr == 0)
>> + lfb_addr = pci_get_lfb_addr(PCI_VID);
>> +
>> + if (lfb_addr > 0)
>> + info.PhysBasePtr = ((Bit32u)lfb_addr << 16);
>> +#endif
>> result = 0x4f;
>>
>> // copy updates in mode_info_block back
>> diff --git a/tools/firmware/vgabios/vgabios.c b/tools/firmware/vgabios/vgabios.c
>> index 1c75b7d..22471c5 100644
>> --- a/tools/firmware/vgabios/vgabios.c
>> +++ b/tools/firmware/vgabios/vgabios.c
>> @@ -209,8 +209,13 @@ vgabios_pci_data:
>> .word 0x1013
>> .word 0x00b8 // CLGD5446
>> #else
>> +#ifdef PCI_VID
>
> This probably wants to be an #elif defined(STDVGA) to avoid the repeated
> #endif's after the error.
This is a side effect of having the old and the new vgabios binaries
building.
Igor
>
> ~Andrew
>
>> +.word PCI_VID
>> +.word PCI_DID
>> +#else
>> #error "Unknown PCI vendor and device id"
>> #endif
>> +#endif
>> .word 0 // reserved
>> .word 0x18 // dlen
>> .byte 0 // revision
>> @@ -3829,6 +3834,69 @@ void printf(s)
>> }
>> #endif
>>
>> +ASM_START
>> + ; get LFB address from PCI
>> + ; in - ax: PCI device vendor
>> + ; out - ax: LFB address (high 16 bit)
>> + ;; NOTE - may be called in protected mode
>> +_pci_get_lfb_addr:
>> + push bx
>> + push cx
>> + push dx
>> + push eax
>> + mov bx, ax
>> + xor cx, cx
>> + mov dl, #0x00
>> + call pci_read_reg
>> + cmp ax, #0xffff
>> + jz pci_get_lfb_addr_fail
>> + pci_get_lfb_addr_next_dev:
>> + mov dl, #0x00
>> + call pci_read_reg
>> + cmp ax, bx ;; check vendor
>> + jz pci_get_lfb_addr_found
>> + add cx, #0x8
>> + cmp cx, #0x200 ;; search bus #0 and #1
>> + jb pci_get_lfb_addr_next_dev
>> + pci_get_lfb_addr_fail:
>> + xor dx, dx ;; no LFB
>> + jmp pci_get_lfb_addr_return
>> + pci_get_lfb_addr_found:
>> + mov dl, #0x10 ;; I/O space #0
>> + call pci_read_reg
>> + test ax, #0xfff1
>> + jz pci_get_lfb_addr_success
>> + mov dl, #0x14 ;; I/O space #1
>> + call pci_read_reg
>> + test ax, #0xfff1
>> + jnz pci_get_lfb_addr_fail
>> + pci_get_lfb_addr_success:
>> + shr eax, #16
>> + mov dx, ax ;; LFB address
>> + pci_get_lfb_addr_return:
>> + pop eax
>> + mov ax, dx
>> + pop dx
>> + pop cx
>> + pop bx
>> + ret
>> +
>> + ; read PCI register
>> + ; in - cx: device/function
>> + ; in - dl: register
>> + ; out - eax: value
>> +pci_read_reg:
>> + mov eax, #0x00800000
>> + mov ax, cx
>> + shl eax, #8
>> + mov al, dl
>> + mov dx, #0xcf8
>> + out dx, eax
>> + add dl, #4
>> + in eax, dx
>> + ret
>> +ASM_END
>> +
>> #ifdef VBE
>> #include "vbe.c"
>> #endif
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-13 15:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-10 14:31 [PATCH] firmware/vgabios: Port PCI based VBE LFB discovery method from QEMU fork Igor Druzhinin
2017-06-13 14:04 ` Andrew Cooper
2017-06-13 15:35 ` Igor Druzhinin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).