[Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Gerd Hoffmann]

Try to pci hotplug a vga card, watch qemu die with hw_error().
This patch fixes it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    4 ++++
 hw/vga-pci.c    |    4 ++++
 hw/vmware_vga.c |    4 ++++
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index ba48289..5175725 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3188,6 +3188,10 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      uint8_t *pci_conf = d->dev.config;
      int device_id = CIRRUS_ID_CLGD5446;
 
+     if (dev->qdev.hotplugged) {
+         return -1;
+     }
+
      /* setup VGA */
      vga_common_init(&s->vga, VGA_RAM_SIZE);
      cirrus_init_common(s, device_id, 1);
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index c8d260c..a2de201 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -79,6 +79,10 @@ static int pci_vga_initfn(PCIDevice *dev)
      VGACommonState *s = &d->vga;
      uint8_t *pci_conf = d->dev.config;
 
+     if (dev->qdev.hotplugged) {
+         return -1;
+     }
+
      // vga + console init
      vga_common_init(s, VGA_RAM_SIZE);
      vga_init(s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 4e7a75d..d12d86f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1232,6 +1232,10 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
     struct pci_vmsvga_state_s *s =
         DO_UPCAST(struct pci_vmsvga_state_s, card, dev);
 
+    if (dev->qdev.hotplugged) {
+        return -1;
+    }
+
     pci_config_set_vendor_id(s->card.config, PCI_VENDOR_ID_VMWARE);
     pci_config_set_device_id(s->card.config, SVGA_PCI_DEVICE_ID);
     s->card.config[PCI_COMMAND]	= PCI_COMMAND_IO |
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Stefano Stabellini]

On Wed, 26 May 2010, Gerd Hoffmann wrote:
> Try to pci hotplug a vga card, watch qemu die with hw_error().
> This patch fixes it.
> 

Do you know the reason why we get hw_error()?
Theoretically vga hotplug should be possible at least for secondary
graphic cards (even though I suspect most operating systems wouldn't
cope).

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Gerd Hoffmann]

On 05/26/10 12:59, Stefano Stabellini wrote:
> On Wed, 26 May 2010, Gerd Hoffmann wrote:
>> Try to pci hotplug a vga card, watch qemu die with hw_error().
>> This patch fixes it.
>>
>
> Do you know the reason why we get hw_error()?

Because the card tries to register the legacy vga ports which are 
already taken by the primary card.  I also don't know whenever you can 
pci hot-plug hardware which uses non-pci ressources at all.

> Theoretically vga hotplug should be possible at least for secondary
> graphic cards (even though I suspect most operating systems wouldn't
> cope).

Yes.  Assuming the virtual hardware in question can actually act as 
secondary, i.e. is fully programmable without the legacy vga ports.  The 
standard vga can't.  The cirrus looks doable, at least you can access 
the vga ports using the mmio bar.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Stefano Stabellini]

On Wed, 26 May 2010, Gerd Hoffmann wrote:
> On 05/26/10 12:59, Stefano Stabellini wrote:
> > On Wed, 26 May 2010, Gerd Hoffmann wrote:
> >> Try to pci hotplug a vga card, watch qemu die with hw_error().
> >> This patch fixes it.
> >>
> >
> > Do you know the reason why we get hw_error()?
> 
> Because the card tries to register the legacy vga ports which are 
> already taken by the primary card.  I also don't know whenever you can 
> pci hot-plug hardware which uses non-pci ressources at all.
> 
> > Theoretically vga hotplug should be possible at least for secondary
> > graphic cards (even though I suspect most operating systems wouldn't
> > cope).
> 
> Yes.  Assuming the virtual hardware in question can actually act as 
> secondary, i.e. is fully programmable without the legacy vga ports.  The 
> standard vga can't.  The cirrus looks doable, at least you can access 
> the vga ports using the mmio bar.
 
I see, good point.

I guess the right fix here would be to return -1 in the stdvga case but
continue in the cirrus case and avoid registering the vga ioports when
used as secondary adapter.

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Gerd Hoffmann]

   Hi,

