Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Paolo Bonzini 

On 19/03/21 11:51, Laurent Vivier wrote:

It would also make the patches that Laurent sent this morning unnecessary, and 
avoid the use of
aliases in the tests (so that it's clear what is tested).


We don't test the virtio frontend, but the blockdev backend, so we don't care 
what we use here.


Sort of, see for example the iothreads which, with your patch, would not 
be covered anymore on s390.



Aliases simplify the code...


Aliases are not deprecated but... let's say despised, because of the 
special casing they add.  But yes, the code simplification from your 
patch is hard to brush away.


So I agree, since you have already mostly written the patches let's just 
complete them.


Paolo

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Max Reitz 

On 19.03.21 11:51, Max Reitz wrote:

On 19.03.21 11:50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
  From that it follows that I don’t see much use in testing 
specific devices either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the 
default (say virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as 
well.  (Perhaps I’m short-sighted,

though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided

virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, 
maybe it would be better to
restrict the iotests to the "main" targets only, e.g. modify 
check-block.sh so that the tests only

run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't 
contain one of these targets.


Yes, but is that really a problem?


I should add: The thing is, I wouldn’t really call it a problem.  But 
still, as I said before somewhere in this thread, in theory we want to 
allow running the tests with every configuration.  It’s just that it’s a 
tradeoff between how much it helps and how much work it is to make them 
work.  (I gave s390 as an example, where effort was undertaken to make 
the iotests work.)


You’ve sent patches, so it seems you’re willing to invest the work. 
Sounds good to me, as long as we know it won’t rot.


Max

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Thomas Huth 

On 19/03/2021 11.50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]

  From that it follows that I don’t see much use in testing specific devices 
either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the default (say 
virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted,
though.)


That's a fair point. But still, if someone compiled QEMU only with a target 
that only provided
virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe it 
would be better to
restrict the iotests to the "main" targets only, e.g. modify check-block.sh so 
that the tests only
run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.


The iotests are skipped in quite a bunch of cases already anyway, e.g. when 
GNU sed or bash are not available in the right version. So I think it would 
be also ok to skip them in builds without one of the major architectures.
Anyway, since your patches are already ready, I think we also could simply 
go with those this time, and reconsider tweaking tests/check-block.sh the 
next time we run into this issue.


 Thomas

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Max Reitz 

On 19.03.21 11:50, Laurent Vivier wrote:

Le 19/03/2021 à 10:20, Max Reitz a écrit :

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]

  From that it follows that I don’t see much use in testing specific devices 
either.  Say there’s
a platform that provides both virtio-pci and virtio-mmio, the default (say 
virtio-pci) is fine
for the iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted,
though.)


That's a fair point. But still, if someone compiled QEMU only with a target 
that only provided
virtio-mmio, the iotests should not fail when running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe it 
would be better to
restrict the iotests to the "main" targets only, e.g. modify check-block.sh so 
that the tests only
run with x86, aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.



The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.


Yes, but is that really a problem?

Max

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Laurent Vivier 
Le 19/03/2021 à 10:29, Paolo Bonzini a écrit :
> On 19/03/21 10:20, Max Reitz wrote:
>> On 19.03.21 07:32, Thomas Huth wrote:
>>> On 18/03/2021 18.28, Max Reitz wrote:
>>> [...]
  From that it follows that I don’t see much use in testing specific 
 devices either.  Say there’s
 a platform that provides both virtio-pci and virtio-mmio, the default (say 
 virtio-pci) is fine
 for the iotests. I see little value in testing virtio-mmio as well.  
 (Perhaps I’m short-sighted,
 though.)
