[Qemu-devel] [PATCH 0/6] More sensible default for -drive interface type

[Qemu-devel] [PATCH 0/6] More sensible default for -drive interface type

[Markus Armbruster]

Block backends defined with -drive if=T, T!=none are meant to be
picked up by machine initialization code: a suitable frontend gets
created and wired up automatically.

if=T drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

Many machine types default to if=ide, even though they don't actually
have an IDE controller.  A few default to if=scsi, even though they
lack a SCSI HBA.  Change their default to if=none.

While their, fix handling of index and unit for a few boards with AHCI
controllers, and stop auto-creating the thoroughly obsolete lsi SCSI
HBAs for new PC machine types.

Markus Armbruster (6):
  hw: Default -drive to if=ide explicitly where it works
  hw/arm/cubieboard hw/arm/xlnx-ep108: Fix units_per_default_bus
  hw: Default -drive to if=none instead of ide when ide cannot work
  hw: Default -drive to if=none instead of scsi when scsi cannot work
  hw/arm/highbank: Default -drive to if=ide instead of if=scsi
  hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

 hw/alpha/dp264.c          |  1 +
 hw/arm/cubieboard.c       |  4 ++++
 hw/arm/highbank.c         |  8 ++++++--
 hw/arm/realview.c         |  1 -
 hw/arm/spitz.c            |  3 +++
 hw/arm/tosa.c             |  1 +
 hw/arm/vexpress.c         |  1 -
 hw/arm/xilinx_zynq.c      |  1 -
 hw/arm/xlnx-ep108.c       |  6 ++++++
 hw/i386/pc.c              |  1 +
 hw/i386/pc_piix.c         | 15 ++++++++++++++-
 hw/i386/pc_q35.c          |  7 ++++++-
 hw/mips/mips_fulong2e.c   |  1 +
 hw/mips/mips_malta.c      |  1 +
 hw/mips/mips_r4k.c        |  1 +
 hw/ppc/mac_newworld.c     |  1 +
 hw/ppc/mac_oldworld.c     |  1 +
 hw/ppc/prep.c             |  1 +
 hw/sh4/r2d.c              |  1 +
 hw/sparc64/sun4u.c        |  2 ++
 include/hw/i386/pc.h      |  3 +++
 include/sysemu/blockdev.h |  9 ++++-----
 22 files changed, 58 insertions(+), 12 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/6] hw: Default -drive to if=ide explicitly where it works

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types default to if=ide, even though they don't actually
have an IDE controller.  A future patch will change these defaults to
something more sensible.  To prepare for it, this patch makes default
"ide" explicit for the machines that actually pick up if=ide drives:

* alpha: clipper
* arm/aarch64: spitz borzoi terrier tosa
* i386/x86_64: generic-pc-machine (with concrete subtypes pc-q35-*
  pc-i440fx-* pc-* isapc xenfv)
* mips64el: fulong2e
* mips/mipsel/mips64el: malta mips
* ppc/ppc64: mac99 g3beige prep
* sh4/sh4eb: r2d
* sparc64: sun4u sun4v

