hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices
PATCH 1/1 hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices
The global hack for creating SCSI devices has recently been removed, but this apparently broke SCSI devices on some boards that were not ready for this change yet. For the 40p machine you now get:
$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso qemu-system-ppc64: -cdrom x.iso: machine type does not support if=scsi,bus=0,unit=2
Fix it by providing a lsi53c810_create() function that takes care of calling scsi_bus_legacy_handle_cmdline() after creating the corresponding SCSI controller.
--- hw/ppc/prep.c | 2 +- hw/scsi/lsi53c895a.c | 7 +++++++ include/hw/pci/pci.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)
hw/ppc/prep.c +1 -1
@@ -788,7 +788,7 @@ static void ibm_40p_init(MachineState *machine)qdev_prop_set_uint32 (dev,"equipment" ,0xc0 );qdev_init_nofail (dev); -pci_create_simple (pci_bus,PCI_DEVFN (1 ,0 ),"lsi53c810" ); +lsi53c810_create (pci_bus,PCI_DEVFN (1 ,0 ));/* XXX: s3-trio at PCI_DEVFN(2, 0) */ pci_vga_init (pci_bus);
hw/scsi/lsi53c895a.c +7 -0
@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)scsi_bus_legacy_handle_cmdline (&s->bus); } + +void lsi53c810_create (PCIBus *bus,int devfn) +{ +LSIState *s =LSI53C895A (pci_create_simple (bus, devfn,"lsi53c810" )); + +scsi_bus_legacy_handle_cmdline (&s->bus); +}
include/hw/pci/pci.h +1 -0
@@ -708,6 +708,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);PCIDevice *pci_create_simple (PCIBus *bus,int devfn,const char *name);void lsi53c895a_create (PCIBus *bus); +void lsi53c810_create (PCIBus *bus,int devfn);qemu_irq pci_allocate_irq (PCIDevice *pci_dev);void pci_set_irq (PCIDevice *pci_dev,int level);
Signature · 1 line
1.8.3.1
David Gibson
AppliedQuoted 7 lines · Thomas Huth
The global hack for creating SCSI devices has recently been removed, but this apparently broke SCSI devices on some boards that were not ready for this change yet. For the 40p machine you now get:
$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso qemu-system-ppc64: -cdrom x.iso: machine type does not support if=scsi,bus=0,unit=2
Quoted 6 lines
Fix it by providing a
lsi53c810_create()function that takes care of callingscsi_bus_legacy_handle_cmdline()after creating the corresponding SCSI controller.Fixes 1454509726719e0933c800fad00d6999752688eaSigned-off-by Thomas Huth <[email protected]>
Applied, thanks.
Quoted 46 lines
--- hw/ppc/prep.c | 2 +- hw/scsi/lsi53c895a.c | 7 +++++++ include/hw/pci/pci.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)hw/ppc/prep.c +1
-1@@ -788,7 +788,7 @@ static void ibm_40p_init(MachineState *machine)qdev_prop_set_uint32 (dev,"equipment" ,0xc0 );qdev_init_nofail (dev); -pci_create_simple (pci_bus,PCI_DEVFN (1 ,0 ),"lsi53c810" ); +lsi53c810_create (pci_bus,PCI_DEVFN (1 ,0 ));/* XXX: s3-trio at PCI_DEVFN(2, 0) */ pci_vga_init (pci_bus);hw/scsi/lsi53c895a.c +7
-0@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)scsi_bus_legacy_handle_cmdline (&s->bus); } + +void lsi53c810_create (PCIBus *bus,int devfn) +{ +LSIState *s =LSI53C895A (pci_create_simple (bus, devfn,"lsi53c810" )); + +scsi_bus_legacy_handle_cmdline (&s->bus); +}include/hw/pci/pci.h +1
-0@@ -708,6 +708,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);PCIDevice *pci_create_simple (PCIBus *bus,int devfn,const char *name);void lsi53c895a_create (PCIBus *bus); +void lsi53c810_create (PCIBus *bus,int devfn);qemu_irq pci_allocate_irq (PCIDevice *pci_dev);void pci_set_irq (PCIDevice *pci_dev,int level);
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Le 07/03/2018 à 10:08, Thomas Huth a écrit :
Quoted 7 lines
The global hack for creating SCSI devices has recently been removed, but this apparently broke SCSI devices on some boards that were not ready for this change yet. For the 40p machine you now get:
$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso qemu-system-ppc64: -cdrom x.iso: machine type does not support if=scsi,bus=0,unit=2
Quoted 6 lines
Fix it by providing a
lsi53c810_create()function that takes care of callingscsi_bus_legacy_handle_cmdline()after creating the corresponding SCSI controller.Fixes 1454509726719e0933c800fad00d6999752688eaSigned-off-by Thomas Huth <[email protected]>
Why is it required?
- because SCSI adapter is not up to date to QEMU standards (QOM, ...)?
- because board is not up to date to QEMU standards (QOM, ...)?
- because board is using SCSI devices by default? (mc->block_default_type = IF_SCSI) ?
In 2 first cases, what is missing?
In third case, maybe it may be better to put it in generic code?
You just fixed 40p and MIPS Jazz machines, but sparc/SS-10 (and other) also have the same problem...
Hervé
Quoted 47 lines
--- hw/ppc/prep.c | 2 +- hw/scsi/lsi53c895a.c | 7 +++++++ include/hw/pci/pci.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)hw/ppc/prep.c +1
-1@@ -788,7 +788,7 @@ static void ibm_40p_init(MachineState *machine)qdev_prop_set_uint32 (dev,"equipment" ,0xc0 );qdev_init_nofail (dev); -pci_create_simple (pci_bus,PCI_DEVFN (1 ,0 ),"lsi53c810" ); +lsi53c810_create (pci_bus,PCI_DEVFN (1 ,0 ));/* XXX: s3-trio at PCI_DEVFN(2, 0) */ pci_vga_init (pci_bus);hw/scsi/lsi53c895a.c +7
-0@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)scsi_bus_legacy_handle_cmdline (&s->bus); } + +void lsi53c810_create (PCIBus *bus,int devfn) +{ +LSIState *s =LSI53C895A (pci_create_simple (bus, devfn,"lsi53c810" )); + +scsi_bus_legacy_handle_cmdline (&s->bus); +}include/hw/pci/pci.h +1
-0@@ -708,6 +708,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);PCIDevice *pci_create_simple (PCIBus *bus,int devfn,const char *name);void lsi53c895a_create (PCIBus *bus); +void lsi53c810_create (PCIBus *bus,int devfn);qemu_irq pci_allocate_irq (PCIDevice *pci_dev);void pci_set_irq (PCIDevice *pci_dev,int level);
Quoted 16 lines · Hervé Poussineau
Le 07/03/2018 à 10:08, Thomas Huth a écrit :
Quoted 8 lines
The global hack for creating SCSI devices has recently been removed, but this apparently broke SCSI devices on some boards that were not ready for this change yet. For the 40p machine you now get:
$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso qemu-system-ppc64: -cdrom x.iso: machine type does not support if=scsi,bus=0,unit=2Quoted 6 lines
Fix it by providing a
lsi53c810_create()function that takes care of callingscsi_bus_legacy_handle_cmdline()after creating the corresponding SCSI controller.Fixes 1454509726719e0933c800fad00d6999752688eaSigned-off-by Thomas Huth <[email protected]>
Quoted 8 lines
Why is it required?
- because SCSI adapter is not up to date to QEMU standards (QOM, ...)?
- because board is not up to date to QEMU standards (QOM, ...)?
- because board is using SCSI devices by default? (mc->block_default_type=IF_SCSI) ?In 2 first cases, what is missing?
In third case, maybe it may be better to put it in generic code?
It's the third case. The "generic" code was just removed with commit 1454509726719e0933 since it was considered as a big hack. The generic code should not have to guess to which SCSI adapter a SCSI drive should be attached to. That's the job of the board init code, and this is what this patch is doing now for the 40p machine.
Other boards like the "pseries" machine were doing this since a long time already (see the spapr_vscsi_create() function in hw/scsi/spapr_vscsi.c for example).
Quoted 2 lines
You just fixed 40p and MIPS Jazz machines, but sparc/SS-10 (and other) also have the same problem...
I also posted a patch for the Sparc machines, you can find it here:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01830.html
Thomas
Quoted 10 lines · Thomas Huth
It's the third case. The "generic" code was just removed with commit
1454509726719e0933since it was considered as a big hack. The generic code should not have to guess to which SCSI adapter a SCSI drive should be attached to. That's the job of the board init code, and this is what this patch is doing now for the 40p machine.Other boards like the "pseries" machine were doing this since a long time already (see the
spapr_vscsi_create()function inhw/scsi/spapr_vscsi.cfor example).
Quoted 6 lines
Quoted 2 lines
You just fixed 40p and MIPS Jazz machines, but sparc/SS-10 (and other) also have the same problem...
I also posted a patch for the Sparc machines, you can find it here:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01830.html
Are we sure this is the entire list of machines that use this?
Can we in general try to avoid removing generic code features until we've checked and fixed everything that relies on them? x86 pc is not the only system we support...
thanks
-- PMM
Quoted 11 lines · Peter Maydell
Quoted 10 lines · Thomas Huth
It's the third case. The "generic" code was just removed with commit
1454509726719e0933since it was considered as a big hack. The generic code should not have to guess to which SCSI adapter a SCSI drive should be attached to. That's the job of the board init code, and this is what this patch is doing now for the 40p machine.Other boards like the "pseries" machine were doing this since a long time already (see the
spapr_vscsi_create()function inhw/scsi/spapr_vscsi.cfor example).
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
You just fixed 40p and MIPS Jazz machines, but sparc/SS-10 (and other) also have the same problem...
I also posted a patch for the Sparc machines, you can find it here:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01830.html
Are we sure this is the entire list of machines that use this?
Yes. The problem only occurs for boards that are using block_default_type = IF_SCSI.
$ grep -rl IF_SCSI hw/ hw/arm/versatilepb.c hw/arm/realview.c hw/mips/mips_jazz.c hw/ppc/prep.c hw/ppc/spapr.c hw/scsi/scsi-bus.c hw/sparc/sun4m.c hw/hppa/machine.c
versatilepb, realview and the hppa machine were already using lsi53c895a_create() that takes care of this already. spapr is properly using scsi_bus_legacy_handle_cmdline() in spapr_vscsi_create(). And for jazz, prep and sun4m, I sent the patches to the list yesterday.
Quoted 3 lines
Can we in general try to avoid removing generic code features until we've checked and fixed everything that relies on them? x86 pc is not the only system we support...
Sure. You know, I'm a ppc / s390x guy, so I am very aware of other platforms, too. In this case it was just a bad misunderstanding - I did not expect that the code would still be used on other platforms, too. Sorry for that.
The problem is rather that we still need more automatic regression tests for stuff like this. I already have got something in mind for a new
qtest: If "mkisofs" or "genisoimage" is available, we could create a boot CD-ROM automatically using the boot code from tests/boot-sector.c. Then the test could try to boot from that CD-ROM to check whether "--cdrom" is still working as expected. What do you think, does that sound reasonable?
Thomas
Quoted 16 lines · Thomas Huth
Quoted 1 line · Peter Maydell
Are we sure this is the entire list of machines that use this?
Yes. The problem only occurs for boards that are using
block_default_type=IF_SCSI.$ grep -rl IF_SCSI hw/ hw/arm/versatilepb.c hw/arm/realview.c hw/mips/mips_jazz.c hw/ppc/prep.c hw/ppc/spapr.c hw/scsi/scsi-bus.c hw/sparc/sun4m.c hw/hppa/machine.c
Quoted 4 lines
versatilepb, realview and the hppa machine were already using
lsi53c895a_create()that takes care of this already. spapr is properly usingscsi_bus_legacy_handle_cmdline()inspapr_vscsi_create(). And for jazz, prep and sun4m, I sent the patches to the list yesterday.
Cool.
Quoted 8 lines
Quoted 3 lines
Can we in general try to avoid removing generic code features until we've checked and fixed everything that relies on them? x86 pc is not the only system we support...
Sure. You know, I'm a ppc / s390x guy, so I am very aware of other platforms, too. In this case it was just a bad misunderstanding - I did not expect that the code would still be used on other platforms, too. Sorry for that.
Sorry for being grumpy -- I should have skipped sending that mail til I'd had some more coffee.
Quoted 7 lines
The problem is rather that we still need more automatic regression tests for stuff like this. I already have got something in mind for a new
qtest: If "mkisofs" or "genisoimage" is available, we could create a boot CD-ROM automatically using the boot code fromtests/boot-sector.c. Then the test could try to boot from that CD-ROM to check whether "--cdrom" is still working as expected. What do you think, does that sound reasonable?
That's probably a good plan, though it wouldn't be able to cover boards like versatile/realview that can't boot from a cdrom.
thanks
-- PMM
Quoted 11 lines · Peter Maydell
Quoted 7 lines · Thomas Huth
The problem is rather that we still need more automatic regression tests for stuff like this. I already have got something in mind for a new
qtest: If "mkisofs" or "genisoimage" is available, we could create a boot CD-ROM automatically using the boot code fromtests/boot-sector.c. Then the test could try to boot from that CD-ROM to check whether "--cdrom" is still working as expected. What do you think, does that sound reasonable?That's probably a good plan, though it wouldn't be able to cover boards like versatile/realview that can't boot from a cdrom.
We don't need to boot, we "only" need to have a qtest that sends an INQUIRY command. The code to bring up the LSI HBA can be copied from SeaBIOS, what's missing is libqos ports for the various PCI bridges...
Paolo
Le 07/03/2018 à 10:08, Thomas Huth a écrit :
Quoted 7 lines
The global hack for creating SCSI devices has recently been removed, but this apparently broke SCSI devices on some boards that were not ready for this change yet. For the 40p machine you now get:
$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso qemu-system-ppc64: -cdrom x.iso: machine type does not support if=scsi,bus=0,unit=2
Quoted 6 lines
Fix it by providing a
lsi53c810_create()function that takes care of callingscsi_bus_legacy_handle_cmdline()after creating the corresponding SCSI controller.Fixes 1454509726719e0933c800fad00d6999752688eaSigned-off-by Thomas Huth <[email protected]>
Quoted 47 lines
--- hw/ppc/prep.c | 2 +- hw/scsi/lsi53c895a.c | 7 +++++++ include/hw/pci/pci.h | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)hw/ppc/prep.c +1
-1@@ -788,7 +788,7 @@ static void ibm_40p_init(MachineState *machine)qdev_prop_set_uint32 (dev,"equipment" ,0xc0 );qdev_init_nofail (dev); -pci_create_simple (pci_bus,PCI_DEVFN (1 ,0 ),"lsi53c810" ); +lsi53c810_create (pci_bus,PCI_DEVFN (1 ,0 ));/* XXX: s3-trio at PCI_DEVFN(2, 0) */ pci_vga_init (pci_bus);hw/scsi/lsi53c895a.c +7
-0@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)scsi_bus_legacy_handle_cmdline (&s->bus); } + +void lsi53c810_create (PCIBus *bus,int devfn) +{ +LSIState *s =LSI53C895A (pci_create_simple (bus, devfn,"lsi53c810" )); + +scsi_bus_legacy_handle_cmdline (&s->bus); +}include/hw/pci/pci.h +1
-0@@ -708,6 +708,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);PCIDevice *pci_create_simple (PCIBus *bus,int devfn,const char *name);void lsi53c895a_create (PCIBus *bus); +void lsi53c810_create (PCIBus *bus,int devfn);qemu_irq pci_allocate_irq (PCIDevice *pci_dev);void pci_set_irq (PCIDevice *pci_dev,int level);