>>>
>>> That's a fair point. But still, if someone compiled QEMU only with a target 
>>> that only provided
>>> virtio-mmio, the iotests should not fail when running "make check".
>>> To avoid that we continue playing whack-a-mole here in the future, maybe it 
>>> would be better to
>>> restrict the iotests to the "main" targets only, e.g. modify check-block.sh 
>>> so that the tests
>>> only run with x86, aarch64, s390x and ppc64 ?
>>
>> Right, that would certainly be the simplest solution.
> 
> It would also make the patches that Laurent sent this morning unnecessary, 
> and avoid the use of
> aliases in the tests (so that it's clear what is tested).
>

We don't test the virtio frontend, but the blockdev backend, so we don't care 
what we use here.

Aliases simplify the code...

Thanks,
Laurent

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Laurent Vivier 
Le 19/03/2021 à 10:20, Max Reitz a écrit :
> On 19.03.21 07:32, Thomas Huth wrote:
>> On 18/03/2021 18.28, Max Reitz wrote:
>> [...]
>>>  From that it follows that I don’t see much use in testing specific devices 
>>> either.  Say there’s
>>> a platform that provides both virtio-pci and virtio-mmio, the default (say 
>>> virtio-pci) is fine
>>> for the iotests. I see little value in testing virtio-mmio as well.  
>>> (Perhaps I’m short-sighted,
>>> though.)
>>
>> That's a fair point. But still, if someone compiled QEMU only with a target 
>> that only provided
>> virtio-mmio, the iotests should not fail when running "make check".
>> To avoid that we continue playing whack-a-mole here in the future, maybe it 
>> would be better to
>> restrict the iotests to the "main" targets only, e.g. modify check-block.sh 
>> so that the tests only
>> run with x86, aarch64, s390x and ppc64 ?
> 
> Right, that would certainly be the simplest solution.
> 

The problem with that is we can't run the tests if target-list doesn't contain 
one of these targets.

Thanks,
Laurent

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Paolo Bonzini 

On 19/03/21 10:20, Max Reitz wrote:

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
 From that it follows that I don’t see much use in testing specific 
devices either.  Say there’s a platform that provides both virtio-pci 
and virtio-mmio, the default (say virtio-pci) is fine for the 
iotests. I see little value in testing virtio-mmio as well.  (Perhaps 
I’m short-sighted, though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided virtio-mmio, the iotests should not fail 
when running "make check".
To avoid that we continue playing whack-a-mole here in the future, 
maybe it would be better to restrict the iotests to the "main" targets 
only, e.g. modify check-block.sh so that the tests only run with x86, 
aarch64, s390x and ppc64 ?


Right, that would certainly be the simplest solution.


It would also make the patches that Laurent sent this morning 
unnecessary, and avoid the use of aliases in the tests (so that it's 
clear what is tested).


Paolo

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Max Reitz 

On 19.03.21 07:32, Thomas Huth wrote:

On 18/03/2021 18.28, Max Reitz wrote:
[...]
 From that it follows that I don’t see much use in testing specific 
devices either.  Say there’s a platform that provides both virtio-pci 
and virtio-mmio, the default (say virtio-pci) is fine for the iotests. 
I see little value in testing virtio-mmio as well.  (Perhaps I’m 
short-sighted, though.)


That's a fair point. But still, if someone compiled QEMU only with a 
target that only provided virtio-mmio, the iotests should not fail when 
running "make check".
To avoid that we continue playing whack-a-mole here in the future, maybe 
it would be better to restrict the iotests to the "main" targets only, 
e.g. modify check-block.sh so that the tests only run with x86, aarch64, 
s390x and ppc64 ?


Right, that would certainly be the simplest solution.

Max

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-19 Thread Thomas Huth 

On 18/03/2021 18.28, Max Reitz wrote:
[...]
 From that it follows that I don’t see much use in testing specific devices 
either.  Say there’s a platform that provides both virtio-pci and 
virtio-mmio, the default (say virtio-pci) is fine for the iotests. I see 
little value in testing virtio-mmio as well.  (Perhaps I’m short-sighted, 
though.)


That's a fair point. But still, if someone compiled QEMU only with a target 
that only provided virtio-mmio, the iotests should not fail when running 
"make check".
To avoid that we continue playing whack-a-mole here in the future, maybe it 
would be better to restrict the iotests to the "main" targets only, e.g. 
modify check-block.sh so that the tests only run with x86, aarch64, s390x 
and ppc64 ?


 Thomas

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Max Reitz 

On 18.03.21 17:25, Philippe Mathieu-Daudé wrote:

On 3/18/21 4:56 PM, Laurent Vivier wrote:

Le 18/03/2021 à 16:51, Laurent Vivier a écrit :

Le 18/03/2021 à 16:36, Philippe Mathieu-Daudé a écrit :

On 3/18/21 11:06 AM, Laurent Vivier wrote:

Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :

On 3/18/21 10:52 AM, Laurent Vivier wrote:

Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :

Hi Laurent,

+Paolo / Thomas

On 3/15/21 9:42 PM, Laurent Vivier wrote:

The machine is based on Goldfish interfaces defined by Google
for Android simulator. It uses Goldfish-rtc (timer and RTC),
Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).

The machine is created with 128 virtio-mmio bus, and they can
be used to use serial console, GPU, disk, NIC, HID, ...

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
---
  default-configs/devices/m68k-softmmu.mak  |   1 +
  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
  hw/m68k/virt.c| 313 ++
  MAINTAINERS   |  13 +
  hw/m68k/Kconfig   |   9 +
  hw/m68k/meson.build   |   1 +
  6 files changed, 355 insertions(+)
  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
  create mode 100644 hw/m68k/virt.c



diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
index 60d7bcfb8f2b..f839f8a03064 100644
--- a/hw/m68k/Kconfig
+++ b/hw/m68k/Kconfig
@@ -23,3 +23,12 @@ config Q800
  select ESP
  select DP8393X
  select OR_IRQ
+
+config M68K_VIRT
+bool
+select M68K_IRQC
+select VIRT_CTRL
+select GOLDFISH_PIC
+select GOLDFISH_TTY
+select GOLDFISH_RTC
+select VIRTIO_MMIO


I had this error on gitlab:

(qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
'virtio-blk-pci' is not a valid device model name
job: check-system-fedora
https://gitlab.com/philmd/qemu/-/jobs/1106469724

I bisected locally to this commit.

check-system-fedora uses build-system-fedora:

build-system-fedora:
 CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
  --enable-fdt=system --enable-slirp=system
  --enable-capstone=system

I'm confused because the machine provides a VIRTIO bus
via MMIO:

config VIRTIO_MMIO
 bool
 select VIRTIO

I remember I tested your machine with virtio-blk-device.

config VIRTIO_BLK
 bool
 default y
 depends on VIRTIO

Ah, this is virtio-blk-pci, which has:

virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
files('virtio-blk-pci.c'))
virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

And VIRTIO_PCI isn't selected...


This machine doesn't have virtio-pci, but only virtio-mmio buses.


Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
for this machine". So the problem isn't the m68k-virt machine addition,
but it shows another problem elsewhere.


Are the tests incorrect then?

libqos isn't restricted to PCI:

tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
"virtio-blk")) {
tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
in virtio-blk-device\n", interface);
tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
tests/qtest/libqos/virtio-blk.c:111:
qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
tests/qtest/libqos/virtio-blk.c:112:
qos_node_consumes("virtio-blk-device", "virtio-bus", );
tests/qtest/libqos/virtio-blk.c:113:
qos_node_produces("virtio-blk-device", "virtio-blk");

But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
use a generic virtio-blk-device instead, hoping it get plugged correctly
to the virtio bus...


Yes, it's how the machine work: it has 128 virtio-mmio buses and virtio-devices 
are plugged directly
in the first free ones.

I think the fix would be to disable the virtio-blk-pci test for the machines 
without PCI bus.

Why is it executed for now?


This is probably the problem root cause.

Possible fix:

-->8 --
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 66ee9fbf450..d7f3fad51c1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -217,13 +217,17 @@
'emc141x-test.c',
'usb-hcd-ohci-test.c',
'virtio-test.c',
-  'virtio-blk-test.c',
-  'virtio-net-test.c',
-  'virtio-rng-test.c',
-  'virtio-scsi-test.c',
'virtio-serial-test.c',
'vmxnet3-test.c',
  )
+if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
+  qos_test_ss.add(
+'virtio-blk-test.c',
+'virtio-net-test.c',
+'virtio-rng-test.c',
+'virtio-scsi-test.c',
+  )
+endif
  if have_virtfs
qos_test_ss.add(files('virtio-9p-test.c'))
  endif
---

I'll test that locally but not on Gitlab.


This approach doesn't

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Paolo Bonzini 

On 18/03/21 16:36, Philippe Mathieu-Daudé wrote:

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 66ee9fbf450..d7f3fad51c1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -217,13 +217,17 @@
'emc141x-test.c',
'usb-hcd-ohci-test.c',
'virtio-test.c',
-  'virtio-blk-test.c',
-  'virtio-net-test.c',
-  'virtio-rng-test.c',
-  'virtio-scsi-test.c',
'virtio-serial-test.c',
'vmxnet3-test.c',
  )