Note that ppc64 machine powernv already sets an "ide" default
explicitly.  Its IDE controller isn't implemented, yet.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/alpha/dp264.c        | 1 +
 hw/arm/spitz.c          | 3 +++
 hw/arm/tosa.c           | 1 +
 hw/i386/pc.c            | 1 +
 hw/mips/mips_fulong2e.c | 1 +
 hw/mips/mips_malta.c    | 1 +
 hw/mips/mips_r4k.c      | 1 +
 hw/ppc/mac_newworld.c   | 1 +
 hw/ppc/mac_oldworld.c   | 1 +
 hw/ppc/prep.c           | 1 +
 hw/sh4/r2d.c            | 1 +
 hw/sparc64/sun4u.c      | 2 ++
 12 files changed, 15 insertions(+)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index d6431fd..85405da 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -177,6 +177,7 @@ static void clipper_machine_init(MachineClass *mc)
 {
     mc->desc = "Alpha DP264/CLIPPER";
     mc->init = clipper_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 4;
     mc->is_default = 1;
 }
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 949a15a..fe2d5a7 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -998,6 +998,7 @@ static void spitzpda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)";
     mc->init = spitz_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo spitzpda_type = {
@@ -1012,6 +1013,7 @@ static void borzoipda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)";
     mc->init = borzoi_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo borzoipda_type = {
@@ -1026,6 +1028,7 @@ static void terrierpda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)";
     mc->init = terrier_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo terrierpda_type = {
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index c3db996..9f58a23 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -263,6 +263,7 @@ static void tosapda_machine_init(MachineClass *mc)
 {
     mc->desc = "Sharp SL-6000 (Tosa) PDA (PXA255)";
     mc->init = tosa_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("tosa", tosapda_machine_init)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 25e8586..33bc5d1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2343,6 +2343,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->query_hotpluggable_cpus = pc_query_hotpluggable_cpus;
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 255;
     mc->reset = pc_machine_reset;
     hc->pre_plug = pc_machine_device_pre_plug_cb;
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9a4dae4..e636c3a 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -387,6 +387,7 @@ static void mips_fulong2e_machine_init(MachineClass *mc)
 {
     mc->desc = "Fulong 2e mini pc";
     mc->init = mips_fulong2e_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("fulong2e", mips_fulong2e_machine_init)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index cf48f42..6b63179 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1264,6 +1264,7 @@ static void mips_malta_machine_init(MachineClass *mc)
 {
     mc->desc = "MIPS Malta Core LV";
     mc->init = mips_malta_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 16;
     mc->is_default = 1;
 }
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 27548c4..748586e 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -306,6 +306,7 @@ static void mips_machine_init(MachineClass *mc)
 {
     mc->desc = "mips r4k platform";
     mc->init = mips_r4k_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("mips", mips_machine_init)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 2bfdb64..716aea6 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -518,6 +518,7 @@ static void core99_machine_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Mac99 based PowerMAC";
     mc->init = ppc_core99_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
     mc->default_boot_order = "cd";
     mc->kvm_type = core99_kvm_type;
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 56282c5..5df94e2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -368,6 +368,7 @@ static void heathrow_machine_init(MachineClass *mc)
 {
     mc->desc = "Heathrow based PowerMAC";
     mc->init = ppc_heathrow_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
 #ifndef TARGET_PPC64
     mc->is_default = 1;
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 054af1e..ebf6af5 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -673,6 +673,7 @@ static void prep_machine_init(MachineClass *mc)
 {
     mc->desc = "PowerPC PREP platform";
     mc->init = ppc_prep_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
     mc->default_boot_order = "cad";
 }
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index db373c7..6d06968 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -362,6 +362,7 @@ static void r2d_machine_init(MachineClass *mc)
 {
     mc->desc = "r2d-plus board";
     mc->init = r2d_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("r2d", r2d_machine_init)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d1a6bca..d347b66 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -579,6 +579,7 @@ static void sun4u_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sun4u platform";
     mc->init = sun4u_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 1; /* XXX for now */
     mc->is_default = 1;
     mc->default_boot_order = "c";
@@ -596,6 +597,7 @@ static void sun4v_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sun4v platform";
     mc->init = sun4v_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 1; /* XXX for now */
     mc->default_boot_order = "c";
 }
-- 
2.7.4

[Qemu-devel] [PATCH 2/6] hw/arm/cubieboard hw/arm/xlnx-ep108: Fix units_per_default_bus

[Markus Armbruster]

Machine types cubieboard, xlnx-ep108, xlnx-zcu102 have an onboard AHCI
controller, but neglect to set their MachineClass member
units_per_default_bus = 1.  This permits -drive if=ide,unit=1, which
makes no sense for AHCI.  It also screws up index=N for odd N, because
it gets desugared to unit=1,bus=N/2

Doesn't really matter, because these machine types fail to honor
-drive if=ide.  Add the missing units_per_default_bus = 1 anyway,
along with a TODO comment on what needs to be done for -drive if=ide.

Also set block_default_type = IF_IDE explicitly.  It's currently the
default, but the next commit will change it to something more
sensible, and we want to keep the IF_IDE default for these three
machines.  See also the previous commit.

Cc: Beniamino Galvani <b.galvani@gmail.com>
Cc: Alistair Francis <alistair.francis@xilinx.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/cubieboard.c | 4 ++++
 hw/arm/xlnx-ep108.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index dd19ba3..b98e1c4 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -71,6 +71,8 @@ static void cubieboard_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
                                 &s->sdram);
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     cubieboard_binfo.ram_size = machine->ram_size;
     cubieboard_binfo.kernel_filename = machine->kernel_filename;
     cubieboard_binfo.kernel_cmdline = machine->kernel_cmdline;
@@ -82,6 +84,8 @@ static void cubieboard_machine_init(MachineClass *mc)
 {
     mc->desc = "cubietech cubieboard";
     mc->init = cubieboard_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("cubieboard", cubieboard_machine_init)
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 4ec590a..860780a 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -106,6 +106,8 @@ static void xlnx_ep108_init(MachineState *machine)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
     }
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     xlnx_ep108_binfo.ram_size = ram_size;
     xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
     xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
@@ -118,6 +120,8 @@ static void xlnx_ep108_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx ZynqMP EP108 board";
     mc->init = xlnx_ep108_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("xlnx-ep108", xlnx_ep108_machine_init)
@@ -126,6 +130,8 @@ static void xlnx_zcu102_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx ZynqMP ZCU102 board";
     mc->init = xlnx_ep108_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("xlnx-zcu102", xlnx_zcu102_machine_init)
-- 
2.7.4

[Qemu-devel] [PATCH 3/6] hw: Default -drive to if=none instead of ide when ide cannot work

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types implicitly default to if=ide that way, even though
they don't actually have an IDE controller.  This makes no sense.

Change the implicit default to if=none.  Affected machines:

* all targets: none
* aarch64/arm: akita ast2500 canon cheetah collie connex imx25
  integratorcp kzm lm3s6965evb lm3s811evb mainstone musicpal n800 n810
  netduino2 nuri palmetto realview romulus sabrelite smdkc210 sx1 sx1
  verdex z2
* cris: axis-dev88
* i386/x86_64: xenpv
* lm32: lm32-evr lm32-uclinux milkymist
* m68k: an5206 dummy mcf5208evb
* microblaze/microblazeel: petalogix-ml605 petalogix-s3adsp1800
* mips/mips64/mips64el/mipsel: mipssim
* moxie: moxiesim
* or32: or32-sim
* ppc/ppc64/ppcemb: bamboo ref405ep taihu virtex-ml507
* ppc/ppc64: mpc8544ds ppce500
* sh4/sh4eb: shix
* sparc: leon3_generic
* sparc64: niagara
* tricore: tricore_testboard
* unicore32: puv3
* xtensa/xtensaeb: kc705 lx200 lx60 ml605 sim

None of these machines have an IDE controller, let alone code to
honor if=ide.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: xen-devel@lists.xensource.com
Cc: Michael Walle <michael@walle.cc>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Anthony Green <green@moxielogic.com>
Cc: Jia Liu <proljc@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Fabien Chouteau <chouteau@adacore.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/sysemu/blockdev.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 16432f3..351a039 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -19,12 +19,11 @@ void blockdev_auto_del(BlockBackend *blk);
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
     /*
-     * IF_IDE must be zero, because we want MachineClass member
-     * block_default_type to default-initialize to IF_IDE
+     * IF_NONE must be zero, because we want MachineClass member
+     * block_default_type to default-initialize to IF_NONE
      */
-    IF_IDE = 0,
-    IF_NONE,
-    IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_NONE = 0,
+    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
 } BlockInterfaceType;
 
-- 
2.7.4

[PATCH 3/6] hw: Default -drive to if=none instead of ide when ide cannot work

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types implicitly default to if=ide that way, even though
they don't actually have an IDE controller.  This makes no sense.

Change the implicit default to if=none.  Affected machines:

* all targets: none
* aarch64/arm: akita ast2500 canon cheetah collie connex imx25
  integratorcp kzm lm3s6965evb lm3s811evb mainstone musicpal n800 n810
  netduino2 nuri palmetto realview romulus sabrelite smdkc210 sx1 sx1
  verdex z2
* cris: axis-dev88
* i386/x86_64: xenpv
* lm32: lm32-evr lm32-uclinux milkymist
* m68k: an5206 dummy mcf5208evb
* microblaze/microblazeel: petalogix-ml605 petalogix-s3adsp1800
* mips/mips64/mips64el/mipsel: mipssim
* moxie: moxiesim
* or32: or32-sim
* ppc/ppc64/ppcemb: bamboo ref405ep taihu virtex-ml507
* ppc/ppc64: mpc8544ds ppce500
* sh4/sh4eb: shix
* sparc: leon3_generic
* sparc64: niagara
* tricore: tricore_testboard
* unicore32: puv3
* xtensa/xtensaeb: kc705 lx200 lx60 ml605 sim

None of these machines have an IDE controller, let alone code to
honor if=ide.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: xen-devel@lists.xensource.com
Cc: Michael Walle <michael@walle.cc>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Anthony Green <green@moxielogic.com>
Cc: Jia Liu <proljc@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Fabien Chouteau <chouteau@adacore.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/sysemu/blockdev.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 16432f3..351a039 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -19,12 +19,11 @@ void blockdev_auto_del(BlockBackend *blk);
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
     /*
-     * IF_IDE must be zero, because we want MachineClass member
-     * block_default_type to default-initialize to IF_IDE
+     * IF_NONE must be zero, because we want MachineClass member
+     * block_default_type to default-initialize to IF_NONE
      */
-    IF_IDE = 0,
-    IF_NONE,
-    IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_NONE = 0,
+    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
 } BlockInterfaceType;
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Qemu-devel] [PATCH 4/6] hw: Default -drive to if=none instead of scsi when scsi cannot work

[Markus Armbruster]

Block backends defined with -drive if=scsi are meant to be picked up
by machine initialization code: a suitable frontend gets created and
wired up automatically.

if=scsi drives not picked up that way can still be used with -device
as if they had if=none, but that's unclean and best avoided.  Unused
ones produce an "Orphaned drive without device" warning.

A few machine types default to if=scsi, even though they don't
actually have a SCSI HBA.  This makes no sense.  Change their default
to if=none.  Affected machines:

* aarch64/arm: realview-pbx-a9 vexpress-a9 vexpress-a15 xilinx-zynq-a9

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alistair Francis <alistair.francis@xilinx.com>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/realview.c    | 1 -
 hw/arm/vexpress.c    | 1 -
 hw/arm/xilinx_zynq.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 8eafcca..8c11c7a 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -443,7 +443,6 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
     mc->init = realview_pbx_a9_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
 }
 
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 58760f4..5756470 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -751,7 +751,6 @@ static void vexpress_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM Versatile Express";
     mc->init = vexpress_common_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
 }
 
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 7dac20d..3985356 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -323,7 +323,6 @@ static void zynq_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9";
     mc->init = zynq_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 1;
     mc->no_sdcard = 1;
 }
