* [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
@ 2010-07-23 12:17 Jes.Sorensen
2010-07-23 15:37 ` Markus Armbruster
0 siblings, 1 reply; 10+ messages in thread
From: Jes.Sorensen @ 2010-07-23 12:17 UTC (permalink / raw)
To: qemu-devel
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.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
hw/pci.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..12bd4aa 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -77,7 +77,7 @@ static struct BusInfo pci_bus_info = {
static void pci_update_mappings(PCIDevice *d);
static void pci_set_irq(void *opaque, int irq_num, int level);
-static int pci_add_option_rom(PCIDevice *pdev);
+static int pci_add_option_rom(PCIDevice *pdev, int hotplugged);
static void pci_del_option_rom(PCIDevice *pdev);
static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
@@ -1693,7 +1693,7 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
/* rom loading */
if (pci_dev->romfile == NULL && info->romfile != NULL)
pci_dev->romfile = qemu_strdup(info->romfile);
- pci_add_option_rom(pci_dev);
+ pci_add_option_rom(pci_dev, qdev->hotplugged);
if (qdev->hotplugged) {
rc = bus->hotplug(bus->hotplug_qdev, pci_dev, 1);
@@ -1797,7 +1797,7 @@ static void pci_map_option_rom(PCIDevice *pdev, int region_num, pcibus_t addr, p
}
/* Add an option rom for the device */
-static int pci_add_option_rom(PCIDevice *pdev)
+static int pci_add_option_rom(PCIDevice *pdev, int hotplugged)
{
int size;
char *path;
@@ -1810,11 +1810,15 @@ 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 (hotplugged)
+ return 0;
+ class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
if (class == 0x0300) {
rom_add_vga(pdev->romfile);
} else {
--
1.7.1.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-23 12:17 [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode Jes.Sorensen
@ 2010-07-23 15:37 ` Markus Armbruster
0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2010-07-23 15:37 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: qemu-devel
Jes.Sorensen@redhat.com writes:
> 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.
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-31 15:21 ` Jes Sorensen
@ 2010-07-31 15:29 ` Aurelien Jarno
0 siblings, 0 replies; 10+ messages in thread
From: Aurelien Jarno @ 2010-07-31 15:29 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Alex.Williamson, qemu-devel
On Sat, Jul 31, 2010 at 05:21:34PM +0200, Jes Sorensen wrote:
> On 07/31/10 17:13, Aurelien Jarno wrote:
> > On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote:
> >> If you want that, please do it in a separate patch for the entire file,
> >> otherwise it will never become consistent. However it doesn't change the
> >> issue either that putting braces around a single line like this is bad
> >> coding style.
> >
> > We got this discussions numerous times, and I am not going to start it
> > again. If you want to see the patch applied, just follow the rules.
> >
>
> Then I would like to know why you do not apply this silly rule to every
> patch submitted. There's patches going in almost every day that doesn't
> use this hopeless formatting.
>
Because not everyone try to enforce the rules.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-31 15:13 ` Aurelien Jarno
@ 2010-07-31 15:21 ` Jes Sorensen
2010-07-31 15:29 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Jes Sorensen @ 2010-07-31 15:21 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Alex.Williamson, qemu-devel
On 07/31/10 17:13, Aurelien Jarno wrote:
> On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote:
>> If you want that, please do it in a separate patch for the entire file,
>> otherwise it will never become consistent. However it doesn't change the
>> issue either that putting braces around a single line like this is bad
>> coding style.
>
> We got this discussions numerous times, and I am not going to start it
> again. If you want to see the patch applied, just follow the rules.
>
Then I would like to know why you do not apply this silly rule to every
patch submitted. There's patches going in almost every day that doesn't
use this hopeless formatting.
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-31 14:40 ` Jes Sorensen
@ 2010-07-31 15:13 ` Aurelien Jarno
2010-07-31 15:21 ` Jes Sorensen
0 siblings, 1 reply; 10+ messages in thread
From: Aurelien Jarno @ 2010-07-31 15:13 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Alex.Williamson, qemu-devel
On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote:
> On 07/31/10 16:19, Aurelien Jarno wrote:
> > On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote:
> >> On 07/30/10 23:08, Aurelien Jarno wrote:
> >>> Missing braces around the return 0 line.
> >>
> >> Half the QEMU code base doesn't have braces around single line if
> >> statements, including in hw/pci.c. Adding braces here would be inconsistent.
> >>
> >
> > If we follow the coding style in new patches, it will eventually become
> > consistent.
> >
>
> If you want that, please do it in a separate patch for the entire file,
> otherwise it will never become consistent. However it doesn't change the
> issue either that putting braces around a single line like this is bad
> coding style.
We got this discussions numerous times, and I am not going to start it
again. If you want to see the patch applied, just follow the rules.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-31 14:19 ` Aurelien Jarno
@ 2010-07-31 14:40 ` Jes Sorensen
2010-07-31 15:13 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Jes Sorensen @ 2010-07-31 14:40 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Alex.Williamson, qemu-devel
On 07/31/10 16:19, Aurelien Jarno wrote:
> On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote:
>> On 07/30/10 23:08, Aurelien Jarno wrote:
>>> Missing braces around the return 0 line.
>>
>> Half the QEMU code base doesn't have braces around single line if
>> statements, including in hw/pci.c. Adding braces here would be inconsistent.
>>
>
> If we follow the coding style in new patches, it will eventually become
> consistent.
>
If you want that, please do it in a separate patch for the entire file,
otherwise it will never become consistent. However it doesn't change the
issue either that putting braces around a single line like this is bad
coding style.
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-31 9:16 ` Jes Sorensen
@ 2010-07-31 14:19 ` Aurelien Jarno
2010-07-31 14:40 ` Jes Sorensen
0 siblings, 1 reply; 10+ messages in thread
From: Aurelien Jarno @ 2010-07-31 14:19 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Alex.Williamson, qemu-devel
On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote:
> On 07/30/10 23:08, Aurelien Jarno wrote:
> > On Fri, Jul 23, 2010 at 05:56:38PM +0200, 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.
> >>
> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> ---
> >> hw/pci.c | 8 ++++++--
> >> 1 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/pci.c b/hw/pci.c
> >> index a98d6f3..2d38643 100644
> >> --- a/hw/pci.c
> >> +++ b/hw/pci.c
> >> @@ -1810,11 +1810,15 @@ 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;
> >
> > Missing braces around the return 0 line.
>
> Half the QEMU code base doesn't have braces around single line if
> statements, including in hw/pci.c. Adding braces here would be inconsistent.
>
If we follow the coding style in new patches, it will eventually become
consistent.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-30 21:08 ` Aurelien Jarno
@ 2010-07-31 9:16 ` Jes Sorensen
2010-07-31 14:19 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Jes Sorensen @ 2010-07-31 9:16 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Alex.Williamson, qemu-devel
On 07/30/10 23:08, Aurelien Jarno wrote:
> On Fri, Jul 23, 2010 at 05:56:38PM +0200, 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.
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> ---
>> hw/pci.c | 8 ++++++--
>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci.c b/hw/pci.c
>> index a98d6f3..2d38643 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -1810,11 +1810,15 @@ 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;
>
> Missing braces around the return 0 line.
Half the QEMU code base doesn't have braces around single line if
statements, including in hw/pci.c. Adding braces here would be inconsistent.
Jes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
2010-07-23 15:56 Jes.Sorensen
@ 2010-07-30 21:08 ` Aurelien Jarno
2010-07-31 9:16 ` Jes Sorensen
0 siblings, 1 reply; 10+ messages in thread
From: Aurelien Jarno @ 2010-07-30 21:08 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: Alex.Williamson, qemu-devel
On Fri, Jul 23, 2010 at 05:56:38PM +0200, 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.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> hw/pci.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index a98d6f3..2d38643 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1810,11 +1810,15 @@ 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;
Missing braces around the return 0 line.
> + class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> if (class == 0x0300) {
> rom_add_vga(pdev->romfile);
> } else {
> --
> 1.7.1.1
Otherwise looks good.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
@ 2010-07-23 15:56 Jes.Sorensen
2010-07-30 21:08 ` Aurelien Jarno
0 siblings, 1 reply; 10+ messages in thread
From: Jes.Sorensen @ 2010-07-23 15:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex.Williamson
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.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
hw/pci.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..2d38643 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1810,11 +1810,15 @@ 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.1.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-07-31 15:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-23 12:17 [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode Jes.Sorensen
2010-07-23 15:37 ` Markus Armbruster
2010-07-23 15:56 Jes.Sorensen
2010-07-30 21:08 ` Aurelien Jarno
2010-07-31 9:16 ` Jes Sorensen
2010-07-31 14:19 ` Aurelien Jarno
2010-07-31 14:40 ` Jes Sorensen
2010-07-31 15:13 ` Aurelien Jarno
2010-07-31 15:21 ` Jes Sorensen
2010-07-31 15:29 ` Aurelien Jarno
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.