[Qemu-devel] [RFC PATCH 0/2] change q35 default NIC to e1000e

[Qemu-devel] [RFC PATCH 0/2] change q35 default NIC to e1000e

[Paolo Bonzini]

The Intel 82574 NIC has better performance and more features than
the aging e1000 (aka 82540), for example MSI-X.  This patch chooses it
by default for the Q35 machine type.

Drivers for 82574 were added first to Linux 2.6.27 (2008) and Windows
2008 R2.  This does mean that Windows 2008 will not work anymore with Q35
machine types and a default "-net nic -net xxx" network configuration;
it did work before because it does have an AHCI driver.  However, Windows
2008 has been declared out of main stream support in 2015.  It will
get out of extended support in 2020.  Windows 2008 R2 has the same end
of support dates and, since the two are basically Vista vs. Windows 7,
R2 probably is more popular.

Opinions?

Paolo

Paolo Bonzini (2):
  net: add e1000e model to the "simple" -net/-nic options
  q35: change default NIC to e1000e

 hw/i386/pc.c         | 5 +++--
 hw/i386/pc_piix.c    | 2 +-
 hw/i386/pc_q35.c     | 2 +-
 hw/pci/pci.c         | 2 ++
 include/hw/i386/pc.h | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/2] net: add e1000e model to the "simple" -net/-nic options

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e006b6ac71..af3c85a46f 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1822,6 +1822,7 @@ static const char * const pci_nic_models[] = {
     "i82559er",
     "rtl8139",
     "e1000",
+    "e1000e",
     "pcnet",
     "virtio",
     "sungem",
@@ -1835,6 +1836,7 @@ static const char * const pci_nic_names[] = {
     "i82559er",
     "rtl8139",
     "e1000",
+    "e1000e",
     "pcnet",
     "virtio-net-pci",
     "sungem",
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/2] net: add e1000e model to the "simple" -net/-nic options

[Thomas Huth]

On 02.03.2018 16:51, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/pci/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index e006b6ac71..af3c85a46f 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1822,6 +1822,7 @@ static const char * const pci_nic_models[] = {
>      "i82559er",
>      "rtl8139",
>      "e1000",
> +    "e1000e",
>      "pcnet",
>      "virtio",
>      "sungem",
> @@ -1835,6 +1836,7 @@ static const char * const pci_nic_names[] = {
>      "i82559er",
>      "rtl8139",
>      "e1000",
> +    "e1000e",
>      "pcnet",
>      "virtio-net-pci",
>      "sungem",

I think it would be better and more consisten to finally fix this mess
and automatically allow all devices in the category
DEVICE_CATEGORY_NETWORK that are derived from TYPE_PCI_DEVICE, instead
of manually maintaining this list here...?

 Thomas

Re: [Qemu-devel] [PATCH 1/2] net: add e1000e model to the "simple" -net/-nic options

[Paolo Bonzini]

On 02/03/2018 18:19, Thomas Huth wrote:
> On 02.03.2018 16:51, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/pci/pci.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index e006b6ac71..af3c85a46f 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1822,6 +1822,7 @@ static const char * const pci_nic_models[] = {
>>      "i82559er",
>>      "rtl8139",
>>      "e1000",
>> +    "e1000e",
>>      "pcnet",
>>      "virtio",
>>      "sungem",
>> @@ -1835,6 +1836,7 @@ static const char * const pci_nic_names[] = {
>>      "i82559er",
>>      "rtl8139",
>>      "e1000",
>> +    "e1000e",
>>      "pcnet",
>>      "virtio-net-pci",
>>      "sungem",
> 
> I think it would be better and more consisten to finally fix this mess
> and automatically allow all devices in the category
> DEVICE_CATEGORY_NETWORK that are derived from TYPE_PCI_DEVICE, instead
> of manually maintaining this list here...?

Yeah, that would make sense too (virtio would be special cased).  Thanks!

Paolo

[Qemu-devel] [PATCH 2/2] q35: change default NIC to e1000e

[Paolo Bonzini]

The e1000 NIC is getting old and is not a very good default for a
PCIe machine type.  Change it to e1000e, which should be supported
by a good number of guests.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c         | 5 +++--
 hw/i386/pc_piix.c    | 2 +-
 hw/i386/pc_q35.c     | 2 +-
 include/hw/i386/pc.h | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2f7746c859..1398ac9b68 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1619,7 +1619,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     }
 }
 
-void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus)
+void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus, const char *default_model)
 {
     int i;
 
@@ -1630,7 +1630,8 @@ void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus)
         if (!pci_bus || (nd->model && strcmp(nd->model, "ne2k_isa") == 0)) {
             pc_init_ne2k_isa(isa_bus, nd);
         } else {
-            pci_nic_init_nofail(nd, pci_bus, "e1000", NULL);
+            const char *model = nd->model ? nd->model : default_model;
+            pci_nic_init_nofail(nd, pci_bus, model, NULL);
         }
     }
     rom_reset_order_override();
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8658bcba63..309b052ed1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -240,7 +240,7 @@ static void pc_init1(MachineState *machine,
     pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
                          (pcms->vmport != ON_OFF_AUTO_ON), pcms->pit, 0x4);
 
-    pc_nic_init(isa_bus, pci_bus);
+    pc_nic_init(isa_bus, pci_bus, "e1000");
 
     ide_drive_get(hd, ARRAY_SIZE(hd));
     if (pcmc->pci_enabled) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0c0bc48137..764179af08 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -272,7 +272,7 @@ static void pc_q35_init(MachineState *machine)
 
     /* the rest devices to which pci devfn is automatically assigned */
     pc_vga_init(isa_bus, host_bus);
-    pc_nic_init(isa_bus, host_bus);
+    pc_nic_init(isa_bus, host_bus, "e1000e");
 
     if (pcms->acpi_nvdimm_state.is_enabled) {
         nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bb49165fe0..25d71f3bb5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -248,7 +248,7 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
 void pc_cmos_init(PCMachineState *pcms,
                   BusState *ide0, BusState *ide1,
                   ISADevice *s);
-void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);
+void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus, const char *default_model);
 void pc_pci_device_init(PCIBus *pci_bus);
 
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
-- 
2.14.3

Re: [Qemu-devel] [PATCH 2/2] q35: change default NIC to e1000e

[Thomas Huth]

On 02.03.2018 16:51, Paolo Bonzini wrote:
> The e1000 NIC is getting old and is not a very good default for a
> PCIe machine type.  Change it to e1000e, which should be supported
> by a good number of guests.

Basically a good idea, but you can only do that for new machine types
(pc-q35-2.12 and later), otherwise migration from an older version will
fail if the user tries to migrate with the default model.

 Thomas

Re: [Qemu-devel] [PATCH 2/2] q35: change default NIC to e1000e

[Paolo Bonzini]

On 02/03/2018 18:24, Thomas Huth wrote:
> On 02.03.2018 16:51, Paolo Bonzini wrote:
>> The e1000 NIC is getting old and is not a very good default for a
>> PCIe machine type.  Change it to e1000e, which should be supported
>> by a good number of guests.
> 
> Basically a good idea, but you can only do that for new machine types
> (pc-q35-2.12 and later), otherwise migration from an older version will
> fail if the user tries to migrate with the default model.

Yeah, I was just throwing out the idea.

Paolo