-- 
2.7.4

[Qemu-devel] [PATCH 5/6] hw/arm/highbank: Default -drive to if=ide instead of if=scsi

[Markus Armbruster]

These machines have no onboard SCSI HBA, and no way to plug one.
-drive if=scsi therefore cannot work.  They do have an onboard IDE
controller (sysbus-ahci), but fail to honor if=ide.

Change their default to if=ide, and add a TODO comment on what needs
to be done to actually honor -drive if=ide.

Cc: Rob Herring <robh@kernel.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/highbank.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 80e5fd4..0a4508c 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -363,6 +363,8 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), 2, pic[82]);
     }
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     highbank_binfo.ram_size = ram_size;
     highbank_binfo.kernel_filename = kernel_filename;
     highbank_binfo.kernel_cmdline = kernel_cmdline;
@@ -405,7 +407,8 @@ static void highbank_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Calxeda Highbank (ECX-1000)";
     mc->init = highbank_init;
-    mc->block_default_type = IF_SCSI;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
     mc->max_cpus = 4;
 }
 
@@ -421,7 +424,8 @@ static void midway_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Calxeda Midway (ECX-2000)";
     mc->init = midway_init;
-    mc->block_default_type = IF_SCSI;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
     mc->max_cpus = 4;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

The PC machines (pc-q35-* pc-i440fx-* pc-* isapc xenfv) automatically
create lsi53c895a SCSI HBAs and SCSI devices to honor -drive if=scsi.
For giggles, try -drive if=scsi,bus=25,media=cdrom --- this makes QEMU
create 25 of them.

The lsi53c895a is thoroughly obsolete (PCI Ultra2 SCSI, ca. 2000), and
currently has no maintainer in QEMU.  megasas is a better choice,
except for old OSes, which lack drivers. virtio-scsi is a much better
choice when you have a driver, but only (newish) Linux comes with one
in the box.  There is no good default that works for all guests.

Instead of supplying the not-so-good default just because we've always
supplied it, make the user pick, by not creating any SCSI HBAs for
-drive if=scsi.  The user now needs to create the SCSI HBA himself
with -device.  The SCSI devices will still be created automatically.

For backward compatibility, keep the traditional behavior for old
machine types.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc_piix.c    | 15 ++++++++++++++-
 hw/i386/pc_q35.c     |  7 ++++++-
 include/hw/i386/pc.h |  3 +++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9f102aa..35e92e4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -292,7 +292,7 @@ static void pc_init1(MachineState *machine,
                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
     }
 
-    if (pcmc->pci_enabled) {
+    if (pcmc->pci_enabled && pcmc->legacy_lsi_hba_auto_create) {
         pc_pci_device_init(pci_bus);
     }
 
@@ -449,9 +449,12 @@ DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
 
 static void pc_i440fx_2_8_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_9_machine_options(m);
     m->is_default = 0;
     m->alias = NULL;
+    pcmc->legacy_lsi_hba_auto_create = true;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
 }
 
