public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
@ 2018-10-23  2:40 yuchenlin
  2018-10-23 10:18 ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: yuchenlin @ 2018-10-23  2:40 UTC (permalink / raw)
  To: edk2-devel
  Cc: jordan.l.justen, lersek, ard.biesheuvel, anthony.perard,
	julien.grall, yuchenlin

From: yuchenlin <yuchenlin@synology.com>

When driver doesn't set fifo config, the vmsvga will fall back
to std vga. However, we don't initialize vbe related port. It
causes blank screen in qemu console.

This patch will fix "Guest has not initialized the display (yet)"
when using qemu -device vmware-svga with ovmf.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: yuchenlin <yuchenlin@synology.com>
---
 OvmfPkg/QemuVideoDxe/Driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 0dce80e59..255c01881 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -1067,8 +1067,7 @@ InitializeVmwareSvgaGraphicsMode (
 
   VmwareSvgaWrite (Private, VmwareSvgaRegEnable, 1);
 
-  SetDefaultPalette (Private);
-  ClearScreen (Private);
+  InitializeBochsGraphicsMode (Private, ModeData);
 }
 
 EFI_STATUS
-- 
2.18.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23  2:40 [PATCH] OvmfPkg: initialize bochs when initializing vmsvga yuchenlin
@ 2018-10-23 10:18 ` Laszlo Ersek
  2018-10-23 11:12   ` yuchenlin
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2018-10-23 10:18 UTC (permalink / raw)
  To: yuchenlin
  Cc: edk2-devel, jordan.l.justen, ard.biesheuvel, anthony.perard,
	julien.grall, Gerd Hoffmann, Phil Dennis-Jordan

