[PATCH] tests/qtest: Fix boot-serial-test when using --without-default-devices

[PATCH] tests/qtest: Fix boot-serial-test when using --without-default-devices

[Thomas Huth]

If "configure" has been run with "--without-default-devices", there is
no e1000 device in the binaries, so the boot-serial-test currently fails
in that case since it tries to use the e1000 with the sam460ex machine.

Since we're testing the serial output here, and not the NIC, let's
simply switch to the "pci-bridge" device here instead, which should
always be there for PCIe-based machines like the sam460ex.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/boot-serial-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 6dd06aeaf4..e3b7d65fe5 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -156,7 +156,7 @@ static const testdef_t tests[] = {
       "Open Firmware" },
     { "ppc64", "powernv8", "", "OPAL" },
     { "ppc64", "powernv9", "", "OPAL" },
-    { "ppc64", "sam460ex", "-device e1000", "8086  100e" },
+    { "ppc64", "sam460ex", "-device pci-bridge,chassis_nr=2", "1b36  0001" },
     { "i386", "isapc", "-cpu qemu32 -M graphics=off", "SeaBIOS" },
     { "i386", "pc", "-M graphics=off", "SeaBIOS" },
     { "i386", "q35", "-M graphics=off", "SeaBIOS" },
-- 
2.43.2

Re: [PATCH] tests/qtest: Fix boot-serial-test when using --without-default-devices

[BALATON Zoltan]

On Mon, 19 Feb 2024, Thomas Huth wrote:
> If "configure" has been run with "--without-default-devices", there is
> no e1000 device in the binaries, so the boot-serial-test currently fails
> in that case since it tries to use the e1000 with the sam460ex machine.
>
> Since we're testing the serial output here, and not the NIC, let's
> simply switch to the "pci-bridge" device here instead, which should
> always be there for PCIe-based machines like the sam460ex.

It's not actually testing PCIe but PCI bus but I think that does not 
matter. PCIe on sam460ex does not work yet, I've only implemented it 
partially to pass the firmware init but devices attached to the PCIe bus 
probably won't work. I have some patches to improve that but not yet 
ready.

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/qtest/boot-serial-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 6dd06aeaf4..e3b7d65fe5 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -156,7 +156,7 @@ static const testdef_t tests[] = {
>       "Open Firmware" },
>     { "ppc64", "powernv8", "", "OPAL" },
>     { "ppc64", "powernv9", "", "OPAL" },
> -    { "ppc64", "sam460ex", "-device e1000", "8086  100e" },
> +    { "ppc64", "sam460ex", "-device pci-bridge,chassis_nr=2", "1b36  0001" },

So if you want to check if PCI bus works then maybe there's no need to add 
a device at all just look for the sm501 display chip ("126f 0501") that's 
soldered on the board so it's always created even with -nodefaults and 
should always present on sam460ex. The -device option just adds a device 
that appears before the sm501 and stops the test there. Not sure if this 
is testing more than looking for a PCI device created by the board code.

Regards,
BALATON Zoltan

>     { "i386", "isapc", "-cpu qemu32 -M graphics=off", "SeaBIOS" },
>     { "i386", "pc", "-M graphics=off", "SeaBIOS" },
>     { "i386", "q35", "-M graphics=off", "SeaBIOS" },
>

Re: [PATCH] tests/qtest: Fix boot-serial-test when using --without-default-devices

[Thomas Huth]

On 19/02/2024 12.37, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Thomas Huth wrote:
>> If "configure" has been run with "--without-default-devices", there is
>> no e1000 device in the binaries, so the boot-serial-test currently fails
>> in that case since it tries to use the e1000 with the sam460ex machine.
>>
>> Since we're testing the serial output here, and not the NIC, let's
>> simply switch to the "pci-bridge" device here instead, which should
>> always be there for PCIe-based machines like the sam460ex.
> 
> It's not actually testing PCIe but PCI bus but I think that does not matter. 
> PCIe on sam460ex does not work yet, I've only implemented it partially to 
> pass the firmware init but devices attached to the PCIe bus probably won't 
> work. I have some patches to improve that but not yet ready.

Ah, ok, I looked at the Kconfig file and saw the "select PCI_EXPRESS" there 
that got selected by PPC440 (which gets selected by SAM460EX), that's why I 
concluded that it must be "PCIe-based" ... I'll drop the "e" here.