@@ -472,6 +475,7 @@ DEFINE_I440FX_MACHINE(v2_7, "pc-i440fx-2.7", NULL,
 static void pc_i440fx_2_6_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_7_machine_options(m);
     pcmc->legacy_cpu_hotplug = true;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
@@ -484,6 +488,7 @@ DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
 static void pc_i440fx_2_5_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_6_machine_options(m);
     pcmc->save_tsc_khz = false;
     m->legacy_fw_cfg_order = 1;
@@ -497,6 +502,7 @@ DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
 static void pc_i440fx_2_4_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_5_machine_options(m);
     m->hw_version = "2.4.0";
     pcmc->broken_reserved_end = true;
@@ -521,6 +527,7 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3,
 static void pc_i440fx_2_2_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_3_machine_options(m);
     m->hw_version = "2.2.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
@@ -534,6 +541,7 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
 static void pc_i440fx_2_1_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_2_machine_options(m);
     m->hw_version = "2.1.0";
     m->default_display = NULL;
@@ -550,6 +558,7 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
 static void pc_i440fx_2_0_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_1_machine_options(m);
     m->hw_version = "2.0.0";
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
@@ -582,6 +591,7 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
 static void pc_i440fx_1_7_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_2_0_machine_options(m);
     m->hw_version = "1.7.0";
     m->default_machine_opts = NULL;
@@ -599,6 +609,7 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
 static void pc_i440fx_1_6_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_1_7_machine_options(m);
     m->hw_version = "1.6.0";
     m->rom_file_has_mr = false;
@@ -858,6 +869,7 @@ DEFINE_I440FX_MACHINE(v0_14, "pc-0.14", pc_compat_1_2,
 static void pc_i440fx_0_13_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_i440fx_0_14_machine_options(m);
     m->hw_version = "0.13";
     SET_MACHINE_COMPAT(m, PC_COMPAT_0_13);
@@ -1084,6 +1096,7 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
 static void isapc_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     m->desc = "ISA-only PC";
     m->max_cpus = 1;
     m->option_rom_has_mr = true;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dd792a8..571565d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -266,7 +266,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);
-    if (pcmc->pci_enabled) {
+    if (pcmc->pci_enabled && pcmc->legacy_lsi_hba_auto_create) {
         pc_pci_device_init(host_bus);
     }
 
@@ -312,8 +312,11 @@ DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
 
 static void pc_q35_2_8_machine_options(MachineClass *m)
 {
+    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_q35_2_9_machine_options(m);
     m->alias = NULL;
+    pcmc->legacy_lsi_hba_auto_create = true;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
 }
 
@@ -333,6 +336,7 @@ DEFINE_Q35_MACHINE(v2_7, "pc-q35-2.7", NULL,
 static void pc_q35_2_6_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_q35_2_7_machine_options(m);
     pcmc->legacy_cpu_hotplug = true;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
@@ -356,6 +360,7 @@ DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
 static void pc_q35_2_4_machine_options(MachineClass *m)
 {
     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
     pc_q35_2_5_machine_options(m);
     m->hw_version = "2.4.0";
     pcmc->broken_reserved_end = true;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 230e9e7..991b87b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -152,6 +152,9 @@ struct PCMachineClass {
     bool save_tsc_khz;
     /* generate legacy CPU hotplug AML */
     bool legacy_cpu_hotplug;
+
+    /* Whether to auto-create lsi53c895a SCSI HBAs */
+    bool legacy_lsi_hba_auto_create;
 };
 
 #define TYPE_PC_MACHINE "generic-pc-machine"
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/6] hw: Default -drive to if=none instead of ide when ide cannot work

[Artyom Tarasenko]

On Mon, Jan 23, 2017 at 10:48 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Block backends defined with -drive if=ide are meant to be picked up by
> machine initialization code: a suitable frontend gets created and
> wired up automatically.
>
> if=ide drives not picked up that way can still be used with -device as
> if they had if=none, but that's unclean and best avoided.  Unused ones
> produce an "Orphaned drive without device" warning.
>
> -drive parameter "if" is optional, and the default depends on the
> machine type.  If a machine type doesn't specify a default, the
> default is "ide".
>
> Many machine types implicitly default to if=ide that way, even though
> they don't actually have an IDE controller.  This makes no sense.
>
> Change the implicit default to if=none.  Affected machines:
>
> * all targets: none
> * aarch64/arm: akita ast2500 canon cheetah collie connex imx25
>   integratorcp kzm lm3s6965evb lm3s811evb mainstone musicpal n800 n810
>   netduino2 nuri palmetto realview romulus sabrelite smdkc210 sx1 sx1
>   verdex z2
> * cris: axis-dev88
> * i386/x86_64: xenpv
> * lm32: lm32-evr lm32-uclinux milkymist
> * m68k: an5206 dummy mcf5208evb
> * microblaze/microblazeel: petalogix-ml605 petalogix-s3adsp1800
> * mips/mips64/mips64el/mipsel: mipssim
> * moxie: moxiesim
> * or32: or32-sim
> * ppc/ppc64/ppcemb: bamboo ref405ep taihu virtex-ml507
> * ppc/ppc64: mpc8544ds ppce500
> * sh4/sh4eb: shix
> * sparc: leon3_generic
> * sparc64: niagara

The sparc64: niagara part,

Acked-By: Artyom Tarasenko <atar4qemu@gmail.com>

> * tricore: tricore_testboard
> * unicore32: puv3
> * xtensa/xtensaeb: kc705 lx200 lx60 ml605 sim
>
> None of these machines have an IDE controller, let alone code to
> honor if=ide.
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: xen-devel@lists.xensource.com
> Cc: Michael Walle <michael@walle.cc>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Anthony Green <green@moxielogic.com>
> Cc: Jia Liu <proljc@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-ppc@nongnu.org
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Fabien Chouteau <chouteau@adacore.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/sysemu/blockdev.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 16432f3..351a039 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -19,12 +19,11 @@ void blockdev_auto_del(BlockBackend *blk);
>  typedef enum {
>      IF_DEFAULT = -1,            /* for use with drive_add() only */
>      /*
> -     * IF_IDE must be zero, because we want MachineClass member
> -     * block_default_type to default-initialize to IF_IDE
> +     * IF_NONE must be zero, because we want MachineClass member
> +     * block_default_type to default-initialize to IF_NONE
>       */
> -    IF_IDE = 0,
> -    IF_NONE,
> -    IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> +    IF_NONE = 0,
> +    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>      IF_COUNT
>  } BlockInterfaceType;
>
> --
> 2.7.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Paolo Bonzini]

On 23/01/2017 10:48, Markus Armbruster wrote:
> The PC machines (pc-q35-* pc-i440fx-* pc-* isapc xenfv) automatically
> create lsi53c895a SCSI HBAs and SCSI devices to honor -drive if=scsi.
> For giggles, try -drive if=scsi,bus=25,media=cdrom --- this makes QEMU
> create 25 of them.
> 
> The lsi53c895a is thoroughly obsolete (PCI Ultra2 SCSI, ca. 2000), and
> currently has no maintainer in QEMU.  megasas is a better choice,
> except for old OSes, which lack drivers. virtio-scsi is a much better
> choice when you have a driver, but only (newish) Linux comes with one
> in the box.  There is no good default that works for all guests.
> 
> Instead of supplying the not-so-good default just because we've always
> supplied it, make the user pick, by not creating any SCSI HBAs for
> -drive if=scsi.  The user now needs to create the SCSI HBA himself
> with -device.  The SCSI devices will still be created automatically.
> 
> For backward compatibility, keep the traditional behavior for old
> machine types.

It would print an "orphaned device" message, right?  Could we change
those messages to errors and then drop PC if=scsi support altogether?

Paolo

> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/pc_piix.c    | 15 ++++++++++++++-
>  hw/i386/pc_q35.c     |  7 ++++++-
>  include/hw/i386/pc.h |  3 +++
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 9f102aa..35e92e4 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -292,7 +292,7 @@ static void pc_init1(MachineState *machine,
>                                   PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);
>      }
>  
> -    if (pcmc->pci_enabled) {
> +    if (pcmc->pci_enabled && pcmc->legacy_lsi_hba_auto_create) {
>          pc_pci_device_init(pci_bus);
>      }
>  
> @@ -449,9 +449,12 @@ DEFINE_I440FX_MACHINE(v2_9, "pc-i440fx-2.9", NULL,
>  
>  static void pc_i440fx_2_8_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_9_machine_options(m);
>      m->is_default = 0;
>      m->alias = NULL;
> +    pcmc->legacy_lsi_hba_auto_create = true;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
>  }
>  
> @@ -472,6 +475,7 @@ DEFINE_I440FX_MACHINE(v2_7, "pc-i440fx-2.7", NULL,
>  static void pc_i440fx_2_6_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_7_machine_options(m);
>      pcmc->legacy_cpu_hotplug = true;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
> @@ -484,6 +488,7 @@ DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL,
>  static void pc_i440fx_2_5_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_6_machine_options(m);
>      pcmc->save_tsc_khz = false;
>      m->legacy_fw_cfg_order = 1;
> @@ -497,6 +502,7 @@ DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
>  static void pc_i440fx_2_4_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_5_machine_options(m);
>      m->hw_version = "2.4.0";
>      pcmc->broken_reserved_end = true;
> @@ -521,6 +527,7 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3,
>  static void pc_i440fx_2_2_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_3_machine_options(m);
>      m->hw_version = "2.2.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
> @@ -534,6 +541,7 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
>  static void pc_i440fx_2_1_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_2_machine_options(m);
>      m->hw_version = "2.1.0";
>      m->default_display = NULL;
> @@ -550,6 +558,7 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
>  static void pc_i440fx_2_0_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_1_machine_options(m);
>      m->hw_version = "2.0.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
> @@ -582,6 +591,7 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
>  static void pc_i440fx_1_7_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_2_0_machine_options(m);
>      m->hw_version = "1.7.0";
>      m->default_machine_opts = NULL;
> @@ -599,6 +609,7 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
>  static void pc_i440fx_1_6_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_1_7_machine_options(m);
>      m->hw_version = "1.6.0";
>      m->rom_file_has_mr = false;
> @@ -858,6 +869,7 @@ DEFINE_I440FX_MACHINE(v0_14, "pc-0.14", pc_compat_1_2,
>  static void pc_i440fx_0_13_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_i440fx_0_14_machine_options(m);
>      m->hw_version = "0.13";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_0_13);
> @@ -1084,6 +1096,7 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
>  static void isapc_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      m->desc = "ISA-only PC";
>      m->max_cpus = 1;
>      m->option_rom_has_mr = true;
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index dd792a8..571565d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -266,7 +266,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);
> -    if (pcmc->pci_enabled) {
> +    if (pcmc->pci_enabled && pcmc->legacy_lsi_hba_auto_create) {
>          pc_pci_device_init(host_bus);
>      }
>  
> @@ -312,8 +312,11 @@ DEFINE_Q35_MACHINE(v2_9, "pc-q35-2.9", NULL,
>  
>  static void pc_q35_2_8_machine_options(MachineClass *m)
>  {
> +    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_2_9_machine_options(m);
>      m->alias = NULL;
> +    pcmc->legacy_lsi_hba_auto_create = true;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_8);
>  }
>  
> @@ -333,6 +336,7 @@ DEFINE_Q35_MACHINE(v2_7, "pc-q35-2.7", NULL,
>  static void pc_q35_2_6_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_2_7_machine_options(m);
>      pcmc->legacy_cpu_hotplug = true;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_6);
> @@ -356,6 +360,7 @@ DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
>  static void pc_q35_2_4_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> +
>      pc_q35_2_5_machine_options(m);
>      m->hw_version = "2.4.0";
>      pcmc->broken_reserved_end = true;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 230e9e7..991b87b 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -152,6 +152,9 @@ struct PCMachineClass {
>      bool save_tsc_khz;
>      /* generate legacy CPU hotplug AML */
>      bool legacy_cpu_hotplug;
> +
> +    /* Whether to auto-create lsi53c895a SCSI HBAs */
> +    bool legacy_lsi_hba_auto_create;
>  };
>  
>  #define TYPE_PC_MACHINE "generic-pc-machine"
> 

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 23/01/2017 10:48, Markus Armbruster wrote:
>> The PC machines (pc-q35-* pc-i440fx-* pc-* isapc xenfv) automatically
>> create lsi53c895a SCSI HBAs and SCSI devices to honor -drive if=scsi.
>> For giggles, try -drive if=scsi,bus=25,media=cdrom --- this makes QEMU
>> create 25 of them.
>> 
>> The lsi53c895a is thoroughly obsolete (PCI Ultra2 SCSI, ca. 2000), and
>> currently has no maintainer in QEMU.  megasas is a better choice,
>> except for old OSes, which lack drivers. virtio-scsi is a much better
>> choice when you have a driver, but only (newish) Linux comes with one
>> in the box.  There is no good default that works for all guests.
>> 
>> Instead of supplying the not-so-good default just because we've always
>> supplied it, make the user pick, by not creating any SCSI HBAs for
>> -drive if=scsi.  The user now needs to create the SCSI HBA himself
>> with -device.  The SCSI devices will still be created automatically.
>> 
>> For backward compatibility, keep the traditional behavior for old
>> machine types.
>
> It would print an "orphaned device" message, right?

Yes.

Before this patch, you can't get an "orhpaned" warning for if=scsi with
a PC machine.

After this patch, you get one for every if=scsi with new PC machine
types.

>                                                      Could we change
> those messages to errors

Fine with me, but when it comes to arguing for backward compatibility of
our byzantine command line, I'm kind of like a lethargic public defender
with an overly deep relationship to Bourbon.  "Your honor, sure capital
punishment is called for?  Yes?  Okay then."

I vaguely recall discussing the topic with Peter (cc'ed).  If memory
serves, one concern was breaking usage of -device with -drive lacking
if=...  Works fine (no warning) with machines that don't pick up drives
with their default block interface type, i.e. most of them.  But PATCH 3
changes their default to if=none, so that usage wouldn't actually break.
What would break is -device with -drive if=T, where T is not none and
not picked up by the board.  Such usage is certainly questionable[*],
but it's questionable enough for us to break it?

>                          and then drop PC if=scsi support altogether?

Different backward compatibility question: here we break usage of
if=scsi with PC machine types.  Legacy way to do things, but it's
documented in qemu.1.  Are we happy to break it?


[*] Here's my review of a prior attempt to outlaw it:
http://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03334.html

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Paolo Bonzini]

On 23/01/2017 20:16, Markus Armbruster wrote:
>>                                                      Could we change
>> those messages to errors
> 
> Fine with me, but when it comes to arguing for backward compatibility of
> our byzantine command line, I'm kind of like a lethargic public defender
> with an overly deep relationship to Bourbon.  "Your honor, sure capital
> punishment is called for?  Yes?  Okay then."
> 
> I vaguely recall discussing the topic with Peter (cc'ed).  If memory
> serves, one concern was breaking usage of -device with -drive lacking
> if=...  Works fine (no warning) with machines that don't pick up drives
> with their default block interface type, i.e. most of them.  But PATCH 3
> changes their default to if=none, so that usage wouldn't actually break.

I think that tips the scale in favor of having errors.

> What would break is -device with -drive if=T, where T is not none and
> not picked up by the board.  Such usage is certainly questionable[*],
> but it's questionable enough for us to break it?
> 
>>                          and then drop PC if=scsi support altogether?
> 
> Different backward compatibility question: here we break usage of
> if=scsi with PC machine types.  Legacy way to do things, but it's
> documented in qemu.1.  Are we happy to break it?

That usage is wrong after this patch, since it mentions
qemu-system-i386.  So it's documented, but almost useless and the
example is not exactly correct.  Let's deprecate it in 2.9 and remove in
2.10.

Paolo

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

Cc'ing in sPAPR maintainers...

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 23/01/2017 20:16, Markus Armbruster wrote:
>>>                                                      Could we change
>>> those messages to errors
>> 
>> Fine with me, but when it comes to arguing for backward compatibility of
>> our byzantine command line, I'm kind of like a lethargic public defender
>> with an overly deep relationship to Bourbon.  "Your honor, sure capital
>> punishment is called for?  Yes?  Okay then."
>> 
>> I vaguely recall discussing the topic with Peter (cc'ed).  If memory
>> serves, one concern was breaking usage of -device with -drive lacking
>> if=...  Works fine (no warning) with machines that don't pick up drives
>> with their default block interface type, i.e. most of them.  But PATCH 3
>> changes their default to if=none, so that usage wouldn't actually break.
>
> I think that tips the scale in favor of having errors.

I can add a patch to make it an error when I respin.

>> What would break is -device with -drive if=T, where T is not none and
>> not picked up by the board.  Such usage is certainly questionable[*],
>> but it's questionable enough for us to break it?
>> 
>>>                          and then drop PC if=scsi support altogether?
>> 
>> Different backward compatibility question: here we break usage of
>> if=scsi with PC machine types.  Legacy way to do things, but it's
>> documented in qemu.1.  Are we happy to break it?
>
> That usage is wrong after this patch, since it mentions
> qemu-system-i386.

You're right, I better fix that.

>                    So it's documented, but almost useless and the
> example is not exactly correct.  Let's deprecate it in 2.9 and remove in
> 2.10.

Let me spell things out a bit more, to make sure we all agree on what
exactly we want to deprecate.

After this series, -drive if=scsi works for the following machine types:

* magnum pica61 LX SPARCClassic SPARCbook SS-10 SS-20 SS-4 SS-5 SS-600MP
  Voyager

  The machine has an onboard SCSI HBA, which adopts the drives with
  bus=0..  Drives with non-zero bus numbers stay orphaned.  This is
  exactly how other interface types work.

  Except when additional HBAs get cold-plugged somehow, non-zero bus
  numbers can work; see below.

* realview-eb realview-eb-mpcore versatileab versatilepb

  These create N lsi53c895a SCSI HBAs, where N is the largest value of
  bus.  If N is too large, machine initialization fails with a "no
  slot/function available for lsi53c895a, all in use" error.

  This is just like the PC machine types work before this patch.

* pseries-*

  Likewise, except create spapr-vscsi SCSI HBAs.  Large N make machine
  initialization s-l-o-w.  I tried to find out whether and how it fails
  when N is too large, but I lost patience.

Additionally, -drive if=scsi works when you cold-plug certain SCSI HBAs,
independent of machine type.  The HBAs get assigned bus numbers in order
of creation, and adopt the drives with their bus number.  Drives with
bus numbers not so assigned stay orphaned.

* SCSI HBAs supporting if=scsi: am53c974 dc390 esp lsi53c810 lsi53c895a
  megasas megasas-gen2 mptsas1068 spapr-vscsi virtio-scsi-device

* Not supporting it: pvscsi usb-storage usb-bot usb-uas

So, what do we want to deprecate?

I think the onboard SCSI HBA adopting if=scsi drives should stay,
because it matches what we do for other interface types.  Unless we
wanted to deprecate interface types other than none entirely.

What about realview and versatile auto-creating lsi53c895a?

What about pseries auto-creating spapr-vscsi?

What about cold-plug of HBAs auto-creating SCSI devices?  You proposed
deprecating it for PC types, but it's currently independent of the
machine type.  Deprecate it for all types?  If not, add a flag to
MachineClass so we can deprecate it just for PC types?

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Paolo Bonzini]

On 24/01/2017 13:56, Markus Armbruster wrote:
> So, what do we want to deprecate?
> 
> I think the onboard SCSI HBA adopting if=scsi drives should stay,
> because it matches what we do for other interface types.  Unless we
> wanted to deprecate interface types other than none entirely.

Yes, of course.

> What about realview and versatile auto-creating lsi53c895a?

That should go.

> What about pseries auto-creating spapr-vscsi?

That should stay I think.  The discriminant being that spapr-vscsi
devices actually are likely to have drivers in the guests.

> What about cold-plug of HBAs auto-creating SCSI devices?  You proposed
> deprecating it for PC types, but it's currently independent of the
> machine type.  Deprecate it for all types?  If not, add a flag to
> MachineClass so we can deprecate it just for PC types?

I need --verbose for this, sorry.

Paolo

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/01/2017 13:56, Markus Armbruster wrote:
>> So, what do we want to deprecate?
>> 
>> I think the onboard SCSI HBA adopting if=scsi drives should stay,
>> because it matches what we do for other interface types.  Unless we
>> wanted to deprecate interface types other than none entirely.
>
> Yes, of course.
>
>> What about realview and versatile auto-creating lsi53c895a?
>
> That should go.

Peter, do you agree?

>> What about pseries auto-creating spapr-vscsi?
>
> That should stay I think.  The discriminant being that spapr-vscsi
> devices actually are likely to have drivers in the guests.

Okay.

>> What about cold-plug of HBAs auto-creating SCSI devices?  You proposed
>> deprecating it for PC types, but it's currently independent of the
>> machine type.  Deprecate it for all types?  If not, add a flag to
>> MachineClass so we can deprecate it just for PC types?
>
> I need --verbose for this, sorry.

No problem!  Let me explain how SCSI auto-creation works, and how it
differs from the other interface types.  Actually, let me start with the
other ones.

When a machine supports some if=T, T other than scsi or none, its
initialization code retrieves the drives of interface type T with
supported bus and unit numbers, creates suitable frontends, and wires
them up.  We commonly don't bother to check whether there are drives
with unsupported bus and unit numbers.

Example: within terrier_init(), spitz_microdrive_attach() runs and calls
drive_get(IF_IDE, 0, 0).  If it gets something back, it creates a
dscm1xxxx device model.

Example: pc_q35_init() calls ide_drive_get() to retrieve the IF_IDE
drives with bus=0..5, unit=0.  It creates ide-cd or ide-hd device models
for the ones it gets back.

Example: within machvirt_init(), create_one_flash() runs two times and
calls drive_get_next(IF_PFLASH) to retrieve bus=0, unit=0..1.  It
creates cfi.pflash01 device models unconditionally, and backs them with
the drives it gets back, if any.  I hate the drive_get_next() interface,
because it uses execution order to encode unit numbers.

if=scsi works differently.

SCSI HBA device models commonly call scsi_bus_legacy_handle_cmdline()
during realize() for their SCSI bus(es).  A SCSI bus gets assigned a bus
number on creation, counting up from zero.  The SCSI HBA device model
declares the maximum unit number it supports (SCSIBusInfo member
max_target).  scsi_bus_legacy_handle_cmdline() calls drive_get(IF_SCSI,
bus->busnr, unit) to retrieve the IF_SCSI drives, and creates
scsi-generic or scsi-disk device models.

When a machine has an onboard SCSI HBA whose realize() calls
scsi_bus_legacy_handle_cmdline(), then machine initialization code
doesn't have to do anything to support if=scsi.

Example: mips_jazz_init() simply calls esp_init(), which then adopts the
if=scsi,bus=0,unit=0..7.  Note that blockdev.c rejects unit>7.

Example: sun4m_hw_init() does the same, but also checks for and rejects
drives with bus>0.  This check becomes redundant when we make orphaned
drives an error.

A few machines don't have onboard SCSI HBA, but their initialization
code creates N default ones, where N is the largest bus number of any
if=scsi drive.  Again, nothing else needs to be done to create the
actual SCSI devices.

Example: before my patch, PC machine initialization calls
pc_pci_device_init(), which creates lsi53c895a device models in a loop.

If a machine doesn't create default SCSI HBAs, the user can do it
himself with -device, and as long as the HBA's realize() calls
scsi_bus_legacy_handle_cmdline(), if=scsi works.

Example: after my patch, PC machine initialization doesn't create SCSI
HBAs.  Options "-drive media=cdrom,if=scsi,bus=1 -device
megasas,id=scsi0 -device megasas,id=scsi1" create two megasas device
models, and one scsi-disk device model plugged into the second one.

My question is: do we want to deprecate "-device of a SCSI HBA
auto-creates SCSI devices for the HBA's bus number(s), with unit numbers
up to max_target"?

For all machine types, or just for PC?

If for all machine types, we need work a little harder for if=scsi in
machine initialization, exactly like we do for the other interface
types.

Clear as mud now?

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Peter Maydell]

On 24 January 2017 at 17:20, Markus Armbruster <armbru@redhat.com> wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> On 24/01/2017 13:56, Markus Armbruster wrote:
>>> So, what do we want to deprecate?
>>>
>>> I think the onboard SCSI HBA adopting if=scsi drives should stay,
>>> because it matches what we do for other interface types.  Unless we
>>> wanted to deprecate interface types other than none entirely.
>>
>> Yes, of course.
>>
>>> What about realview and versatile auto-creating lsi53c895a?
>>
>> That should go.
>
> Peter, do you agree?

I'm not sure what you're suggesting deprecation of -- will
command lines like
 "qemu-system-arm -M versatilepb -hda somedisk.qcow2"
still work (ie auto-create the LSI scsi adaptor and plug
the disk into it) ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Paolo Bonzini]

On 24/01/2017 18:24, Peter Maydell wrote:
> On 24 January 2017 at 17:20, Markus Armbruster <armbru@redhat.com> wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> On 24/01/2017 13:56, Markus Armbruster wrote:
>>>> So, what do we want to deprecate?
>>>>
>>>> I think the onboard SCSI HBA adopting if=scsi drives should stay,
>>>> because it matches what we do for other interface types.  Unless we
>>>> wanted to deprecate interface types other than none entirely.
>>>
>>> Yes, of course.
>>>
>>>> What about realview and versatile auto-creating lsi53c895a?
>>>
>>> That should go.
>>
>> Peter, do you agree?
> 
> I'm not sure what you're suggesting deprecation of -- will
> command lines like
>  "qemu-system-arm -M versatilepb -hda somedisk.qcow2"
> still work (ie auto-create the LSI scsi adaptor and plug
> the disk into it) ?

No, I was confusing with the boards that Markus is fixing in patch 4.
But is really lsi53c895a the best we can provide for
versatilepb/versatileab/realview-eb/realview-eb-mpcore? Could virtio-pci
work well too?...

Paolo

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Peter Maydell]

On 24 January 2017 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> But is really lsi53c895a the best we can provide for
> versatilepb/versatileab/realview-eb/realview-eb-mpcore? Could virtio-pci
> work well too?...

I assume that people will still have legacy kernel images
which include the LSI drivers but not the virtio-pci drivers.
versatilepb in particular is pretty widely used as an example
ARM QEMU system (it was the sanest one before virt came along),
so there's a good chance of breaking a bunch of tutorials
and old images if you stop creating LSI devices.

More generally, I'd rather not spend any time at all messing
about with these old board models if they're not actually broken.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 24 January 2017 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> But is really lsi53c895a the best we can provide for
>> versatilepb/versatileab/realview-eb/realview-eb-mpcore? Could virtio-pci
>> work well too?...
>
> I assume that people will still have legacy kernel images
> which include the LSI drivers but not the virtio-pci drivers.
> versatilepb in particular is pretty widely used as an example
> ARM QEMU system (it was the sanest one before virt came along),
> so there's a good chance of breaking a bunch of tutorials
> and old images if you stop creating LSI devices.
>
> More generally, I'd rather not spend any time at all messing
> about with these old board models if they're not actually broken.

Okay, I'll keep adding lsi devices for if=scsi drives with these four
machine types.

Re: [Qemu-devel] [PATCH 6/6] hw/i386/i386: Stop auto-creating lsi53c895a SCSI HBAs

[Markus Armbruster]

David, Alex, got a sPAPR question for you.

Markus Armbruster <armbru@redhat.com> writes:

> Cc'ing in sPAPR maintainers...
[...]
> Let me spell things out a bit more, to make sure we all agree on what
> exactly we want to deprecate.
>
> After this series, -drive if=scsi works for the following machine types:
>
> * magnum pica61 LX SPARCClassic SPARCbook SS-10 SS-20 SS-4 SS-5 SS-600MP
>   Voyager
>
>   The machine has an onboard SCSI HBA, which adopts the drives with
>   bus=0..  Drives with non-zero bus numbers stay orphaned.  This is
>   exactly how other interface types work.
>
>   Except when additional HBAs get cold-plugged somehow, non-zero bus
>   numbers can work; see below.
>
> * realview-eb realview-eb-mpcore versatileab versatilepb
>
>   These create N lsi53c895a SCSI HBAs, where N is the largest value of
>   bus.  If N is too large, machine initialization fails with a "no
>   slot/function available for lsi53c895a, all in use" error.
>
>   This is just like the PC machine types work before this patch.
>
> * pseries-*
>
>   Likewise, except create spapr-vscsi SCSI HBAs.  Large N make machine
>   initialization s-l-o-w.  I tried to find out whether and how it fails
>   when N is too large, but I lost patience.

Totally unscientific experiment: run "qemu-system-ppc64 -S -display none
-monitor stdio -M pseries,accel=qtest -drive if=scsi,media=cdrom,bus=N"
for various N and check SZ in ps, with "ps -C qemu-system-ppc64 -O sz".
Results:

      N      SZ
      0  339413
     10  353182
     20  375342
     30  405857
     40  444716
     50  491988
     60  547582
     70  611589
     80  683916
     90  764658
    100  853743

Definitely super-linear.  Is this expected?

No wonder my previous bus=999 experiement thrashed my machine;
extrapolating from the above I guesstimate SZ in the order of 50 million
pages.

Of course, asking for 1000 spapr-vscsi devices is silly, but trashing is
not a nice way to say "don't be silly".  Should we reject bus=N when N
exceeds a certain limit?  Care to suggest a limit?

[...]