+if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
+  qos_test_ss.add(
+'virtio-blk-test.c',
+'virtio-net-test.c',
+'virtio-rng-test.c',
+'virtio-scsi-test.c',
+  )
+endif
  if have_virtfs
qos_test_ss.add(files('virtio-9p-test.c'))
  endif


I don't understand, what would this be trying to do?  (And besides, why 
would it work?  The CI failure is in qemu-iotests that has no connection 
to qtest at all).



Maybe you need to complete it for your arch? I've been using that:


Instead of completing it, you can drop your arch from virtio-*-pci, so:


+{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL
+  & ~(QEMU_ARCH_S390X |
QEMU_ARCH_M68K) },


but do not add m68k anywhere.

Paolo

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Philippe Mathieu-Daudé 
On 3/18/21 4:56 PM, Laurent Vivier wrote:
> Le 18/03/2021 à 16:51, Laurent Vivier a écrit :
>> Le 18/03/2021 à 16:36, Philippe Mathieu-Daudé a écrit :
>>> On 3/18/21 11:06 AM, Laurent Vivier wrote:
 Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :
> On 3/18/21 10:52 AM, Laurent Vivier wrote:
>> Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
>>> Hi Laurent,
>>>
>>> +Paolo / Thomas
>>>
>>> On 3/15/21 9:42 PM, Laurent Vivier wrote:
 The machine is based on Goldfish interfaces defined by Google
 for Android simulator. It uses Goldfish-rtc (timer and RTC),
 Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).

 The machine is created with 128 virtio-mmio bus, and they can
 be used to use serial console, GPU, disk, NIC, HID, ...

 Signed-off-by: Laurent Vivier 
 Reviewed-by: Richard Henderson 
 Tested-by: Philippe Mathieu-Daudé 
 Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
 ---
  default-configs/devices/m68k-softmmu.mak  |   1 +
  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
  hw/m68k/virt.c| 313 ++
  MAINTAINERS   |  13 +
  hw/m68k/Kconfig   |   9 +
  hw/m68k/meson.build   |   1 +
  6 files changed, 355 insertions(+)
  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
  create mode 100644 hw/m68k/virt.c
>>>
 diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
 index 60d7bcfb8f2b..f839f8a03064 100644
 --- a/hw/m68k/Kconfig
 +++ b/hw/m68k/Kconfig
 @@ -23,3 +23,12 @@ config Q800
  select ESP
  select DP8393X
  select OR_IRQ
 +
 +config M68K_VIRT
 +bool
 +select M68K_IRQC
 +select VIRT_CTRL
 +select GOLDFISH_PIC
 +select GOLDFISH_TTY
 +select GOLDFISH_RTC
 +select VIRTIO_MMIO
>>>
>>> I had this error on gitlab:
>>>
>>> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
>>> 'virtio-blk-pci' is not a valid device model name
>>> job: check-system-fedora
>>> https://gitlab.com/philmd/qemu/-/jobs/1106469724
>>>
>>> I bisected locally to this commit.
>>>
>>> check-system-fedora uses build-system-fedora:
>>>
>>> build-system-fedora:
>>> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>>>  --enable-fdt=system --enable-slirp=system
>>>  --enable-capstone=system
>>>
>>> I'm confused because the machine provides a VIRTIO bus
>>> via MMIO:
>>>
>>> config VIRTIO_MMIO
>>> bool
>>> select VIRTIO
>>>
>>> I remember I tested your machine with virtio-blk-device.
>>>
>>> config VIRTIO_BLK
>>> bool
>>> default y
>>> depends on VIRTIO
>>>
>>> Ah, this is virtio-blk-pci, which has:
>>>
>>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
>>> files('virtio-blk-pci.c'))
>>> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>>>
>>> And VIRTIO_PCI isn't selected...
>>
>> This machine doesn't have virtio-pci, but only virtio-mmio buses.
>
> Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
> for this machine". So the problem isn't the m68k-virt machine addition,
> but it shows another problem elsewhere.
>
>>> Are the tests incorrect then?
>>>
>>> libqos isn't restricted to PCI:
>>>
>>> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
>>> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
>>> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
>>> "virtio-blk")) {
>>> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
>>> in virtio-blk-device\n", interface);
>>> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
>>> tests/qtest/libqos/virtio-blk.c:111:
>>> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
>>> tests/qtest/libqos/virtio-blk.c:112:
>>> qos_node_consumes("virtio-blk-device", "virtio-bus", );
>>> tests/qtest/libqos/virtio-blk.c:113:
>>> qos_node_produces("virtio-blk-device", "virtio-blk");
>>>
>>> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
>>> use a generic virtio-blk-device instead, hoping it get plugged correctly
>>> to the virtio bus...
>>
>> Yes, it's how the machine work: it has 128 virtio-mmio buses and 
>> virtio-devices are plugged directly
>> in the first free ones.
>>
>> I think the fix would be to disable the virtio-blk-pci test

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Laurent Vivier 
Le 18/03/2021 à 16:51, Laurent Vivier a écrit :
> Le 18/03/2021 à 16:36, Philippe Mathieu-Daudé a écrit :
>> On 3/18/21 11:06 AM, Laurent Vivier wrote:
>>> Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :
 On 3/18/21 10:52 AM, Laurent Vivier wrote:
> Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
>> Hi Laurent,
>>
>> +Paolo / Thomas
>>
>> On 3/15/21 9:42 PM, Laurent Vivier wrote:
>>> The machine is based on Goldfish interfaces defined by Google
>>> for Android simulator. It uses Goldfish-rtc (timer and RTC),
>>> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>>>
>>> The machine is created with 128 virtio-mmio bus, and they can
>>> be used to use serial console, GPU, disk, NIC, HID, ...
>>>
>>> Signed-off-by: Laurent Vivier 
>>> Reviewed-by: Richard Henderson 
>>> Tested-by: Philippe Mathieu-Daudé 
>>> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
>>> ---
>>>  default-configs/devices/m68k-softmmu.mak  |   1 +
>>>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>>>  hw/m68k/virt.c| 313 ++
>>>  MAINTAINERS   |  13 +
>>>  hw/m68k/Kconfig   |   9 +
>>>  hw/m68k/meson.build   |   1 +
>>>  6 files changed, 355 insertions(+)
>>>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>>>  create mode 100644 hw/m68k/virt.c
>>
>>> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
>>> index 60d7bcfb8f2b..f839f8a03064 100644
>>> --- a/hw/m68k/Kconfig
>>> +++ b/hw/m68k/Kconfig
>>> @@ -23,3 +23,12 @@ config Q800
>>>  select ESP
>>>  select DP8393X
>>>  select OR_IRQ
>>> +
>>> +config M68K_VIRT
>>> +bool
>>> +select M68K_IRQC
>>> +select VIRT_CTRL
>>> +select GOLDFISH_PIC
>>> +select GOLDFISH_TTY
>>> +select GOLDFISH_RTC
>>> +select VIRTIO_MMIO
>>
>> I had this error on gitlab:
>>
>> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
>> 'virtio-blk-pci' is not a valid device model name
>> job: check-system-fedora
>> https://gitlab.com/philmd/qemu/-/jobs/1106469724
>>
>> I bisected locally to this commit.
>>
>> check-system-fedora uses build-system-fedora:
>>
>> build-system-fedora:
>> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>>  --enable-fdt=system --enable-slirp=system
>>  --enable-capstone=system
>>
>> I'm confused because the machine provides a VIRTIO bus
>> via MMIO:
>>
>> config VIRTIO_MMIO
>> bool
>> select VIRTIO
>>
>> I remember I tested your machine with virtio-blk-device.
>>
>> config VIRTIO_BLK
>> bool
>> default y
>> depends on VIRTIO
>>
>> Ah, this is virtio-blk-pci, which has:
>>
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
>> files('virtio-blk-pci.c'))
>> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>>
>> And VIRTIO_PCI isn't selected...
>
> This machine doesn't have virtio-pci, but only virtio-mmio buses.

 Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
 for this machine". So the problem isn't the m68k-virt machine addition,
 but it shows another problem elsewhere.

