[Qemu-devel] [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Qemu-devel] [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
at init time, it is not possible to load an option ROM for a hotplug
device when running in compat mode.

v2: Alex Williamson pointed out that one can get to qdev directly from
pci_dev, so no need to pass it down.

v3: Braces

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hw/pci.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..a20f796 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1810,11 +1810,16 @@ static int pci_add_option_rom(PCIDevice *pdev)
         return 0;
 
     if (!pdev->rom_bar) {
+        int class;
         /*
          * Load rom via fw_cfg instead of creating a rom bar,
-         * for 0.11 compatibility.
+         * for 0.11 compatibility. fw_cfg is initialized at boot, so
+         * we cannot do hotplug load of option roms.
          */
-        int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
+        if (pdev->qdev.hotplugged) {
+            return 0;
+        }
+        class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
         if (class == 0x0300) {
             rom_add_vga(pdev->romfile);
         } else {
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Anthony Liguori]

On 08/30/2010 03:16 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
> at init time, it is not possible to load an option ROM for a hotplug
> device when running in compat mode.
>
> v2: Alex Williamson pointed out that one can get to qdev directly from
> pci_dev, so no need to pass it down.
>
> v3: Braces
>    

What's the specific bug?  The devices themselves have a check for 
hotplug which inhibits rom addition during hotplug so either there's a 
device missing this check or if we're going to go this route, we ought 
to remove those checks in the other devices.

Regards,

Anthony Liguori

> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   hw/pci.c |    9 +++++++--
>   1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index a98d6f3..a20f796 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1810,11 +1810,16 @@ static int pci_add_option_rom(PCIDevice *pdev)
>           return 0;
>
>       if (!pdev->rom_bar) {
> +        int class;
>           /*
>            * Load rom via fw_cfg instead of creating a rom bar,
> -         * for 0.11 compatibility.
> +         * for 0.11 compatibility. fw_cfg is initialized at boot, so
> +         * we cannot do hotplug load of option roms.
>            */
> -        int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> +        if (pdev->qdev.hotplugged) {
> +            return 0;
> +        }
> +        class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
>           if (class == 0x0300) {
>               rom_add_vga(pdev->romfile);
>           } else {
>    

[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Jes Sorensen]

On 08/30/10 15:00, Anthony Liguori wrote:
> On 08/30/2010 03:16 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
>> at init time, it is not possible to load an option ROM for a hotplug
>> device when running in compat mode.
>>
>> v2: Alex Williamson pointed out that one can get to qdev directly from
>> pci_dev, so no need to pass it down.
>>
>> v3: Braces
>>    
> 
> What's the specific bug?  The devices themselves have a check for
> hotplug which inhibits rom addition during hotplug so either there's a
> device missing this check or if we're going to go this route, we ought
> to remove those checks in the other devices.

If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg,
which means QEMU is unable to load it after boot time if you try to
hot-plug a new network device via the monitor. Instead it decides to
exit with an error.

My patch makes QEMU not try to load the option ROM in this case, which
IMHO is a reasonable workaround. It means you can't PXE from the
hot-plugged device, but at least QEMU doesn't exit out on you.

Cheers,
Jes

Re: [Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 08/30/2010 03:16 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
>> at init time, it is not possible to load an option ROM for a hotplug
>> device when running in compat mode.
>>
>> v2: Alex Williamson pointed out that one can get to qdev directly from
>> pci_dev, so no need to pass it down.
>>
>> v3: Braces
>>    
>
> What's the specific bug?  The devices themselves have a check for
> hotplug which inhibits rom addition during hotplug so either there's a
> device missing this check or if we're going to go this route, we ought
> to remove those checks in the other devices.

Quoting my reply to v1[*]:

    Example:

        $ qemu -M pc-0.11 -S -monitor stdio
        QEMU 0.12.50 monitor - type 'help' for more information
        (qemu) device_add e1000
        qemu: hardware error: ROM images must be loaded at startup
    [...]
        Aborted (core dumped)

    The fix silently omits the option ROM when we can't load it.  Works for
    me.

[*] http://lists.nongnu.org/archive/html/qemu-devel/2010-07/msg01397.html

[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Anthony Liguori]

On 08/30/2010 08:11 AM, Jes Sorensen wrote:
> On 08/30/10 15:00, Anthony Liguori wrote:
>    
>> On 08/30/2010 03:16 AM, Jes.Sorensen@redhat.com wrote:
>>      
>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>
>>> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
>>> at init time, it is not possible to load an option ROM for a hotplug
>>> device when running in compat mode.
>>>
>>> v2: Alex Williamson pointed out that one can get to qdev directly from
>>> pci_dev, so no need to pass it down.
>>>
>>> v3: Braces
>>>
>>>        
>> What's the specific bug?  The devices themselves have a check for
>> hotplug which inhibits rom addition during hotplug so either there's a
>> device missing this check or if we're going to go this route, we ought
>> to remove those checks in the other devices.
>>      
> If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg,
> which means QEMU is unable to load it after boot time if you try to
> hot-plug a new network device via the monitor. Instead it decides to
> exit with an error.
>    

Which network device?

Take a look at ne2k.c's rom loading.  It's got logic for rom loading 
with hotplug but e1000 and rtl8139 don't.  Maybe it's because ne2k also 
supports an ISA mode?

Gerd, what was your intention here?

Regards,

Anthony Liguori

> My patch makes QEMU not try to load the option ROM in this case, which
> IMHO is a reasonable workaround. It means you can't PXE from the
> hot-plugged device, but at least QEMU doesn't exit out on you.
>
> Cheers,
> Jes
>    

[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Gerd Hoffmann]

On 08/30/10 15:45, Anthony Liguori wrote:
> On 08/30/2010 08:11 AM, Jes Sorensen wrote:
>> On 08/30/10 15:00, Anthony Liguori wrote:
>>> On 08/30/2010 03:16 AM, Jes.Sorensen@redhat.com wrote:
>>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>>
>>>> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is
>>>> setup
>>>> at init time, it is not possible to load an option ROM for a hotplug
>>>> device when running in compat mode.
>>>>
>>>> v2: Alex Williamson pointed out that one can get to qdev directly from
>>>> pci_dev, so no need to pass it down.
>>>>
>>>> v3: Braces
>>>>
>>> What's the specific bug? The devices themselves have a check for
>>> hotplug which inhibits rom addition during hotplug so either there's a
>>> device missing this check or if we're going to go this route, we ought
>>> to remove those checks in the other devices.
>> If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg,
>> which means QEMU is unable to load it after boot time if you try to
>> hot-plug a new network device via the monitor. Instead it decides to
>> exit with an error.
>
> Which network device?
>
> Take a look at ne2k.c's rom loading. It's got logic for rom loading with
> hotplug but e1000 and rtl8139 don't. Maybe it's because ne2k also
> supports an ISA mode?

I think I just forgot to convert ne2k over to using .romfile instead.

Just skipping fw_cfg-based rom loading looks sane to me.  After all it 
is just for pc-0.11 compatibility.  And it is even bug compatible: 
hot-plug nic + reboot + pxe-boot from the hot-plugged nic didn't work in 
0.11 too ;)

cheers,
   Gerd

[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode

[Jes Sorensen]

On 08/30/10 15:45, Anthony Liguori wrote:
> On 08/30/2010 08:11 AM, Jes Sorensen wrote:
>> If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg,
>> which means QEMU is unable to load it after boot time if you try to
>> hot-plug a new network device via the monitor. Instead it decides to
>> exit with an error.
>>    
> 
> Which network device?
> 
> Take a look at ne2k.c's rom loading.  It's got logic for rom loading
> with hotplug but e1000 and rtl8139 don't.  Maybe it's because ne2k also
> supports an ISA mode?

It happens with virtio-net as well which is where I saw it first.
However, it's really only an issue in compat mode since with 0.12
onwards we can load option ROMs on the fly.

Cheers,
Jes