[Qemu-devel] [PATCH v3 0/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Qemu-devel] [PATCH v3 0/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Marcel Apfelbaum]

Hot-plugging a device that has a romfile (either supplied by user
or built-in) using rombar=0 option is an user error,
do not allow the device to be hot-plugged.

v2 -> v3:
 - Reverse the decision to forbit the hotplug on user error (Michael S. Tsirkin)
 - Split the patch in two, first part being a separate fix (Markus Armbruster)  

v1 -> v2:
 After a discussion with Michael, Paolo and Alex, this
 patch silent drops the romfile instead of not allowing
 the hotplug.

Marcel Apfelbaum (2):
  hw/pci: fixed error flow in pci_qdev_init
  hw/pci: fixed hotplug crash when using rombar=0 with devices having
    romfile

 hw/pci/pci.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 1/2] hw/pci: fixed error flow in pci_qdev_init

[Marcel Apfelbaum]

Verify return code for pci_add_option_rom.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6ce75aa..36226eb 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1776,7 +1776,12 @@ static int pci_qdev_init(DeviceState *qdev)
         pci_dev->romfile = g_strdup(pc->romfile);
         is_default_rom = true;
     }
-    pci_add_option_rom(pci_dev, is_default_rom);
+
+    rc = pci_add_option_rom(pci_dev, is_default_rom);
+    if (rc != 0) {
+        pci_unregister_device(DEVICE(pci_dev));
+        return rc;
+    }
 
     return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 2/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Marcel Apfelbaum]

Hot-plugging a device that has a romfile (either supplied by user
or built-in) using rombar=0 option is an user error,
do not allow the device to be hot-plugged.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/pci/pci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 36226eb..371699c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1942,6 +1942,15 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
          * for 0.11 compatibility.
          */
         int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
+
+        /*
+         * Hot-plugged devices can't use the option ROM
+         * if the rom bar is disabled.
+         */
+        if (DEVICE(pdev)->hotplugged) {
+            return -1;
+        }
+
         if (class == 0x0300) {
             rom_add_vga(pdev->romfile);
         } else {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 2/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On 10/27/2014 10:41 AM, Marcel Apfelbaum wrote:
> Hot-plugging a device that has a romfile (either supplied by user
> or built-in) using rombar=0 option is an user error,

s/an/a/ (user is pronounced with a leading hard "y" sound rather than a
soft "u", it may help to remember you can "give a unicorn an umbrella")

> do not allow the device to be hot-plugged.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  hw/pci/pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v3 2/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Marcel Apfelbaum]

On Mon, 2014-10-27 at 11:10 -0600, Eric Blake wrote:
> On 10/27/2014 10:41 AM, Marcel Apfelbaum wrote:
> > Hot-plugging a device that has a romfile (either supplied by user
> > or built-in) using rombar=0 option is an user error,
> 
> s/an/a/ (user is pronounced with a leading hard "y" sound rather than a
> soft "u", it may help to remember you can "give a unicorn an umbrella")
Thanks, still hard to get it right, but your tip will surely help :)

Michael, do you want me to send another version,
or you can change it on the fly?

Thanks,
Marcel

> 
> > do not allow the device to be hot-plugged.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  hw/pci/pci.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Re: [Qemu-devel] [PATCH v3 2/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile

[Michael S. Tsirkin]

On Mon, Oct 27, 2014 at 07:15:38PM +0200, Marcel Apfelbaum wrote:
> On Mon, 2014-10-27 at 11:10 -0600, Eric Blake wrote:
> > On 10/27/2014 10:41 AM, Marcel Apfelbaum wrote:
> > > Hot-plugging a device that has a romfile (either supplied by user
> > > or built-in) using rombar=0 option is an user error,
> > 
> > s/an/a/ (user is pronounced with a leading hard "y" sound rather than a
> > soft "u", it may help to remember you can "give a unicorn an umbrella")
> Thanks, still hard to get it right, but your tip will surely help :)
> 
> Michael, do you want me to send another version,
> or you can change it on the fly?
> 
> Thanks,
> Marcel

go ahead and repost, easier for me.

> > 
> > > do not allow the device to be hot-plugged.
> > > 
> > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > > ---
> > >  hw/pci/pci.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > 
> 
>