>> Are the tests incorrect then?
>>
>> libqos isn't restricted to PCI:
>>
>> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
>> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
>> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
>> "virtio-blk")) {
>> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
>> in virtio-blk-device\n", interface);
>> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
>> tests/qtest/libqos/virtio-blk.c:111:
>> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
>> tests/qtest/libqos/virtio-blk.c:112:
>> qos_node_consumes("virtio-blk-device", "virtio-bus", );
>> tests/qtest/libqos/virtio-blk.c:113:
>> qos_node_produces("virtio-blk-device", "virtio-blk");
>>
>> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
>> use a generic virtio-blk-device instead, hoping it get plugged correctly
>> to the virtio bus...
>
> Yes, it's how the machine work: it has 128 virtio-mmio buses and 
> virtio-devices are plugged directly
> in the first free ones.
>
> I think the fix would be to disable the virtio-blk-pci test for the 
> machines without PCI bus.
>
> Why is it executed for now?

 This is probably the problem root cause.

 Possible fix:

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Laurent Vivier 
Le 18/03/2021 à 16:36, Philippe Mathieu-Daudé a écrit :
> On 3/18/21 11:06 AM, Laurent Vivier wrote:
>> Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :
>>> On 3/18/21 10:52 AM, Laurent Vivier wrote:
 Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
> Hi Laurent,
>
> +Paolo / Thomas
>
> On 3/15/21 9:42 PM, Laurent Vivier wrote:
>> The machine is based on Goldfish interfaces defined by Google
>> for Android simulator. It uses Goldfish-rtc (timer and RTC),
>> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>>
>> The machine is created with 128 virtio-mmio bus, and they can
>> be used to use serial console, GPU, disk, NIC, HID, ...
>>
>> Signed-off-by: Laurent Vivier 
>> Reviewed-by: Richard Henderson 
>> Tested-by: Philippe Mathieu-Daudé 
>> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
>> ---
>>  default-configs/devices/m68k-softmmu.mak  |   1 +
>>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>>  hw/m68k/virt.c| 313 ++
>>  MAINTAINERS   |  13 +
>>  hw/m68k/Kconfig   |   9 +
>>  hw/m68k/meson.build   |   1 +
>>  6 files changed, 355 insertions(+)
>>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>>  create mode 100644 hw/m68k/virt.c
>
>> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
>> index 60d7bcfb8f2b..f839f8a03064 100644
>> --- a/hw/m68k/Kconfig
>> +++ b/hw/m68k/Kconfig
>> @@ -23,3 +23,12 @@ config Q800
>>  select ESP
>>  select DP8393X
>>  select OR_IRQ
>> +
>> +config M68K_VIRT
>> +bool
>> +select M68K_IRQC
>> +select VIRT_CTRL
>> +select GOLDFISH_PIC
>> +select GOLDFISH_TTY
>> +select GOLDFISH_RTC
>> +select VIRTIO_MMIO
>
> I had this error on gitlab:
>
> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
> 'virtio-blk-pci' is not a valid device model name
> job: check-system-fedora
> https://gitlab.com/philmd/qemu/-/jobs/1106469724
>
> I bisected locally to this commit.
>
> check-system-fedora uses build-system-fedora:
>
> build-system-fedora:
> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>  --enable-fdt=system --enable-slirp=system
>  --enable-capstone=system
>
> I'm confused because the machine provides a VIRTIO bus
> via MMIO:
>
> config VIRTIO_MMIO
> bool
> select VIRTIO
>
> I remember I tested your machine with virtio-blk-device.
>
> config VIRTIO_BLK
> bool
> default y
> depends on VIRTIO
>
> Ah, this is virtio-blk-pci, which has:
>
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
> files('virtio-blk-pci.c'))
> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>
> And VIRTIO_PCI isn't selected...

 This machine doesn't have virtio-pci, but only virtio-mmio buses.
