diff mbox

network: Added option to disable NIC option roms

Message ID 4F212EC7.3010500@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Jan. 26, 2012, 10:45 a.m. UTC 
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
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(-)

Comments

Markus Armbruster Jan. 26, 2012, 11 a.m. UTC | #1 
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.
Gerhard Wiesinger Jan. 27, 2012, 6:56 a.m. UTC | #2 
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/
Gerhard Wiesinger Jan. 27, 2012, 4:02 p.m. UTC | #3 
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/
Anthony Liguori Feb. 1, 2012, 10:19 p.m. UTC | #4 
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
Gerhard Wiesinger Feb. 13, 2012, 6:23 a.m. UTC | #5 
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/
Gerhard Wiesinger Feb. 16, 2012, 6:41 a.m. UTC | #6 
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/
diff mbox

Patch

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,