>> Yes.  Assuming the virtual hardware in question can actually act as
>> secondary, i.e. is fully programmable without the legacy vga ports.  The
>> standard vga can't.  The cirrus looks doable, at least you can access
>> the vga ports using the mmio bar.
>
> I see, good point.
>
> I guess the right fix here would be to return -1 in the stdvga case but
> continue in the cirrus case and avoid registering the vga ioports when
> used as secondary adapter.

Except that this most likely is a non-trivial effort as we have to find 
and test sane ways to handle multiple guest displays.

I think having two gfx screens mapped to two qemu consoles, then be able 
to switch between them via Ctrl-Alt-<nr> (like you switch today to text 
consoles) could be doable without too much effort.  Question is how 
useful this would be as you can't see your two screens at the same time.

With qxl+spice the spice client will open a new window for the secondary 
display.  With vnc+sdl you'll see the primary display only.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Stefano Stabellini]

On Wed, 26 May 2010, Gerd Hoffmann wrote:
>    Hi,
> 
> >> Yes.  Assuming the virtual hardware in question can actually act as
> >> secondary, i.e. is fully programmable without the legacy vga ports.  The
> >> standard vga can't.  The cirrus looks doable, at least you can access
> >> the vga ports using the mmio bar.
> >
> > I see, good point.
> >
> > I guess the right fix here would be to return -1 in the stdvga case but
> > continue in the cirrus case and avoid registering the vga ioports when
> > used as secondary adapter.
> 
> Except that this most likely is a non-trivial effort as we have to find 
> and test sane ways to handle multiple guest displays.
> 
> I think having two gfx screens mapped to two qemu consoles, then be able 
> to switch between them via Ctrl-Alt-<nr> (like you switch today to text 
> consoles) could be doable without too much effort.  Question is how 
> useful this would be as you can't see your two screens at the same time.
> 

Actually I was thinking of registering multiple graphic consoles, each
of them could be rendered by a different frontend (sdl/vnc)
independently. We would have multiple DisplayStates for that.

> With qxl+spice the spice client will open a new window for the secondary 
> display.  With vnc+sdl you'll see the primary display only.
 
So you are doing exactly what I wrote above, right?

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Gerd Hoffmann]

   Hi,

>> I think having two gfx screens mapped to two qemu consoles, then be able
>> to switch between them via Ctrl-Alt-<nr>  (like you switch today to text
>> consoles) could be doable without too much effort.  Question is how
>> useful this would be as you can't see your two screens at the same time.
>
> Actually I was thinking of registering multiple graphic consoles, each
> of them could be rendered by a different frontend (sdl/vnc)
> independently. We would have multiple DisplayStates for that.

Possible, but certainly alot of work.  Tons of code in qemu can't handle 
multiple DisplayStates.  Try it and you'll see.

>> With qxl+spice the spice client will open a new window for the secondary
>> display.  With vnc+sdl you'll see the primary display only.
>
> So you are doing exactly what I wrote above, right?

No.  The secondary display has no DisplayState.  That is the reason why 
only the spice client will see it.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Paul Brook]

> Try to pci hotplug a vga card, watch qemu die with hw_error().
> This patch fixes it.

I think this is wrong. There's no reason why VGA adapters shouldn't be 
hotplugged.  You should fix the underlying problems that prevent hotplugging 
(or make them fail gracefully).

Paul

Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.

[Gerd Hoffmann]

On 05/28/10 16:21, Paul Brook wrote:
>> Try to pci hotplug a vga card, watch qemu die with hw_error().
>> This patch fixes it.
>
> I think this is wrong. There's no reason why VGA adapters shouldn't be
> hotplugged.  You should fix the underlying problems that prevent hotplugging

The qemu code base simply isn't prepared for that.  Making vga hotplug 
requires alot of infrastructure work within qemu, see discussion with 
Stefano in this thread.  I'm also not fully sure it is possible to 
hotplug the primary vga due to the legacy vga i/o ports.

> (or make them fail gracefully).

Make hotplug fail gracefully is exactly what the patch does because 
making hotplug work is impossible short-term IMHO.

cheers,
   Gerd