Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-08 Thread Paolo Bonzini
On 08/03/2018 11:45, Peter Maydell wrote:
> On 8 March 2018 at 10:26, Thomas Huth  wrote:
>> 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?
> 
> 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



Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-08 Thread Peter Maydell
On 8 March 2018 at 10:26, Thomas Huth  wrote:
> On 08.03.2018 11:07, Peter Maydell wrote:
>> 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.

Cool.

>> 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.

> 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?

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



Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-08 Thread Thomas Huth
On 08.03.2018 11:07, Peter Maydell wrote:
> On 8 March 2018 at 07:09, Thomas Huth  wrote:
>> 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).
>>
>>> 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.

> 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



Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-08 Thread Peter Maydell
On 8 March 2018 at 07:09, Thomas Huth  wrote:
> 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).
>
>> 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



Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-08 Thread Hervé Poussineau

Le 07/03/2018 à 10:08, Thomas Huth a écrit :

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.

Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 


Reviewed-by: Hervé Poussineau 


---
  hw/ppc/prep.c| 2 +-
  hw/scsi/lsi53c895a.c | 7 +++
  include/hw/pci/pci.h | 1 +
  3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 096d4d4..3361509 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -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);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index f3d4c4d..160657f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)
  
  scsi_bus_legacy_handle_cmdline(>bus);

  }
+
+void lsi53c810_create(PCIBus *bus, int devfn)
+{
+LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
+
+scsi_bus_legacy_handle_cmdline(>bus);
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d8c18c7..e255941 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -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);






Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread Thomas Huth
On 08.03.2018 07:58, Hervé Poussineau wrote:
> Le 07/03/2018 à 10:08, Thomas Huth a écrit :
>> 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.
>>
>> Fixes: 1454509726719e0933c800fad00d6999752688ea
>> Signed-off-by: Thomas Huth 
> 
> 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).

> 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




Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread Hervé Poussineau

Le 07/03/2018 à 10:08, Thomas Huth a écrit :

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.

Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 


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é


---
  hw/ppc/prep.c| 2 +-
  hw/scsi/lsi53c895a.c | 7 +++
  include/hw/pci/pci.h | 1 +
  3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 096d4d4..3361509 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -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);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index f3d4c4d..160657f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)
  
  scsi_bus_legacy_handle_cmdline(>bus);

  }
+
+void lsi53c810_create(PCIBus *bus, int devfn)
+{
+LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
+
+scsi_bus_legacy_handle_cmdline(>bus);
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d8c18c7..e255941 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -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);






Re: [Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread David Gibson
On Wed, Mar 07, 2018 at 10:08:14AM +0100, Thomas Huth wrote:
> 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.
> 
> Fixes: 1454509726719e0933c800fad00d6999752688ea
> Signed-off-by: Thomas Huth 

Applied, thanks.

> ---
>  hw/ppc/prep.c| 2 +-
>  hw/scsi/lsi53c895a.c | 7 +++
>  include/hw/pci/pci.h | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 096d4d4..3361509 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -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);
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index f3d4c4d..160657f 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)
>  
>  scsi_bus_legacy_handle_cmdline(>bus);
>  }
> +
> +void lsi53c810_create(PCIBus *bus, int devfn)
> +{
> +LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
> +
> +scsi_bus_legacy_handle_cmdline(>bus);
> +}
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d8c18c7..e255941 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -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);

-- 
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


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread 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

Fix it by providing a lsi53c810_create() function that takes care
of calling scsi_bus_legacy_handle_cmdline() after creating the
corresponding SCSI controller.

Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 
---
 hw/ppc/prep.c| 2 +-
 hw/scsi/lsi53c895a.c | 7 +++
 include/hw/pci/pci.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 096d4d4..3361509 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -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);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index f3d4c4d..160657f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)
 
 scsi_bus_legacy_handle_cmdline(>bus);
 }
+
+void lsi53c810_create(PCIBus *bus, int devfn)
+{
+LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
+
+scsi_bus_legacy_handle_cmdline(>bus);
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d8c18c7..e255941 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -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);
-- 
1.8.3.1