>>>
>>> Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
>>> for this machine". So the problem isn't the m68k-virt machine addition,
>>> but it shows another problem elsewhere.
>>>
> Are the tests incorrect then?
>
> libqos isn't restricted to PCI:
>
> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
> "virtio-blk")) {
> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
> in virtio-blk-device\n", interface);
> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
> tests/qtest/libqos/virtio-blk.c:111:
> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
> tests/qtest/libqos/virtio-blk.c:112:
> qos_node_consumes("virtio-blk-device", "virtio-bus", );
> tests/qtest/libqos/virtio-blk.c:113:
> qos_node_produces("virtio-blk-device", "virtio-blk");
>
> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
> use a generic virtio-blk-device instead, hoping it get plugged correctly
> to the virtio bus...

 Yes, it's how the machine work: it has 128 virtio-mmio buses and 
 virtio-devices are plugged directly
 in the first free ones.

 I think the fix would be to disable the virtio-blk-pci test for the 
 machines without PCI bus.

 Why is it executed for now?
>>>
>>> This is probably the problem root cause.
>>>
>>> Possible fix:
>>>
>>> -->8 --
>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>>> index 66ee9fbf450..d7f3fad51c1 100644
>>> --- a/tests/qtest/meson.build
>>> +++

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Philippe Mathieu-Daudé 
On 3/18/21 11:06 AM, Laurent Vivier wrote:
> Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :
>> On 3/18/21 10:52 AM, Laurent Vivier wrote:
>>> Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
 Hi Laurent,

 +Paolo / Thomas

 On 3/15/21 9:42 PM, Laurent Vivier wrote:
> The machine is based on Goldfish interfaces defined by Google
> for Android simulator. It uses Goldfish-rtc (timer and RTC),
> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>
> The machine is created with 128 virtio-mmio bus, and they can
> be used to use serial console, GPU, disk, NIC, HID, ...
>
> Signed-off-by: Laurent Vivier 
> Reviewed-by: Richard Henderson 
> Tested-by: Philippe Mathieu-Daudé 
> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
> ---
>  default-configs/devices/m68k-softmmu.mak  |   1 +
>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>  hw/m68k/virt.c| 313 ++
>  MAINTAINERS   |  13 +
>  hw/m68k/Kconfig   |   9 +
>  hw/m68k/meson.build   |   1 +
>  6 files changed, 355 insertions(+)
>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>  create mode 100644 hw/m68k/virt.c

> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
> index 60d7bcfb8f2b..f839f8a03064 100644
> --- a/hw/m68k/Kconfig
> +++ b/hw/m68k/Kconfig
> @@ -23,3 +23,12 @@ config Q800
>  select ESP
>  select DP8393X
>  select OR_IRQ
> +
> +config M68K_VIRT
> +bool
> +select M68K_IRQC
> +select VIRT_CTRL
> +select GOLDFISH_PIC
> +select GOLDFISH_TTY
> +select GOLDFISH_RTC
> +select VIRTIO_MMIO

 I had this error on gitlab:

 (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
 'virtio-blk-pci' is not a valid device model name
 job: check-system-fedora
 https://gitlab.com/philmd/qemu/-/jobs/1106469724

 I bisected locally to this commit.

 check-system-fedora uses build-system-fedora:

 build-system-fedora:
 CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
  --enable-fdt=system --enable-slirp=system
  --enable-capstone=system

 I'm confused because the machine provides a VIRTIO bus
 via MMIO:

 config VIRTIO_MMIO
 bool
 select VIRTIO

 I remember I tested your machine with virtio-blk-device.

 config VIRTIO_BLK
 bool
 default y
 depends on VIRTIO

 Ah, this is virtio-blk-pci, which has:

 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
 files('virtio-blk-pci.c'))
 virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

 And VIRTIO_PCI isn't selected...
>>>
>>> This machine doesn't have virtio-pci, but only virtio-mmio buses.
>>
>> Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
>> for this machine". So the problem isn't the m68k-virt machine addition,
>> but it shows another problem elsewhere.
>>
 Are the tests incorrect then?

 libqos isn't restricted to PCI:

 tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
 tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
 tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
 "virtio-blk")) {
 tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
 in virtio-blk-device\n", interface);
 tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
 tests/qtest/libqos/virtio-blk.c:111:
 qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
 tests/qtest/libqos/virtio-blk.c:112:
 qos_node_consumes("virtio-blk-device", "virtio-bus", );
 tests/qtest/libqos/virtio-blk.c:113:
 qos_node_produces("virtio-blk-device", "virtio-blk");

 But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
 use a generic virtio-blk-device instead, hoping it get plugged correctly
 to the virtio bus...
>>>
>>> Yes, it's how the machine work: it has 128 virtio-mmio buses and 
>>> virtio-devices are plugged directly
>>> in the first free ones.
>>>
>>> I think the fix would be to disable the virtio-blk-pci test for the 
>>> machines without PCI bus.
>>>
>>> Why is it executed for now?
>>
>> This is probably the problem root cause.
>>
>> Possible fix:
>>
>> -->8 --
>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>> index 66ee9fbf450..d7f3fad51c1 100644
>> --- a/tests/qtest/meson.build
>> +++ b/tests/qtest/meson.build
>> @@ -217,13 +217,17 @@
>>'emc141x-test.c',
>>'usb-hcd-ohci-test.c',
>>'virtio-test.c',
>> -  'virtio-blk-test.c',
>> -  'virtio-net-test.c',
>> -  'virtio-rng-test.c',
>>

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Philippe Mathieu-Daudé 
On 3/18/21 12:10 PM, Peter Maydell wrote:
> On Thu, 18 Mar 2021 at 10:45, Paolo Bonzini  wrote:
>>
>> On 18/03/21 11:40, Peter Maydell wrote:
>>> On Thu, 18 Mar 2021 at 10:37, Paolo Bonzini  wrote:

 On 18/03/21 11:06, Laurent Vivier wrote:
> This also removes the virtio-devices test, I think we should keep the
> files, but in the files to disable the PCI part when it is not
> available.

 I think we should just shuffle the targets in the gitlab YAML to bypass
 the issue.
>>>
>>> Then we'll hit it again later. I'm pretty sure this isn't the
>>> first time we've run into "some test makes dubious assumptions"...
>>
>> We can both fix qemu-iotests and CI configuration, but m68k is certainly
>> not the culprit here.  And we are going to make more assumptions over
>> time, not fewer, in order to keep the CI time at bay.
> 
> I don't see why CI time is relevant to whether the test says
> "I require X,Y,Z, so don't run me on configs without those"
> or whether it just randomly assumes X,Y,Z are always present
> or that if it says "I require W" than W must imply X,Y,Z...

Recently we changed a bit our view and are trying to have smarter
tests. In particular building target/device agnostic tests, and
have the test queries the QEMU binary what features/devices are
available before running. This will take some time before we get
there, unlikely for the 6.0 release. For short term, Paolo's
"shuffle gitlab YAML" suggestion is simpler.

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Peter Maydell 
On Thu, 18 Mar 2021 at 10:45, Paolo Bonzini  wrote:
>
> On 18/03/21 11:40, Peter Maydell wrote:
> > On Thu, 18 Mar 2021 at 10:37, Paolo Bonzini  wrote:
> >>
> >> On 18/03/21 11:06, Laurent Vivier wrote:
> >>> This also removes the virtio-devices test, I think we should keep the
> >>> files, but in the files to disable the PCI part when it is not
> >>> available.
> >>
> >> I think we should just shuffle the targets in the gitlab YAML to bypass
> >> the issue.
> >
> > Then we'll hit it again later. I'm pretty sure this isn't the
> > first time we've run into "some test makes dubious assumptions"...
>
> We can both fix qemu-iotests and CI configuration, but m68k is certainly
> not the culprit here.  And we are going to make more assumptions over
> time, not fewer, in order to keep the CI time at bay.

I don't see why CI time is relevant to whether the test says
"I require X,Y,Z, so don't run me on configs without those"
or whether it just randomly assumes X,Y,Z are always present
or that if it says "I require W" than W must imply X,Y,Z...

thanks
-- PMM

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Paolo Bonzini 

On 18/03/21 11:40, Peter Maydell wrote:

On Thu, 18 Mar 2021 at 10:37, Paolo Bonzini  wrote:


On 18/03/21 11:06, Laurent Vivier wrote:

This also removes the virtio-devices test, I think we should keep the
files, but in the files to disable the PCI part when it is not
available.


I think we should just shuffle the targets in the gitlab YAML to bypass
the issue.


Then we'll hit it again later. I'm pretty sure this isn't the
first time we've run into "some test makes dubious assumptions"...


We can both fix qemu-iotests and CI configuration, but m68k is certainly 
not the culprit here.  And we are going to make more assumptions over 
time, not fewer, in order to keep the CI time at bay.


Paolo

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Peter Maydell 
On Thu, 18 Mar 2021 at 10:37, Paolo Bonzini  wrote:
>
> On 18/03/21 11:06, Laurent Vivier wrote:
> > This also removes the virtio-devices test, I think we should keep the
> > files, but in the files to disable the PCI part when it is not
> > available.
>
> I think we should just shuffle the targets in the gitlab YAML to bypass
> the issue.

Then we'll hit it again later. I'm pretty sure this isn't the
first time we've run into "some test makes dubious assumptions"...

-- PMM

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Paolo Bonzini 

On 18/03/21 11:06, Laurent Vivier wrote:

This also removes the virtio-devices test, I think we should keep the
files, but in the files to disable the PCI part when it is not
available.


I think we should just shuffle the targets in the gitlab YAML to bypass 
the issue.


Paolo

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Laurent Vivier 
Le 18/03/2021 à 11:02, Philippe Mathieu-Daudé a écrit :
> On 3/18/21 10:52 AM, Laurent Vivier wrote:
>> Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
>>> Hi Laurent,
>>>
>>> +Paolo / Thomas
>>>
>>> On 3/15/21 9:42 PM, Laurent Vivier wrote:
 The machine is based on Goldfish interfaces defined by Google
 for Android simulator. It uses Goldfish-rtc (timer and RTC),
 Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).

 The machine is created with 128 virtio-mmio bus, and they can
 be used to use serial console, GPU, disk, NIC, HID, ...

 Signed-off-by: Laurent Vivier 
 Reviewed-by: Richard Henderson 
 Tested-by: Philippe Mathieu-Daudé 
 Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
 ---
  default-configs/devices/m68k-softmmu.mak  |   1 +
  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
  hw/m68k/virt.c| 313 ++
  MAINTAINERS   |  13 +
  hw/m68k/Kconfig   |   9 +
  hw/m68k/meson.build   |   1 +
  6 files changed, 355 insertions(+)
  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
  create mode 100644 hw/m68k/virt.c
>>>
 diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
 index 60d7bcfb8f2b..f839f8a03064 100644
 --- a/hw/m68k/Kconfig
 +++ b/hw/m68k/Kconfig
 @@ -23,3 +23,12 @@ config Q800
  select ESP
  select DP8393X
  select OR_IRQ
 +
 +config M68K_VIRT
 +bool
 +select M68K_IRQC
 +select VIRT_CTRL
 +select GOLDFISH_PIC
 +select GOLDFISH_TTY
 +select GOLDFISH_RTC
 +select VIRTIO_MMIO
>>>
>>> I had this error on gitlab:
>>>
>>> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
>>> 'virtio-blk-pci' is not a valid device model name
>>> job: check-system-fedora
>>> https://gitlab.com/philmd/qemu/-/jobs/1106469724
>>>
>>> I bisected locally to this commit.
>>>
>>> check-system-fedora uses build-system-fedora:
>>>
>>> build-system-fedora:
>>> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>>>  --enable-fdt=system --enable-slirp=system
>>>  --enable-capstone=system
>>>
>>> I'm confused because the machine provides a VIRTIO bus
>>> via MMIO:
>>>
>>> config VIRTIO_MMIO
>>> bool
>>> select VIRTIO
>>>
>>> I remember I tested your machine with virtio-blk-device.
>>>
>>> config VIRTIO_BLK
>>> bool
>>> default y
>>> depends on VIRTIO
>>>
>>> Ah, this is virtio-blk-pci, which has:
>>>
>>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
>>> files('virtio-blk-pci.c'))
>>> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>>>
>>> And VIRTIO_PCI isn't selected...
>>
>> This machine doesn't have virtio-pci, but only virtio-mmio buses.
> 
> Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
> for this machine". So the problem isn't the m68k-virt machine addition,
> but it shows another problem elsewhere.
> 
>>> Are the tests incorrect then?
>>>
>>> libqos isn't restricted to PCI:
>>>
>>> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
>>> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
>>> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
>>> "virtio-blk")) {
>>> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
>>> in virtio-blk-device\n", interface);
>>> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
>>> tests/qtest/libqos/virtio-blk.c:111:
>>> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
>>> tests/qtest/libqos/virtio-blk.c:112:
>>> qos_node_consumes("virtio-blk-device", "virtio-bus", );
>>> tests/qtest/libqos/virtio-blk.c:113:
>>> qos_node_produces("virtio-blk-device", "virtio-blk");
>>>
>>> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
>>> use a generic virtio-blk-device instead, hoping it get plugged correctly
>>> to the virtio bus...
>>
>> Yes, it's how the machine work: it has 128 virtio-mmio buses and 
>> virtio-devices are plugged directly
>> in the first free ones.
>>
>> I think the fix would be to disable the virtio-blk-pci test for the machines 
>> without PCI bus.
>>
>> Why is it executed for now?
> 
> This is probably the problem root cause.
> 
> Possible fix:
> 
> -->8 --
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 66ee9fbf450..d7f3fad51c1 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -217,13 +217,17 @@
>'emc141x-test.c',
>'usb-hcd-ohci-test.c',
>'virtio-test.c',
> -  'virtio-blk-test.c',
> -  'virtio-net-test.c',
> -  'virtio-rng-test.c',
> -  'virtio-scsi-test.c',
>'virtio-serial-test.c',
>'vmxnet3-test.c',
>  )
> +if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
> +  qos_test_ss.add(
> +

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Philippe Mathieu-Daudé 
On 3/18/21 10:52 AM, Laurent Vivier wrote:
> Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
>> Hi Laurent,
>>
>> +Paolo / Thomas
>>
>> On 3/15/21 9:42 PM, Laurent Vivier wrote:
>>> The machine is based on Goldfish interfaces defined by Google
>>> for Android simulator. It uses Goldfish-rtc (timer and RTC),
>>> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>>>
>>> The machine is created with 128 virtio-mmio bus, and they can
>>> be used to use serial console, GPU, disk, NIC, HID, ...
>>>
>>> Signed-off-by: Laurent Vivier 
>>> Reviewed-by: Richard Henderson 
>>> Tested-by: Philippe Mathieu-Daudé 
>>> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
>>> ---
>>>  default-configs/devices/m68k-softmmu.mak  |   1 +
>>>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>>>  hw/m68k/virt.c| 313 ++
>>>  MAINTAINERS   |  13 +
>>>  hw/m68k/Kconfig   |   9 +
>>>  hw/m68k/meson.build   |   1 +
>>>  6 files changed, 355 insertions(+)
>>>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>>>  create mode 100644 hw/m68k/virt.c
>>
>>> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
>>> index 60d7bcfb8f2b..f839f8a03064 100644
>>> --- a/hw/m68k/Kconfig
>>> +++ b/hw/m68k/Kconfig
>>> @@ -23,3 +23,12 @@ config Q800
>>>  select ESP
>>>  select DP8393X
>>>  select OR_IRQ
>>> +
>>> +config M68K_VIRT
>>> +bool
>>> +select M68K_IRQC
>>> +select VIRT_CTRL
>>> +select GOLDFISH_PIC
>>> +select GOLDFISH_TTY
>>> +select GOLDFISH_RTC
>>> +select VIRTIO_MMIO
>>
>> I had this error on gitlab:
>>
>> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
>> 'virtio-blk-pci' is not a valid device model name
>> job: check-system-fedora
>> https://gitlab.com/philmd/qemu/-/jobs/1106469724
>>
>> I bisected locally to this commit.
>>
>> check-system-fedora uses build-system-fedora:
>>
>> build-system-fedora:
>> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>>  --enable-fdt=system --enable-slirp=system
>>  --enable-capstone=system
>>
>> I'm confused because the machine provides a VIRTIO bus
>> via MMIO:
>>
>> config VIRTIO_MMIO
>> bool
>> select VIRTIO
>>
>> I remember I tested your machine with virtio-blk-device.
>>
>> config VIRTIO_BLK
>> bool
>> default y
>> depends on VIRTIO
>>
>> Ah, this is virtio-blk-pci, which has:
>>
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
>> files('virtio-blk-pci.c'))
>> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>>
>> And VIRTIO_PCI isn't selected...
> 
> This machine doesn't have virtio-pci, but only virtio-mmio buses.