>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> tests/qtest/boot-serial-test.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
>> index 6dd06aeaf4..e3b7d65fe5 100644
>> --- a/tests/qtest/boot-serial-test.c
>> +++ b/tests/qtest/boot-serial-test.c
>> @@ -156,7 +156,7 @@ static const testdef_t tests[] = {
>>       "Open Firmware" },
>>     { "ppc64", "powernv8", "", "OPAL" },
>>     { "ppc64", "powernv9", "", "OPAL" },
>> -    { "ppc64", "sam460ex", "-device e1000", "8086  100e" },
>> +    { "ppc64", "sam460ex", "-device pci-bridge,chassis_nr=2", "1b36  
>> 0001" },
> 
> So if you want to check if PCI bus works then maybe there's no need to add a 
> device at all just look for the sm501 display chip ("126f 0501") that's 
> soldered on the board so it's always created even with -nodefaults and 
> should always present on sam460ex. The -device option just adds a device 
> that appears before the sm501 and stops the test there. Not sure if this is 
> testing more than looking for a PCI device created by the board code.

I was considering the sm501, too, but I thought that we might test a little 
bit more if we check that cold-plugging via "-device" works, too, so I'd 
prefer to keep it this way.

  Thomas

Re: [PATCH] tests/qtest: Fix boot-serial-test when using --without-default-devices

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 2780 bytes --]

On Mon, 19 Feb 2024, Thomas Huth wrote:
> On 19/02/2024 12.37, BALATON Zoltan wrote:
>> On Mon, 19 Feb 2024, Thomas Huth wrote:
>>> If "configure" has been run with "--without-default-devices", there is
>>> no e1000 device in the binaries, so the boot-serial-test currently fails
>>> in that case since it tries to use the e1000 with the sam460ex machine.
>>> 
>>> Since we're testing the serial output here, and not the NIC, let's
>>> simply switch to the "pci-bridge" device here instead, which should
>>> always be there for PCIe-based machines like the sam460ex.
>> 
>> It's not actually testing PCIe but PCI bus but I think that does not 
>> matter. PCIe on sam460ex does not work yet, I've only implemented it 
>> partially to pass the firmware init but devices attached to the PCIe bus 
>> probably won't work. I have some patches to improve that but not yet ready.
>
> Ah, ok, I looked at the Kconfig file and saw the "select PCI_EXPRESS" there 
> that got selected by PPC440 (which gets selected by SAM460EX), that's why I 
> concluded that it must be "PCIe-based" ... I'll drop the "e" here.
>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> tests/qtest/boot-serial-test.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/tests/qtest/boot-serial-test.c 
>>> b/tests/qtest/boot-serial-test.c
>>> index 6dd06aeaf4..e3b7d65fe5 100644
>>> --- a/tests/qtest/boot-serial-test.c
>>> +++ b/tests/qtest/boot-serial-test.c
>>> @@ -156,7 +156,7 @@ static const testdef_t tests[] = {
>>>       "Open Firmware" },
>>>     { "ppc64", "powernv8", "", "OPAL" },
>>>     { "ppc64", "powernv9", "", "OPAL" },
>>> -    { "ppc64", "sam460ex", "-device e1000", "8086  100e" },
>>> +    { "ppc64", "sam460ex", "-device pci-bridge,chassis_nr=2", "1b36 
>>> 0001" },
>> 
>> So if you want to check if PCI bus works then maybe there's no need to add 
>> a device at all just look for the sm501 display chip ("126f 0501") that's 
>> soldered on the board so it's always created even with -nodefaults and 
>> should always present on sam460ex. The -device option just adds a device 
>> that appears before the sm501 and stops the test there. Not sure if this is 
>> testing more than looking for a PCI device created by the board code.
>
> I was considering the sm501, too, but I thought that we might test a little 
> bit more if we check that cold-plugging via "-device" works, too, so I'd 
> prefer to keep it this way.

No problem, if testing -device is adding more coverage than testing for a 
device created by the board then I'm OK with it but not sure these are 
really different other than maybe -device going through options parsing 
but at the end for testing the PCI but they are probably the same.

Regards,
BALATON Zoltan