[Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

Option ROM for network interface cards (NICs) can now explicitly disabled
with romfile=disabled parameter. With hotplugable NICs (currently NE2000, PCNET)
romfile=(empty) didn't work. This patch disables Option ROMs for iPXE for alls
supported NICs (hotplugable and non hotplugable).

Examples with 2 NICs with disabled Option ROM (separated on different lines for readability):
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
-net tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
-net tap,ifname=tap1,script=no,downscript=no,vlan=1

Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
---
  hw/ne2000.c    |    2 +-
  hw/pci.c       |   28 +++++++++++++++++++++++++---
  hw/pci.h       |    5 +++++
  hw/pcnet-pci.c |    2 +-
  4 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/hw/ne2000.c b/hw/ne2000.c
index 62e082f..67bf458 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -765,7 +765,7 @@ static int pci_ne2000_init(PCIDevice *pci_dev)

      if (!pci_dev->qdev.hotplugged) {
          static int loaded = 0;
-        if (!loaded) {
+        if (!loaded && pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
              rom_add_option("pxe-ne2k_pci.rom", -1);
              loaded = 1;
          }
diff --git a/hw/pci.c b/hw/pci.c
index c3082bc..fbce1a7 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -126,6 +126,30 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
      bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
  }

+int pci_has_enabled_option_romfile(PCIDevice *pdev)
+{
+    PCI_DPRINTF("pci_has_enabled_option_romfile: device=%s, romfile=%s\n", pdev->name, pdev->romfile);
+    if (pdev->romfile == NULL)
+        return 0;
+    if (strlen(pdev->romfile) == 0)
+        return 0;
+    if (strcmp(pdev->romfile, PCI_DEVICE_DISABLED_OPTION_ROMFILE) == 0)
+        return 0;
+    return 1;
+}
+
+int pci_has_not_explicitly_disabled_option_romfile(PCIDevice *pdev)
+{
+    PCI_DPRINTF("pci_has_not_explicitly_disabled_option_romfile: device=%s, romfile=%s\n", pdev->name, pdev->romfile);
+
+    /* No romfile is present for hotplugged devices, therefore dynamic codes decides */
+    if (pdev->romfile == NULL)
+        return 1;
+    if (strcmp(pdev->romfile, PCI_DEVICE_DISABLED_OPTION_ROMFILE) == 0)
+        return 0;
+    return 1;
+}
+
  int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
  {
      assert(irq_num >= 0);
@@ -1725,9 +1749,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
      void *ptr;
      char name[32];

-    if (!pdev->romfile)
-        return 0;
-    if (strlen(pdev->romfile) == 0)
+    if (!pci_has_enabled_option_romfile(pdev))
          return 0;

      if (!pdev->rom_bar) {
diff --git a/hw/pci.h b/hw/pci.h
index 625e717..5146bba 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -78,6 +78,8 @@

  #define FMT_PCIBUS                      PRIx64

+#define PCI_DEVICE_DISABLED_OPTION_ROMFILE "disabled"
+
  typedef void PCIConfigWriteFunc(PCIDevice *pci_dev,
                                  uint32_t address, uint32_t data, int len);
  typedef uint32_t PCIConfigReadFunc(PCIDevice *pci_dev,
@@ -275,6 +277,9 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,

  void pci_device_deassert_intx(PCIDevice *dev);

+int pci_has_enabled_option_romfile(PCIDevice *pdev);
+int pci_has_not_explicitly_disabled_option_romfile(PCIDevice *pdev);
+
  static inline void
  pci_set_byte(uint8_t *config, uint8_t val)
  {
diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
index 4e164da..e65745f 100644
--- a/hw/pcnet-pci.c
+++ b/hw/pcnet-pci.c
@@ -332,7 +332,7 @@ static int pci_pcnet_init(PCIDevice *pci_dev)

      if (!pci_dev->qdev.hotplugged) {
          static int loaded = 0;
-        if (!loaded) {
+        if (!loaded && pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
              rom_add_option("pxe-pcnet.rom", -1);
              loaded = 1;
          }
-- 
1.7.6.5

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Stefan Hajnoczi]

On Sun, Jan 8, 2012 at 11:55 AM, Gerhard Wiesinger <lists@wiesinger.com> wrote:
> Option ROM for network interface cards (NICs) can now explicitly disabled
> with romfile=disabled parameter. With hotplugable NICs (currently NE2000,
> PCNET)
> romfile=(empty) didn't work. This patch disables Option ROMs for iPXE for
> alls
> supported NICs (hotplugable and non hotplugable).
>
> Examples with 2 NICs with disabled Option ROM (separated on different lines
> for readability):
> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
> -net tap,ifname=tap0,script=no,downscript=no,vlan=0
> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
> -net tap,ifname=tap1,script=no,downscript=no,vlan=1

Did you consider "no" or "none"?  Those are already used by -net
script=no and -vga none.  I'm afraid we don't have much consistency in
the command-line and adding more variants of basically the same
concept should be avoided when possible.

Stefan

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Sun, 8 Jan 2012, Stefan Hajnoczi wrote:

> On Sun, Jan 8, 2012 at 11:55 AM, Gerhard Wiesinger <lists@wiesinger.com> wrote:
>> Option ROM for network interface cards (NICs) can now explicitly disabled
>> with romfile=disabled parameter. With hotplugable NICs (currently NE2000,
>> PCNET)
>> romfile=(empty) didn't work. This patch disables Option ROMs for iPXE for
>> alls
>> supported NICs (hotplugable and non hotplugable).
>>
>> Examples with 2 NICs with disabled Option ROM (separated on different lines
>> for readability):
>> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
>> -net tap,ifname=tap0,script=no,downscript=no,vlan=0
>> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
>> -net tap,ifname=tap1,script=no,downscript=no,vlan=1
>
> Did you consider "no" or "none"?  Those are already used by -net
> script=no and -vga none.  I'm afraid we don't have much consistency in
> the command-line and adding more variants of basically the same
> concept should be avoided when possible.

Whether the option is spelled "disabled", "no", "none" or all of them 
above: I don't care about that, just the group of maintainers should 
decide (I would prefer all of them ...).

The only thing: just help me with git and doing another commit and 
preparing V2 patch then ...

# I did:
git stash save mypatch
git branch -b mybranch lastcommit
# can be optimized with -b option ...
git checkout mybranch
git stash pop
git merge --squash master
git commit -a -F - <<EOF
commit message
EOF
# Signoff must be added
git format-patch -s master
git checkout master
# Send mail

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Stefan Hajnoczi]

On Sun, Jan 08, 2012 at 09:56:20PM +0100, Gerhard Wiesinger wrote:
> On Sun, 8 Jan 2012, Stefan Hajnoczi wrote:
> 
> >On Sun, Jan 8, 2012 at 11:55 AM, Gerhard Wiesinger <lists@wiesinger.com> wrote:
> >>Option ROM for network interface cards (NICs) can now explicitly disabled
> >>with romfile=disabled parameter. With hotplugable NICs (currently NE2000,
> >>PCNET)
> >>romfile=(empty) didn't work. This patch disables Option ROMs for iPXE for
> >>alls
> >>supported NICs (hotplugable and non hotplugable).
> >>
> >>Examples with 2 NICs with disabled Option ROM (separated on different lines
> >>for readability):
> >>-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
> >>-net tap,ifname=tap0,script=no,downscript=no,vlan=0
> >>-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
> >>-net tap,ifname=tap1,script=no,downscript=no,vlan=1
> >
> >Did you consider "no" or "none"?  Those are already used by -net
> >script=no and -vga none.  I'm afraid we don't have much consistency in
> >the command-line and adding more variants of basically the same
> >concept should be avoided when possible.
> 
> Whether the option is spelled "disabled", "no", "none" or all of
> them above: I don't care about that, just the group of maintainers
> should decide (I would prefer all of them ...).
> 
> The only thing: just help me with git and doing another commit and
> preparing V2 patch then ...
> 
> # I did:
> git stash save mypatch
> git branch -b mybranch lastcommit
> # can be optimized with -b option ...
> git checkout mybranch
> git stash pop
> git merge --squash master
> git commit -a -F - <<EOF
> commit message
> EOF
> # Signoff must be added
> git format-patch -s master
> git checkout master
> # Send mail

Okay, you commited your patch onto the 'mypatch' branch.

$ git checkout mypatch  # go back to your branch
$ $EDITOR               # make changes to the code
$ git commit --amend -a # modify the last commit
$ git format-patch -s HEAD^ # generate new patch

Stefan

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerd Hoffmann]

  Hi,

>      if (!pci_dev->qdev.hotplugged) {
>          static int loaded = 0;
> -        if (!loaded) {
> +        if (!loaded &&
> pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
>              rom_add_option("pxe-ne2k_pci.rom", -1);
>              loaded = 1;
>          }

I think you can just remove this altogether and add a .romfile = "..."
entry instead like it is done for the other nics.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Mon, 9 Jan 2012, Gerd Hoffmann wrote:

>  Hi,
>
>>      if (!pci_dev->qdev.hotplugged) {
>>          static int loaded = 0;
>> -        if (!loaded) {
>> +        if (!loaded &&
>> pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
>>              rom_add_option("pxe-ne2k_pci.rom", -1);
>>              loaded = 1;
>>          }
>
> I think you can just remove this altogether and add a .romfile = "..."
> entry instead like it is done for the other nics.

I'm not sure about the consequences (hotplugging feature, etc.) when 
changing it to romfile as in other PCI devices. Also the patch is more 
generic and supports static and dynamic devices (hotplugable and possible 
future devices). Patch supports without hotplugging both options to 
disable the romfile:
,romfile=
,romfile=disabled

And the patch has already been tested ...

Therefore I suggest to commit it and maybe make a refactoring without 
hotplugging later on.

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerd Hoffmann]

  Hi,

> I'm not sure about the consequences (hotplugging feature, etc.) when
> changing it to romfile as in other PCI devices.

There should be no noticable difference.

> Also the patch is more
> generic and supports static and dynamic devices (hotplugable and
> possible future devices).

Hotplug works just fine for the other pci devices.

> Patch supports without hotplugging both
> options to disable the romfile:
> ,romfile=
> ,romfile=disabled

That should be a separate patch.  And it should update
pci_add_option_rom() so it works equally for all pci devices.

> And the patch has already been tested ...

Well, it adds a bunch of code which would not be needed in the first
place if you would simply make use of the romfile support of the pci layer.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Thu, 12 Jan 2012, Gerd Hoffmann wrote:

>  Hi,
>
>> I'm not sure about the consequences (hotplugging feature, etc.) when
>> changing it to romfile as in other PCI devices.
>
> There should be no noticable difference.

I don't know the consequences there so I think it is better to let that 
code and just fix the romfile issues.

>
>> Also the patch is more
>> generic and supports static and dynamic devices (hotplugable and
>> possible future devices).
>
> Hotplug works just fine for the other pci devices.
>
>> Patch supports without hotplugging both
>> options to disable the romfile:
>> ,romfile=
>> ,romfile=disabled
>
> That should be a separate patch.  And it should update
> pci_add_option_rom() so it works equally for all pci devices.

pci_add_option_rom() has been updated, so it is generic for all pci 
devices. Please look at the patch ...

>> And the patch has already been tested ...
>
> Well, it adds a bunch of code which would not be needed in the first
> place if you would simply make use of the romfile support of the pci layer.

As discussed it modifies romfile support on the pci layer ...

I reworked code for "disabled", "no" and "none" options, all these options 
work now.

Submitted new patch.

Ciao,
Gerhard

--
http://www.wiesinger.com/

[Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

Option ROM for network interface cards (NICs) can now explicitly disabled
with romfile=disabled (or romfile=no or romfile=none) parameter.
With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
This patch disables Option ROMs for iPXE for alls supported NICs
(hotplugable and non hotplugable).

Examples with 2 NICs with disabled Option ROM (separated on different lines for readabi$
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
-net tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
-net tap,ifname=tap1,script=no,downscript=no,vlan=1

Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>
---
  hw/ne2000.c    |    2 +-
  hw/pci.c       |   39 ++++++++++++++++++++++++++++++++++++---
  hw/pci.h       |    7 +++++++
  hw/pcnet-pci.c |    2 +-
  4 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/hw/ne2000.c b/hw/ne2000.c
index 62e082f..67bf458 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -765,7 +765,7 @@ static int pci_ne2000_init(PCIDevice *pci_dev)

      if (!pci_dev->qdev.hotplugged) {
          static int loaded = 0;
-        if (!loaded) {
+        if (!loaded && pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
              rom_add_option("pxe-ne2k_pci.rom", -1);
              loaded = 1;
          }
diff --git a/hw/pci.c b/hw/pci.c
index c3082bc..a9e0758 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -126,6 +126,41 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
      bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
  }

+static int pci_is_disabled_romfile_string(const char *s)
+{
+    if (strcmp(s, PCI_DEVICE_DISABLED_OPTION_ROMFILE) == 0)
+        return 1;
+    if (strcmp(s, PCI_DEVICE_DISABLED_OPTION_ROMFILE_ALTERNATE1) == 0)
+        return 1;
+    if (strcmp(s, PCI_DEVICE_DISABLED_OPTION_ROMFILE_ALTERNATE2) == 0)
+        return 1;
+    return 0;
+}
+
+int pci_has_enabled_option_romfile(PCIDevice *pdev)
+{
+    PCI_DPRINTF("pci_has_enabled_option_romfile: device=%s, romfile=%s\n", pdev->name, pdev->romfile);
+    if (pdev->romfile == NULL)
+        return 0;
+    if (strlen(pdev->romfile) == 0)
+        return 0;
+    if (pci_is_disabled_romfile_string(pdev->romfile))
+        return 0;
+    return 1;
+}
+
+int pci_has_not_explicitly_disabled_option_romfile(PCIDevice *pdev)
+{
+    PCI_DPRINTF("pci_has_not_explicitly_disabled_option_romfile: device=%s, romfile=%s\n", pdev->name, pdev->romfile);
+
+    /* No romfile is present for hotplugged devices, therefore dynamic codes decides */
+    if (pdev->romfile == NULL)
+        return 1;
+    if (pci_is_disabled_romfile_string(pdev->romfile))
+        return 0;
+    return 1;
+}
+
  int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
  {
      assert(irq_num >= 0);
@@ -1725,9 +1760,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
      void *ptr;
      char name[32];

-    if (!pdev->romfile)
-        return 0;
-    if (strlen(pdev->romfile) == 0)
+    if (!pci_has_enabled_option_romfile(pdev))
          return 0;

      if (!pdev->rom_bar) {
diff --git a/hw/pci.h b/hw/pci.h
index 625e717..c5d071d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -78,6 +78,10 @@

  #define FMT_PCIBUS                      PRIx64

+#define PCI_DEVICE_DISABLED_OPTION_ROMFILE "disabled"
+#define PCI_DEVICE_DISABLED_OPTION_ROMFILE_ALTERNATE1 "none"
+#define PCI_DEVICE_DISABLED_OPTION_ROMFILE_ALTERNATE2 "no"
+
  typedef void PCIConfigWriteFunc(PCIDevice *pci_dev,
                                  uint32_t address, uint32_t data, int len);
  typedef uint32_t PCIConfigReadFunc(PCIDevice *pci_dev,
@@ -275,6 +279,9 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp,

  void pci_device_deassert_intx(PCIDevice *dev);

+int pci_has_enabled_option_romfile(PCIDevice *pdev);
+int pci_has_not_explicitly_disabled_option_romfile(PCIDevice *pdev);
+
  static inline void
  pci_set_byte(uint8_t *config, uint8_t val)
  {
diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
index 4e164da..e65745f 100644
--- a/hw/pcnet-pci.c
+++ b/hw/pcnet-pci.c
@@ -332,7 +332,7 @@ static int pci_pcnet_init(PCIDevice *pci_dev)

      if (!pci_dev->qdev.hotplugged) {
          static int loaded = 0;
-        if (!loaded) {
+        if (!loaded && pci_has_not_explicitly_disabled_option_romfile(pci_dev)) {
              rom_add_option("pxe-pcnet.rom", -1);
              loaded = 1;
          }
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Markus Armbruster]

Gerhard Wiesinger <lists@wiesinger.com> writes:

> Option ROM for network interface cards (NICs) can now explicitly disabled
> with romfile=disabled (or romfile=no or romfile=none) parameter.
> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
> This patch disables Option ROMs for iPXE for alls supported NICs
> (hotplugable and non hotplugable).

And now filenames "disabled", "no" and "none" don't work.

Any way to fix "romfile="?

> Examples with 2 NICs with disabled Option ROM (separated on different lines for readabi$
> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,romfile=disabled
> -net tap,ifname=tap0,script=no,downscript=no,vlan=0
> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,romfile=disabled
> -net tap,ifname=tap1,script=no,downscript=no,vlan=1
>
> Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com>

I_do_not_usually_do_this_but_I_make_an_exception_due_to_your_choice_of_illegibly_long_identifiers:
CODING_STYLE, please feed to scripts/checkpatch.pl and correct :)

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerd Hoffmann]

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

On 01/26/12 08:45, Markus Armbruster wrote:
> Gerhard Wiesinger <lists@wiesinger.com> writes:
> 
>> Option ROM for network interface cards (NICs) can now explicitly disabled
>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
>> This patch disables Option ROMs for iPXE for alls supported NICs
>> (hotplugable and non hotplugable).
> 
> And now filenames "disabled", "no" and "none" don't work.
> 
> Any way to fix "romfile="?

Sure.

cheers,
  Gerd

[-- Attachment #2: 0001-nic-zap-obsolote-romloading-bits-from-ne2k-pcnet.patch --]
[-- Type: text/plain, Size: 2396 bytes --]

>From 4c17b5a36e6228536318e06a18a41166ff8356c5 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 26 Jan 2012 11:40:07 +0100
Subject: [PATCH] nic: zap obsolote romloading bits from ne2k + pcnet

These days one just needs to specify the romfile in PCiDeviceInfo and
everything magically works.  It also allows to disable pxe rom loading
via "romfile=<emptystring>" like it is possible for all other nics.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ne2000.c    |    9 +--------
 hw/pcnet-pci.c |    9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/hw/ne2000.c b/hw/ne2000.c
index 62e082f..83328bb 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -763,14 +763,6 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
                           pci_dev->qdev.info->name, pci_dev->qdev.id, s);
     qemu_format_nic_info_str(&s->nic->nc, s->c.macaddr.a);
 
-    if (!pci_dev->qdev.hotplugged) {
-        static int loaded = 0;
-        if (!loaded) {
-            rom_add_option("pxe-ne2k_pci.rom", -1);
-            loaded = 1;
-        }
-    }
-
     add_boot_device_path(s->c.bootindex, &pci_dev->qdev, "/ethernet-phy@0");
 
     return 0;
@@ -792,6 +784,7 @@ static PCIDeviceInfo ne2000_info = {
     .qdev.vmsd  = &vmstate_pci_ne2000,
     .init       = pci_ne2000_init,
     .exit       = pci_ne2000_exit,
+    .romfile    = "pxe-ne2k_pci.rom",
     .vendor_id  = PCI_VENDOR_ID_REALTEK,
     .device_id  = PCI_DEVICE_ID_REALTEK_8029,
     .class_id   = PCI_CLASS_NETWORK_ETHERNET,
diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
index 4e164da..2f333e2 100644
--- a/hw/pcnet-pci.c
+++ b/hw/pcnet-pci.c
@@ -330,14 +330,6 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
     s->phys_mem_write = pci_physical_memory_write;
     s->dma_opaque = pci_dev;
 
-    if (!pci_dev->qdev.hotplugged) {
-        static int loaded = 0;
-        if (!loaded) {
-            rom_add_option("pxe-pcnet.rom", -1);
-            loaded = 1;
-        }
-    }
-
     return pcnet_common_init(&pci_dev->qdev, s, &net_pci_pcnet_info);
 }
 
@@ -355,6 +347,7 @@ static PCIDeviceInfo pcnet_info = {
     .qdev.vmsd  = &vmstate_pci_pcnet,
     .init       = pci_pcnet_init,
     .exit       = pci_pcnet_uninit,
+    .romfile    = "pxe-pcnet.rom",
     .vendor_id  = PCI_VENDOR_ID_AMD,
     .device_id  = PCI_DEVICE_ID_AMD_LANCE,
     .revision   = 0x10,
-- 
1.7.1

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 01/26/12 08:45, Markus Armbruster wrote:
>> Gerhard Wiesinger <lists@wiesinger.com> writes:
>> 
>>> Option ROM for network interface cards (NICs) can now explicitly disabled
>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>> (hotplugable and non hotplugable).
>> 
>> And now filenames "disabled", "no" and "none" don't work.
>> 
>> Any way to fix "romfile="?
>
> Sure.

This patch looks much better.  Gerhard does it solve your problem?
Gerd, you might want to repost it in its own thread, as maintainers can
easily miss patches buried deep in replies.

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Thu, 26 Jan 2012, Markus Armbruster wrote:

> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> On 01/26/12 08:45, Markus Armbruster wrote:
>>> Gerhard Wiesinger <lists@wiesinger.com> writes:
>>>
>>>> Option ROM for network interface cards (NICs) can now explicitly disabled
>>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
>>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>>> (hotplugable and non hotplugable).
>>>
>>> And now filenames "disabled", "no" and "none" don't work.
>>>
>>> Any way to fix "romfile="?
>>
>> Sure.
>
> This patch looks much better.  Gerhard does it solve your problem?
> Gerd, you might want to repost it in its own thread, as maintainers can
> easily miss patches buried deep in replies.

Patch looks good to me, it just initializes like all other devices.

But I still have to test it. Hopefully today in the evening or tomorrow.

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Thu, 26 Jan 2012, Markus Armbruster wrote:

> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> On 01/26/12 08:45, Markus Armbruster wrote:
>>> Gerhard Wiesinger <lists@wiesinger.com> writes:
>>>
>>>> Option ROM for network interface cards (NICs) can now explicitly disabled
>>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
>>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>>> (hotplugable and non hotplugable).
>>>
>>> And now filenames "disabled", "no" and "none" don't work.
>>>
>>> Any way to fix "romfile="?
>>
>> Sure.
>
> This patch looks much better.  Gerhard does it solve your problem?
> Gerd, you might want to repost it in its own thread, as maintainers can
> easily miss patches buried deep in replies.

Can confirm this patch works well in all tried combinations.

3 Comments:
1.) NOK?: 2 NICs installed, no bootindex specified: Tries to boot only 
from one NIC, then from C: (one NIC has index first, second one has last 
index)
-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0 -net tap,ifname=tap0,script=no,downscript=no,vlan=0
  -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1 -net tap,ifname=tap1,script=no,downscript=no,vlan=1
I would expect to try to boot from both NICs
1. iPXE (PCI 00:04:0)
...
8. iPXE (PCI 00:05:0)

2.) OK: 2 NICs installed, bootindex specified: Tries to boot from first 
and second NIC
-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=1 -net tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=2 -net tap,ifname=tap1,script=no,downscript=no,vlan=1
1. iPXE (PCI 00:04:0)
2. iPXE (PCI 00:05:0)
...

3.) NOK: 2 NICs installed, bootindex specified in reverse order: Tries to 
boot from 7e NIC and reboots ...
-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=2 -net tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=1 -net tap,ifname=tap1,script=no,downscript=no,vlan=1
1. iPXE (PCI 00:05:0)
2. iPXE (PCI 00:04:0)
...

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Fri, 27 Jan 2012, Gerhard Wiesinger wrote:

> On Thu, 26 Jan 2012, Markus Armbruster wrote:
>
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>> 
>>> On 01/26/12 08:45, Markus Armbruster wrote:
>>>> Gerhard Wiesinger <lists@wiesinger.com> writes:
>>>> 
>>>>> Option ROM for network interface cards (NICs) can now explicitly 
>>>>> disabled
>>>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't 
>>>>> work.
>>>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>>>> (hotplugable and non hotplugable).
>>>> 
>>>> And now filenames "disabled", "no" and "none" don't work.
>>>> 
>>>> Any way to fix "romfile="?
>>> 
>>> Sure.
>> 
>> This patch looks much better.  Gerhard does it solve your problem?
>> Gerd, you might want to repost it in its own thread, as maintainers can
>> easily miss patches buried deep in replies.
>
> Can confirm this patch works well in all tried combinations.
>
> 3 Comments:
> 1.) NOK?: 2 NICs installed, no bootindex specified: Tries to boot only from 
> one NIC, then from C: (one NIC has index first, second one has last index)
> -boot order=nca,menu=on
> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0 -net 
> tap,ifname=tap0,script=no,downscript=no,vlan=0
> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1 -net 
> tap,ifname=tap1,script=no,downscript=no,vlan=1
> I would expect to try to boot from both NICs
> 1. iPXE (PCI 00:04:0)
> ...
> 8. iPXE (PCI 00:05:0)
>
> 2.) OK: 2 NICs installed, bootindex specified: Tries to boot from first and 
> second NIC
> -boot order=nca,menu=on
> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=1 -net 
> tap,ifname=tap0,script=no,downscript=no,vlan=0
> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=2 -net 
> tap,ifname=tap1,script=no,downscript=no,vlan=1
> 1. iPXE (PCI 00:04:0)
> 2. iPXE (PCI 00:05:0)
> ...
>
> 3.) NOK: 2 NICs installed, bootindex specified in reverse order: Tries to 
> boot from 7e NIC and reboots ...
> -boot order=nca,menu=on
> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=2 -net 
> tap,ifname=tap0,script=no,downscript=no,vlan=0
> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=1 -net 
> tap,ifname=tap1,script=no,downscript=no,vlan=1
> 1. iPXE (PCI 00:05:0)
> 2. iPXE (PCI 00:04:0)
> ...

Still waiting for commit ...

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Gerhard Wiesinger]

On Mon, 13 Feb 2012, Gerhard Wiesinger wrote:

> On Fri, 27 Jan 2012, Gerhard Wiesinger wrote:
>
>> On Thu, 26 Jan 2012, Markus Armbruster wrote:
>> 
>>> Gerd Hoffmann <kraxel@redhat.com> writes:
>>> 
>>>> On 01/26/12 08:45, Markus Armbruster wrote:
>>>>> Gerhard Wiesinger <lists@wiesinger.com> writes:
>>>>> 
>>>>>> Option ROM for network interface cards (NICs) can now explicitly 
>>>>>> disabled
>>>>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>>>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't 
>>>>>> work.
>>>>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>>>>> (hotplugable and non hotplugable).
>>>>> 
>>>>> And now filenames "disabled", "no" and "none" don't work.
>>>>> 
>>>>> Any way to fix "romfile="?
>>>> 
>>>> Sure.
>>> 
>>> This patch looks much better.  Gerhard does it solve your problem?
>>> Gerd, you might want to repost it in its own thread, as maintainers can
>>> easily miss patches buried deep in replies.
>> 
>> Can confirm this patch works well in all tried combinations.
>> 
>> 3 Comments:
>> 1.) NOK?: 2 NICs installed, no bootindex specified: Tries to boot only from 
>> one NIC, then from C: (one NIC has index first, second one has last index)
>> -boot order=nca,menu=on
>> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0 -net 
>> tap,ifname=tap0,script=no,downscript=no,vlan=0
>> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1 -net 
>> tap,ifname=tap1,script=no,downscript=no,vlan=1
>> I would expect to try to boot from both NICs
>> 1. iPXE (PCI 00:04:0)
>> ...
>> 8. iPXE (PCI 00:05:0)
>> 
>> 2.) OK: 2 NICs installed, bootindex specified: Tries to boot from first and 
>> second NIC
>> -boot order=nca,menu=on
>> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=1 -net 
>> tap,ifname=tap0,script=no,downscript=no,vlan=0
>> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=2 -net 
>> tap,ifname=tap1,script=no,downscript=no,vlan=1
>> 1. iPXE (PCI 00:04:0)
>> 2. iPXE (PCI 00:05:0)
>> ...
>> 
>> 3.) NOK: 2 NICs installed, bootindex specified in reverse order: Tries to 
>> boot from 7e NIC and reboots ...
>> -boot order=nca,menu=on
>> -device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=2 -net 
>> tap,ifname=tap0,script=no,downscript=no,vlan=0
>> -device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=1 -net 
>> tap,ifname=tap1,script=no,downscript=no,vlan=1
>> 1. iPXE (PCI 00:05:0)
>> 2. iPXE (PCI 00:04:0)
>> ...
>
> Still waiting for commit ...
>
> Ciao,
> Gerhard


Still waiting for commit ...

Ciao,
Gerhard

--
http://www.wiesinger.com/

Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

[Anthony Liguori]

On 01/26/2012 04:45 AM, Gerd Hoffmann wrote:
> On 01/26/12 08:45, Markus Armbruster wrote:
>> Gerhard Wiesinger<lists@wiesinger.com>  writes:
>>
>>> Option ROM for network interface cards (NICs) can now explicitly disabled
>>> with romfile=disabled (or romfile=no or romfile=none) parameter.
>>> With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't work.
>>> This patch disables Option ROMs for iPXE for alls supported NICs
>>> (hotplugable and non hotplugable).
>>
>> And now filenames "disabled", "no" and "none" don't work.
>>
>> Any way to fix "romfile="?
>
> Sure.

Care to submit as a proper patch?

Regards,

Anthony Liguori

>
> cheers,
>    Gerd