Yes. I meant "VIRTIO_PCI isn't selected, which is the correct config
for this machine". So the problem isn't the m68k-virt machine addition,
but it shows another problem elsewhere.

>> Are the tests incorrect then?
>>
>> libqos isn't restricted to PCI:
>>
>> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
>> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
>> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
>> "virtio-blk")) {
>> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
>> in virtio-blk-device\n", interface);
>> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
>> tests/qtest/libqos/virtio-blk.c:111:
>> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
>> tests/qtest/libqos/virtio-blk.c:112:
>> qos_node_consumes("virtio-blk-device", "virtio-bus", );
>> tests/qtest/libqos/virtio-blk.c:113:
>> qos_node_produces("virtio-blk-device", "virtio-blk");
>>
>> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
>> use a generic virtio-blk-device instead, hoping it get plugged correctly
>> to the virtio bus...
> 
> Yes, it's how the machine work: it has 128 virtio-mmio buses and 
> virtio-devices are plugged directly
> in the first free ones.
> 
> I think the fix would be to disable the virtio-blk-pci test for the machines 
> without PCI bus.
> 
> Why is it executed for now?

This is probably the problem root cause.

Possible fix:

-->8 --
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 66ee9fbf450..d7f3fad51c1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -217,13 +217,17 @@
   'emc141x-test.c',
   'usb-hcd-ohci-test.c',
   'virtio-test.c',
-  'virtio-blk-test.c',
-  'virtio-net-test.c',
-  'virtio-rng-test.c',
-  'virtio-scsi-test.c',
   'virtio-serial-test.c',
   'vmxnet3-test.c',
 )
+if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
+  qos_test_ss.add(
+'virtio-blk-test.c',
+'virtio-net-test.c',
+'virtio-rng-test.c',
+'virtio-scsi-test.c',
+  )
+endif
 if have_virtfs
   qos_test_ss.add(files('virtio-9p-test.c'))
 endif
---

I'll test that locally but not on Gitlab.

Thanks,

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Laurent Vivier 
Le 18/03/2021 à 10:19, Philippe Mathieu-Daudé a écrit :
> Hi Laurent,
> 
> +Paolo / Thomas
> 
> On 3/15/21 9:42 PM, Laurent Vivier wrote:
>> The machine is based on Goldfish interfaces defined by Google
>> for Android simulator. It uses Goldfish-rtc (timer and RTC),
>> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
>>
>> The machine is created with 128 virtio-mmio bus, and they can
>> be used to use serial console, GPU, disk, NIC, HID, ...
>>
>> Signed-off-by: Laurent Vivier 
>> Reviewed-by: Richard Henderson 
>> Tested-by: Philippe Mathieu-Daudé 
>> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
>> ---
>>  default-configs/devices/m68k-softmmu.mak  |   1 +
>>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>>  hw/m68k/virt.c| 313 ++
>>  MAINTAINERS   |  13 +
>>  hw/m68k/Kconfig   |   9 +
>>  hw/m68k/meson.build   |   1 +
>>  6 files changed, 355 insertions(+)
>>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>>  create mode 100644 hw/m68k/virt.c
> 
>> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
>> index 60d7bcfb8f2b..f839f8a03064 100644
>> --- a/hw/m68k/Kconfig
>> +++ b/hw/m68k/Kconfig
>> @@ -23,3 +23,12 @@ config Q800
>>  select ESP
>>  select DP8393X
>>  select OR_IRQ
>> +
>> +config M68K_VIRT
>> +bool
>> +select M68K_IRQC
>> +select VIRT_CTRL
>> +select GOLDFISH_PIC
>> +select GOLDFISH_TTY
>> +select GOLDFISH_RTC
>> +select VIRTIO_MMIO
> 
> I had this error on gitlab:
> 
> (qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
> 'virtio-blk-pci' is not a valid device model name
> job: check-system-fedora
> https://gitlab.com/philmd/qemu/-/jobs/1106469724
> 
> I bisected locally to this commit.
> 
> check-system-fedora uses build-system-fedora:
> 
> build-system-fedora:
> CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
>  --enable-fdt=system --enable-slirp=system
>  --enable-capstone=system
> 
> I'm confused because the machine provides a VIRTIO bus
> via MMIO:
> 
> config VIRTIO_MMIO
> bool
> select VIRTIO
> 
> I remember I tested your machine with virtio-blk-device.
> 
> config VIRTIO_BLK
> bool
> default y
> depends on VIRTIO
> 
> Ah, this is virtio-blk-pci, which has:
> 
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
> files('virtio-blk-pci.c'))
> virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
> 
> And VIRTIO_PCI isn't selected...

This machine doesn't have virtio-pci, but only virtio-mmio buses.

> Are the tests incorrect then?
> 
> libqos isn't restricted to PCI:
> 
> tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
> tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
> tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
> "virtio-blk")) {
> tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
> in virtio-blk-device\n", interface);
> tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
> tests/qtest/libqos/virtio-blk.c:111:
> qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
> tests/qtest/libqos/virtio-blk.c:112:
> qos_node_consumes("virtio-blk-device", "virtio-bus", );
> tests/qtest/libqos/virtio-blk.c:113:
> qos_node_produces("virtio-blk-device", "virtio-blk");
> 
> But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
> use a generic virtio-blk-device instead, hoping it get plugged correctly
> to the virtio bus...

Yes, it's how the machine work: it has 128 virtio-mmio buses and virtio-devices 
are plugged directly
in the first free ones.

I think the fix would be to disable the virtio-blk-pci test for the machines 
without PCI bus.

Why is it executed for now?

Thanks,
Laurent

Re: [PULL 5/5] m68k: add Virtual M68k Machine

2021-03-18 Thread Philippe Mathieu-Daudé 
Hi Laurent,

+Paolo / Thomas

On 3/15/21 9:42 PM, Laurent Vivier wrote:
> The machine is based on Goldfish interfaces defined by Google
> for Android simulator. It uses Goldfish-rtc (timer and RTC),
> Goldfish-pic (PIC) and Goldfish-tty (for serial port and early tty).
> 
> The machine is created with 128 virtio-mmio bus, and they can
> be used to use serial console, GPU, disk, NIC, HID, ...
> 
> Signed-off-by: Laurent Vivier 
> Reviewed-by: Richard Henderson 
> Tested-by: Philippe Mathieu-Daudé 
> Message-Id: <20210312214145.2936082-6-laur...@vivier.eu>
> ---
>  default-configs/devices/m68k-softmmu.mak  |   1 +
>  .../standard-headers/asm-m68k/bootinfo-virt.h |  18 +
>  hw/m68k/virt.c| 313 ++
>  MAINTAINERS   |  13 +
>  hw/m68k/Kconfig   |   9 +
>  hw/m68k/meson.build   |   1 +
>  6 files changed, 355 insertions(+)
>  create mode 100644 include/standard-headers/asm-m68k/bootinfo-virt.h
>  create mode 100644 hw/m68k/virt.c

> diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
> index 60d7bcfb8f2b..f839f8a03064 100644
> --- a/hw/m68k/Kconfig
> +++ b/hw/m68k/Kconfig
> @@ -23,3 +23,12 @@ config Q800
>  select ESP
>  select DP8393X
>  select OR_IRQ
> +
> +config M68K_VIRT
> +bool
> +select M68K_IRQC
> +select VIRT_CTRL
> +select GOLDFISH_PIC
> +select GOLDFISH_TTY
> +select GOLDFISH_RTC
> +select VIRTIO_MMIO

I had this error on gitlab:

(qemu) QEMU_PROG: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio:
'virtio-blk-pci' is not a valid device model name
job: check-system-fedora
https://gitlab.com/philmd/qemu/-/jobs/1106469724

I bisected locally to this commit.

check-system-fedora uses build-system-fedora:

build-system-fedora:
CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
 --enable-fdt=system --enable-slirp=system
 --enable-capstone=system

I'm confused because the machine provides a VIRTIO bus
via MMIO:

config VIRTIO_MMIO
bool
select VIRTIO

I remember I tested your machine with virtio-blk-device.

config VIRTIO_BLK
bool
default y
depends on VIRTIO

Ah, this is virtio-blk-pci, which has:

virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true:
files('virtio-blk-pci.c'))
virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

And VIRTIO_PCI isn't selected...

Are the tests incorrect then?

libqos isn't restricted to PCI:

tests/qtest/libqos/virtio-blk.c:24:#include "virtio-blk.h"
tests/qtest/libqos/virtio-blk.c:29:/* virtio-blk-device */
tests/qtest/libqos/virtio-blk.c:33:if (!g_strcmp0(interface,
"virtio-blk")) {
tests/qtest/libqos/virtio-blk.c:40:fprintf(stderr, "%s not present
in virtio-blk-device\n", interface);
tests/qtest/libqos/virtio-blk.c:109:/* virtio-blk-device */
tests/qtest/libqos/virtio-blk.c:111:
qos_node_create_driver("virtio-blk-device", virtio_blk_device_create);
tests/qtest/libqos/virtio-blk.c:112:
qos_node_consumes("virtio-blk-device", "virtio-bus", );
tests/qtest/libqos/virtio-blk.c:113:
qos_node_produces("virtio-blk-device", "virtio-blk");

But qemu-iotests / qtests do use virtio-blk-pci. Maybe they should
use a generic virtio-blk-device instead, hoping it get plugged correctly
to the virtio bus...