(1) Adding Gerd (because he maintains video in QEMU), and Phil
Dennis-Jordan (for authoring commit c137d9508169, "OvmfPkg/QemuVideoDxe:
VMWare SVGA device support", 2017-04-07).


On 10/23/18 04:40, yuchenlin@synology.com wrote:
> From: yuchenlin <yuchenlin@synology.com>
> 
> When driver doesn't set fifo config, the vmsvga will fall back
> to std vga. However, we don't initialize vbe related port. It
> causes blank screen in qemu console.

(2) The words "when driver doesn't set fifo config" tell me nothing. The
QemuVideoDxe directory has zero instances of the word "fifo". The same
applies to "OvmfPkg/Include/IndustryStandard/VmwareSvga.h".

In addition, the "vmsvga will fall back to std vga" statement is also
unclear. Is that a statement about the QEMU device model's behavior?

I vaguely suspect that your intent might be to say, "QemuVideoDxe does
not perform a necessary configuration step, and therefore it cannot
drive QEMU's VMW SVGA device". However, if that is indeed your intent,
then I believe something must have changed recently in QEMU, because
QemuVideoDxe *definitely* worked when Phil contributed the VMW SVGA
driver logic, in commit c137d9508169.

Are we talking about a QEMU regression, or a driver-side configuration
step that QemuVideoDxe has always missed (and we're being punished for
it only now)?


> This patch will fix "Guest has not initialized the display (yet)"
> when using qemu -device vmware-svga with ovmf.

Right, as I write above, this definitely worked earlier. I suggest
bisecting QEMU (and/or testing older QEMU machine types) to identify the
QEMU side change. Once we know that, we can decide whether this is a
QEMU regression, or just exposing a long-standing OVMF bug.

Comments about the code below:


> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: yuchenlin <yuchenlin@synology.com>
> ---
>  OvmfPkg/QemuVideoDxe/Driver.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
> index 0dce80e59..255c01881 100644
> --- a/OvmfPkg/QemuVideoDxe/Driver.c
> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
> @@ -1067,8 +1067,7 @@ InitializeVmwareSvgaGraphicsMode (
>  
>    VmwareSvgaWrite (Private, VmwareSvgaRegEnable, 1);
>  
> -  SetDefaultPalette (Private);
> -  ClearScreen (Private);
> +  InitializeBochsGraphicsMode (Private, ModeData);
>  }
>  
>  EFI_STATUS
> 

(3) Calling InitializeBochsGraphicsMode() from within
InitializeVmwareSvgaGraphicsMode() seems wrong, considering the current
structure of the driver.

We only have InitializeXxxGraphicsMode() calls in
QemuVideoGraphicsOutputSetMode(). In order to determine which variant to
call, we check the "Private->Variant" field, in a "switch" statement.
Therefore:

(3a) If a general fallback from "VmwareSvga" to "Bochs" is necessary,
then the fallback logic should be added to earlier code that sets the
Variant field.

You can see an example for that in the QemuVideoControllerDriverStart()
function, near the debug message "QemuVideo: No mmio bar, fallback to
port io". There the Variant field is degraded from the originally
detected QEMU_VIDEO_BOCHS_MMIO value, to QEMU_VIDEO_BOCHS.

(3b) Or else, if calling InitializeVmwareSvgaGraphicsMode() is fine as a
basis, but we also need the actions of InitializeBochsGraphicsMode() *in
addition*, then please:

- extract the common actions from InitializeBochsGraphicsMode() to a new
helper function, and call the helper from both
InitializeBochsGraphicsMode() and InitializeVmwareSvgaGraphicsMode(),

- explain, in InitializeVmwareSvgaGraphicsMode(), *why* the Bochs config
actions are necessary, in addition to the VmwareSvga actions.

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23 10:18 ` Laszlo Ersek
@ 2018-10-23 11:12   ` yuchenlin
  2018-10-23 12:45     ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: yuchenlin @ 2018-10-23 11:12 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: edk2-devel, jordan.l.justen, ard.biesheuvel, anthony.perard,
	julien.grall, Gerd Hoffmann, Phil Dennis-Jordan

Hi, Laszlo

On 2018-10-23 18:18, Laszlo Ersek wrote:
> (1) Adding Gerd (because he maintains video in QEMU), and Phil
> Dennis-Jordan (for authoring commit c137d9508169, 
> "OvmfPkg/QemuVideoDxe:
> VMWare SVGA device support", 2017-04-07).
> 
> 
> On 10/23/18 04:40, yuchenlin@synology.com wrote:
>> From: yuchenlin <yuchenlin@synology.com>
>> 
>> When driver doesn't set fifo config, the vmsvga will fall back
>> to std vga. However, we don't initialize vbe related port. It
>> causes blank screen in qemu console.
> 
> (2) The words "when driver doesn't set fifo config" tell me nothing. 
> The
> QemuVideoDxe directory has zero instances of the word "fifo". The same
> applies to "OvmfPkg/Include/IndustryStandard/VmwareSvga.h".
> 
> In addition, the "vmsvga will fall back to std vga" statement is also
> unclear. Is that a statement about the QEMU device model's behavior?
> 

About FIFO, you can refer to 
https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L34,

"The main advantage of VMware's SVGA device over alternatives like VBE 
is that the virtual
machine can explicitly indicate which ares of the screen have changed by 
sending update rectangles
through the device's command FIFO."

According to 
https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L533,
to use vmsvga's FIFO. Driver should setup FIFO and set 
SVGA_REG_CONFIG_DONE to 1.
However, OVMF doesn't do it. In this case, vmsvga will fall back to vga. 
It is QEMU
device model's behavior after commit 104bd1dc70 (vmsvga: fix 
vmsvga_update_display).

> I vaguely suspect that your intent might be to say, "QemuVideoDxe does
> not perform a necessary configuration step, and therefore it cannot
> drive QEMU's VMW SVGA device". However, if that is indeed your intent,
> then I believe something must have changed recently in QEMU, because
> QemuVideoDxe *definitely* worked when Phil contributed the VMW SVGA
> driver logic, in commit c137d9508169.
> 
> Are we talking about a QEMU regression, or a driver-side configuration
> step that QemuVideoDxe has always missed (and we're being punished for
> it only now)?
> 

This is QEMU behavior change in commit 104bd1dc70 (vmsvga: fix 
vmsvga_update_display).
In my opinion, it is a correct change.

> 
>> This patch will fix "Guest has not initialized the display (yet)"
>> when using qemu -device vmware-svga with ovmf.
> 
> Right, as I write above, this definitely worked earlier. I suggest
> bisecting QEMU (and/or testing older QEMU machine types) to identify 
> the
> QEMU side change. Once we know that, we can decide whether this is a
> QEMU regression, or just exposing a long-standing OVMF bug.

In my opinion, it is a long-standing OVMF bug, which is based on the 
wrong behavior of QEMU's vmsvga.
It relied on dirty memory region to call dpy_gfx_update for updating 
display.

> 
> Comments about the code below:
> 
> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: yuchenlin <yuchenlin@synology.com>
>> ---
>>  OvmfPkg/QemuVideoDxe/Driver.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c 
>> b/OvmfPkg/QemuVideoDxe/Driver.c
>> index 0dce80e59..255c01881 100644
>> --- a/OvmfPkg/QemuVideoDxe/Driver.c
>> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
>> @@ -1067,8 +1067,7 @@ InitializeVmwareSvgaGraphicsMode (
>> 
>>    VmwareSvgaWrite (Private, VmwareSvgaRegEnable, 1);
>> 
>> -  SetDefaultPalette (Private);
>> -  ClearScreen (Private);
>> +  InitializeBochsGraphicsMode (Private, ModeData);
>>  }
>> 
>>  EFI_STATUS
>> 
> 
> (3) Calling InitializeBochsGraphicsMode() from within
> InitializeVmwareSvgaGraphicsMode() seems wrong, considering the current
> structure of the driver.
> 
> We only have InitializeXxxGraphicsMode() calls in
> QemuVideoGraphicsOutputSetMode(). In order to determine which variant 
> to
> call, we check the "Private->Variant" field, in a "switch" statement.
> Therefore:
> 
> (3a) If a general fallback from "VmwareSvga" to "Bochs" is necessary,
> then the fallback logic should be added to earlier code that sets the
> Variant field.
> 
> You can see an example for that in the QemuVideoControllerDriverStart()
> function, near the debug message "QemuVideo: No mmio bar, fallback to
> port io". There the Variant field is degraded from the originally
> detected QEMU_VIDEO_BOCHS_MMIO value, to QEMU_VIDEO_BOCHS.
> 
> (3b) Or else, if calling InitializeVmwareSvgaGraphicsMode() is fine as 
> a
> basis, but we also need the actions of InitializeBochsGraphicsMode() 
> *in
> addition*, then please:
> 
> - extract the common actions from InitializeBochsGraphicsMode() to a 
> new
> helper function, and call the helper from both
> InitializeBochsGraphicsMode() and InitializeVmwareSvgaGraphicsMode(),
> 
> - explain, in InitializeVmwareSvgaGraphicsMode(), *why* the Bochs 
> config
> actions are necessary, in addition to the VmwareSvga actions.
> 
> Thanks,
> Laszlo

Thanks,
yuchenlin


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23 11:12   ` yuchenlin
@ 2018-10-23 12:45     ` Laszlo Ersek
  2018-10-23 13:42       ` Gerd Hoffmann
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2018-10-23 12:45 UTC (permalink / raw)
  To: yuchenlin
  Cc: edk2-devel, jordan.l.justen, ard.biesheuvel, anthony.perard,
	julien.grall, Gerd Hoffmann, Phil Dennis-Jordan

On 10/23/18 13:12, yuchenlin wrote:
> Hi, Laszlo
>
> On 2018-10-23 18:18, Laszlo Ersek wrote:
>> (1) Adding Gerd (because he maintains video in QEMU), and Phil
>> Dennis-Jordan (for authoring commit c137d9508169,
>> "OvmfPkg/QemuVideoDxe: VMWare SVGA device support", 2017-04-07).
>>
>>
>> On 10/23/18 04:40, yuchenlin@synology.com wrote:
>>> From: yuchenlin <yuchenlin@synology.com>
>>>
>>> When driver doesn't set fifo config, the vmsvga will fall back
>>> to std vga. However, we don't initialize vbe related port. It
>>> causes blank screen in qemu console.
>>
>> (2) The words "when driver doesn't set fifo config" tell me nothing.
>> The QemuVideoDxe directory has zero instances of the word "fifo". The
>> same applies to "OvmfPkg/Include/IndustryStandard/VmwareSvga.h".
>>
>> In addition, the "vmsvga will fall back to std vga" statement is also
>> unclear. Is that a statement about the QEMU device model's behavior?
>>
>
> About FIFO, you can refer to
> https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L34,
>
>
> "The main advantage of VMware's SVGA device over alternatives like VBE
> is that the virtual machine can explicitly indicate which ares of the
> screen have changed by sending update rectangles through the device's
> command FIFO."
>
> According to
> https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L533,
>
> to use vmsvga's FIFO. Driver should setup FIFO and set
> SVGA_REG_CONFIG_DONE to 1. However, OVMF doesn't do it. In this case,
> vmsvga will fall back to vga. It is QEMU device model's behavior after
> commit 104bd1dc70 (vmsvga: fix vmsvga_update_display).
>
>> I vaguely suspect that your intent might be to say, "QemuVideoDxe
>> does not perform a necessary configuration step, and therefore it
>> cannot drive QEMU's VMW SVGA device". However, if that is indeed your
>> intent, then I believe something must have changed recently in QEMU,
>> because QemuVideoDxe *definitely* worked when Phil contributed the
>> VMW SVGA driver logic, in commit c137d9508169.
>>
>> Are we talking about a QEMU regression, or a driver-side
>> configuration step that QemuVideoDxe has always missed (and we're
>> being punished for it only now)?
>>
>
> This is QEMU behavior change in commit 104bd1dc70 (vmsvga: fix
> vmsvga_update_display). In my opinion, it is a correct change.
>
>>
>>> This patch will fix "Guest has not initialized the display (yet)"
>>> when using qemu -device vmware-svga with ovmf.
>>
>> Right, as I write above, this definitely worked earlier. I suggest
>> bisecting QEMU (and/or testing older QEMU machine types) to identify
>> the QEMU side change. Once we know that, we can decide whether this
>> is a QEMU regression, or just exposing a long-standing OVMF bug.
>
> In my opinion, it is a long-standing OVMF bug, which is based on the
> wrong behavior of QEMU's vmsvga. It relied on dirty memory region to
> call dpy_gfx_update for updating display.

Thank you for the explanation!

Please help me see the situation better. Here's my current
understanding.

(1) QemuVideoDxe doesn't set up the VMW SVGA FIFO, and does not store 1
    to the SVGA_REG_CONFIG_DONE register. And this is not a "small
    missing step" -- the FIFO setup can be considered a separate
    feature.

(2) We don't intend to implement the FIFO setup feature. (In particular
    because we don't intend to track changed rectangles and send updates
    via the FIFO.)

(3) The intent of the original VMW SVGA enablement patch for
    QemuVideoDxe, namely commit c137d9508169, was to enable booting some
    UEFI operating systems on OVMF that had guest drivers only for VMW
    SVGA.

(4) The QEMU device model now -- since commit 104bd1dc70 -- falls back
    to stdvga (that is, Bochs) in response to QemuVideoDxe's actions.

(5) Your proposal is to set up the Bochs interface in QemuVideoDxe, *in
    addition* to the -- now dysfunctional! -- VMW SVGA interface.

Is my understanding correct?

Because, here's what I think: the VMW SVGA guest driver logic in
QemuVideoDxe is basically dead code now, since QEMU commit 104bd1dc70.
As I understand it, we don't rely on it *at all*. It does nothing for
us.

(And that has been the case since QEMU v2.10.0, released on 2017-Aug-30.
It's quite telling that the first issue report comes in now, after more
than one year...)

So, what do you think of the following approach, instead of your
currently proposed patch:

- revert commit c137d9508169 ("OvmfPkg/QemuVideoDxe: VMWare SVGA device
  support", 2017-04-07)

- revert commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions
  for unaligned port I/O.", 2017-04-07)

- given that QEMU provides the Bochs interface anyway, with the VMW SVGA
  device model, simply recognize the "QEMU VMWare SVGA" card, in the
  "gQemuVideoCardList" array, as the QEMU_VIDEO_BOCHS_MMIO variant.

Would that work? Because, if our current VMW SVGA driver code is
basically dead, I prefer a solution that simplifies QemuVideoDxe,
instead of complicating it further. And, importantly, guest OS drivers
could *still* set up the real VMW SVGA FIFO thing, after they boot, on
the same device.

The remaining question is how this approach would work with QEMU
versions and/or machine types that precede v2.10.0. To your knowledge:

Before QEMU commit 104bd1dc70, was it really *required* to use the (now
dead) QemuVideoDxe code for VMW SVGA, or would it always have been
possible to simply use the Bochs interface, to drive the VMW SVGA
device?

(I don't think we have ever asked this question. Checking the original
RFC thread:

  [edk2] [RFC PATCH v1 0/2]
  OvmfPkg/QemuVideoDxe: Add VMWare SVGA2 framebuffer support

  https://lists.01.org/pipermail/edk2-devel/2017-March/009201.html

I don't see a question whether using the Bochs VBE interface would
simply work. I think we all assumed that the VMW SVGA-specific interface
was a hard requirement, for enabling QemuVideoDxe on QEMU's VMW SVGA
device model.)


Either way, I would certainly like to hear Gerd's opinion on this.

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23 12:45     ` Laszlo Ersek
@ 2018-10-23 13:42       ` Gerd Hoffmann
  2018-10-23 14:02         ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2018-10-23 13:42 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: yuchenlin, edk2-devel, jordan.l.justen, ard.biesheuvel,
	anthony.perard, julien.grall, Phil Dennis-Jordan

  Hi,

> Please help me see the situation better. Here's my current
> understanding.
> 
> (1) QemuVideoDxe doesn't set up the VMW SVGA FIFO, and does not store 1
>     to the SVGA_REG_CONFIG_DONE register. And this is not a "small
>     missing step" -- the FIFO setup can be considered a separate
>     feature.
> 
> (2) We don't intend to implement the FIFO setup feature. (In particular
>     because we don't intend to track changed rectangles and send updates
>     via the FIFO.)
> 
> (3) The intent of the original VMW SVGA enablement patch for
>     QemuVideoDxe, namely commit c137d9508169, was to enable booting some
>     UEFI operating systems on OVMF that had guest drivers only for VMW
>     SVGA.
> 
> (4) The QEMU device model now -- since commit 104bd1dc70 -- falls back
>     to stdvga (that is, Bochs) in response to QemuVideoDxe's actions.
> 
> (5) Your proposal is to set up the Bochs interface in QemuVideoDxe, *in
>     addition* to the -- now dysfunctional! -- VMW SVGA interface.
> 
> Is my understanding correct?

I understand things the same way.

> So, what do you think of the following approach, instead of your
> currently proposed patch:
> 
> - revert commit c137d9508169 ("OvmfPkg/QemuVideoDxe: VMWare SVGA device
>   support", 2017-04-07)
> 
> - revert commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions
>   for unaligned port I/O.", 2017-04-07)
> 
> - given that QEMU provides the Bochs interface anyway, with the VMW SVGA
>   device model, simply recognize the "QEMU VMWare SVGA" card, in the
>   "gQemuVideoCardList" array, as the QEMU_VIDEO_BOCHS_MMIO variant.

Makes sense to me.

Alternatively write a full-blown vmsvga driver which works simliar to
the virtio-gpu driver.  I have my doubts this is worth the effort
though.

> Before QEMU commit 104bd1dc70, was it really *required* to use the (now
> dead) QemuVideoDxe code for VMW SVGA, or would it always have been
> possible to simply use the Bochs interface, to drive the VMW SVGA
> device?

I think bochs interface works works with all vmsvga version (in qemu).
Didn't test though.

cheers,
  Gerd



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23 13:42       ` Gerd Hoffmann
@ 2018-10-23 14:02         ` Laszlo Ersek
  2018-10-24  1:33           ` yuchenlin
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2018-10-23 14:02 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: yuchenlin, edk2-devel, jordan.l.justen, ard.biesheuvel,
	anthony.perard, julien.grall, Phil Dennis-Jordan

On 10/23/18 15:42, Gerd Hoffmann wrote:
>   Hi,
> 
>> Please help me see the situation better. Here's my current
>> understanding.
>>
>> (1) QemuVideoDxe doesn't set up the VMW SVGA FIFO, and does not store 1
>>     to the SVGA_REG_CONFIG_DONE register. And this is not a "small
>>     missing step" -- the FIFO setup can be considered a separate
>>     feature.
>>
>> (2) We don't intend to implement the FIFO setup feature. (In particular
>>     because we don't intend to track changed rectangles and send updates
>>     via the FIFO.)
>>
>> (3) The intent of the original VMW SVGA enablement patch for
>>     QemuVideoDxe, namely commit c137d9508169, was to enable booting some
>>     UEFI operating systems on OVMF that had guest drivers only for VMW
>>     SVGA.
>>
>> (4) The QEMU device model now -- since commit 104bd1dc70 -- falls back
>>     to stdvga (that is, Bochs) in response to QemuVideoDxe's actions.
>>
>> (5) Your proposal is to set up the Bochs interface in QemuVideoDxe, *in
>>     addition* to the -- now dysfunctional! -- VMW SVGA interface.
>>
>> Is my understanding correct?
> 
> I understand things the same way.
> 
>> So, what do you think of the following approach, instead of your
>> currently proposed patch:
>>
>> - revert commit c137d9508169 ("OvmfPkg/QemuVideoDxe: VMWare SVGA device
>>   support", 2017-04-07)
>>
>> - revert commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions
>>   for unaligned port I/O.", 2017-04-07)
>>
>> - given that QEMU provides the Bochs interface anyway, with the VMW SVGA
>>   device model, simply recognize the "QEMU VMWare SVGA" card, in the
>>   "gQemuVideoCardList" array, as the QEMU_VIDEO_BOCHS_MMIO variant.
> 
> Makes sense to me.

Great, thank you.

> Alternatively write a full-blown vmsvga driver which works simliar to
> the virtio-gpu driver.  I have my doubts this is worth the effort
> though.

Right, please let us *not* do this.

> 
>> Before QEMU commit 104bd1dc70, was it really *required* to use the (now
>> dead) QemuVideoDxe code for VMW SVGA, or would it always have been
>> possible to simply use the Bochs interface, to drive the VMW SVGA
>> device?
> 
> I think bochs interface works works with all vmsvga version (in qemu).

Great!

> Didn't test though.

That's fine; I think I haven't tested VMW SVGA, as in "ever". That's up
to people that actually use the device model. As I wrote previously,
it's quite telling that the consequences of QEMU commit 104bd1dc70, from
release v2.10.0, are reported only now, more than a year after the
release -- VMW SVGA must not be a very popular device model.

Yuchenlin, can you then please investigate this approach, including
testing the reverts (the Bochs-only implementation) against QEMU v2.9.1?

(FWIW, upstream QEMU doesn't seem to support earlier releases than
v2.10.2 any longer, according to <https://www.qemu.org/>. So even if the
v2.9.1 test failed, maybe we shouldn't care, in *upstream* edk2. Still,
knowing the status would be useful.)

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga
  2018-10-23 14:02         ` Laszlo Ersek
@ 2018-10-24  1:33           ` yuchenlin
  0 siblings, 0 replies; 7+ messages in thread
From: yuchenlin @ 2018-10-24  1:33 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Gerd Hoffmann, edk2-devel, jordan.l.justen, ard.biesheuvel,
	anthony.perard, julien.grall, Phil Dennis-Jordan

On 2018-10-23 22:02, Laszlo Ersek wrote:
> On 10/23/18 15:42, Gerd Hoffmann wrote:
>>   Hi,
>> 
>>> Please help me see the situation better. Here's my current
>>> understanding.
>>> 
>>> (1) QemuVideoDxe doesn't set up the VMW SVGA FIFO, and does not store 
>>> 1
>>>     to the SVGA_REG_CONFIG_DONE register. And this is not a "small
>>>     missing step" -- the FIFO setup can be considered a separate
>>>     feature.
>>> 
>>> (2) We don't intend to implement the FIFO setup feature. (In 
>>> particular
>>>     because we don't intend to track changed rectangles and send 
>>> updates
>>>     via the FIFO.)
>>> 
>>> (3) The intent of the original VMW SVGA enablement patch for
>>>     QemuVideoDxe, namely commit c137d9508169, was to enable booting 
>>> some
>>>     UEFI operating systems on OVMF that had guest drivers only for 
>>> VMW
>>>     SVGA.
>>> 
>>> (4) The QEMU device model now -- since commit 104bd1dc70 -- falls 
>>> back
>>>     to stdvga (that is, Bochs) in response to QemuVideoDxe's actions.
>>> 
>>> (5) Your proposal is to set up the Bochs interface in QemuVideoDxe, 
>>> *in
>>>     addition* to the -- now dysfunctional! -- VMW SVGA interface.
>>> 
>>> Is my understanding correct?
>> 
>> I understand things the same way.
>> 
>>> So, what do you think of the following approach, instead of your
>>> currently proposed patch:
>>> 
>>> - revert commit c137d9508169 ("OvmfPkg/QemuVideoDxe: VMWare SVGA 
>>> device
>>>   support", 2017-04-07)
>>> 
>>> - revert commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions
>>>   for unaligned port I/O.", 2017-04-07)
>>> 
>>> - given that QEMU provides the Bochs interface anyway, with the VMW 
>>> SVGA
>>>   device model, simply recognize the "QEMU VMWare SVGA" card, in the
>>>   "gQemuVideoCardList" array, as the QEMU_VIDEO_BOCHS_MMIO variant.
>> 
>> Makes sense to me.
> 
> Great, thank you.
> 
>> Alternatively write a full-blown vmsvga driver which works simliar to
>> the virtio-gpu driver.  I have my doubts this is worth the effort
>> though.
> 
> Right, please let us *not* do this.
> 
>> 
>>> Before QEMU commit 104bd1dc70, was it really *required* to use the 
>>> (now
>>> dead) QemuVideoDxe code for VMW SVGA, or would it always have been
>>> possible to simply use the Bochs interface, to drive the VMW SVGA
>>> device?
>> 
>> I think bochs interface works works with all vmsvga version (in qemu).
> 
> Great!
> 
>> Didn't test though.
> 
> That's fine; I think I haven't tested VMW SVGA, as in "ever". That's up
> to people that actually use the device model. As I wrote previously,
> it's quite telling that the consequences of QEMU commit 104bd1dc70, 
> from
> release v2.10.0, are reported only now, more than a year after the
> release -- VMW SVGA must not be a very popular device model.
> 
> Yuchenlin, can you then please investigate this approach, including
> testing the reverts (the Bochs-only implementation) against QEMU 
> v2.9.1?
> 

Sure!

Thanks,
yuchenlin

> (FWIW, upstream QEMU doesn't seem to support earlier releases than
> v2.10.2 any longer, according to <https://www.qemu.org/>. So even if 
> the
> v2.9.1 test failed, maybe we shouldn't care, in *upstream* edk2. Still,
> knowing the status would be useful.)
> 
> Thanks!
> Laszlo



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-10-24  1:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-23  2:40 [PATCH] OvmfPkg: initialize bochs when initializing vmsvga yuchenlin
2018-10-23 10:18 ` Laszlo Ersek
2018-10-23 11:12   ` yuchenlin
2018-10-23 12:45     ` Laszlo Ersek
2018-10-23 13:42       ` Gerd Hoffmann
2018-10-23 14:02         ` Laszlo Ersek
2018-10-24  1:33           ` yuchenlin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox