[PATCH v3 0/3] target/arm: disable-tcg and without-default-devices fixes

[PATCH v3 0/3] target/arm: disable-tcg and without-default-devices fixes

[Fabiano Rosas]

Changed the cdrom test to apply to only the x86 and s390x cdrom boot
tests.

CI run: https://gitlab.com/farosas/qemu/-/pipelines/860488769

v2:
https://lore.kernel.org/r/20230505123524.23401-1-farosas@suse.de

v1:
https://lore.kernel.org/r/20230503193833.29047-1-farosas@suse.de

Here's the fix for the cdrom test failure that we discussed in the
list, plus 2 fixes for the ---without-default-devices build.

When I moved the boards CONFIGs from default.mak to Kconfig, it became
possible (due to --without-default-devices) to disable the CONFIGs for
all the boards that require ARM_V7M. That breaks the build because
ARM_V7M is required to be always set.

Fabiano Rosas (3):
  target/arm: Select SEMIHOSTING when using TCG
  target/arm: Select CONFIG_ARM_V7M when TCG is enabled
  tests/qtest: Don't run cdrom boot tests if no accelerator is present

 target/arm/Kconfig       |  9 ++-------
 tests/qtest/cdrom-test.c | 10 ++++++++++
 2 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.35.3

[PATCH v3 1/3] target/arm: Select SEMIHOSTING when using TCG

[Fabiano Rosas]

Semihosting has been made a 'default y' entry in Kconfig, which does
not work because when building --without-default-devices, the
semihosting code would not be available.

Make semihosting unconditional when TCG is present.

Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 target/arm/Kconfig | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/target/arm/Kconfig b/target/arm/Kconfig
index 39f05b6420..3fffdcb61b 100644
--- a/target/arm/Kconfig
+++ b/target/arm/Kconfig
@@ -1,13 +1,7 @@
 config ARM
     bool
+    select ARM_COMPATIBLE_SEMIHOSTING if TCG
 
 config AARCH64
     bool
     select ARM
-
-# This config exists just so we can make SEMIHOSTING default when TCG
-# is selected without also changing it for other architectures.
-config ARM_SEMIHOSTING
-    bool
-    default y if TCG && ARM
-    select ARM_COMPATIBLE_SEMIHOSTING
-- 
2.35.3

Re: [PATCH v3 1/3] target/arm: Select SEMIHOSTING when using TCG

[Richard Henderson]

On 5/8/23 19:16, Fabiano Rosas wrote:
> Semihosting has been made a 'default y' entry in Kconfig, which does
> not work because when building --without-default-devices, the
> semihosting code would not be available.
> 
> Make semihosting unconditional when TCG is present.
> 
> Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build")
> Signed-off-by: Fabiano Rosas<farosas@suse.de>
> ---
>   target/arm/Kconfig | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v3 2/3] target/arm: Select CONFIG_ARM_V7M when TCG is enabled

[Fabiano Rosas]

We cannot allow this config to be disabled at the moment as not all of
the relevant code is protected by it.

Commit 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a
KVM-only build") moved the CONFIGs of several boards to Kconfig, so it
is now possible that nothing selects ARM_V7M (e.g. when doing a
--without-default-devices build).

Return the CONFIG_ARM_V7M entry to a state where it is always selected
whenever TCG is available.

Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 target/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/Kconfig b/target/arm/Kconfig
index 3fffdcb61b..5947366f6e 100644
--- a/target/arm/Kconfig
+++ b/target/arm/Kconfig
@@ -1,6 +1,7 @@
 config ARM
     bool
     select ARM_COMPATIBLE_SEMIHOSTING if TCG
+    select ARM_V7M if TCG
 
 config AARCH64
     bool
-- 
2.35.3

Re: [PATCH v3 2/3] target/arm: Select CONFIG_ARM_V7M when TCG is enabled

[Richard Henderson]

On 5/8/23 19:16, Fabiano Rosas wrote:
> We cannot allow this config to be disabled at the moment as not all of
> the relevant code is protected by it.
> 
> Commit 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a
> KVM-only build") moved the CONFIGs of several boards to Kconfig, so it
> is now possible that nothing selects ARM_V7M (e.g. when doing a
> --without-default-devices build).
> 
> Return the CONFIG_ARM_V7M entry to a state where it is always selected
> whenever TCG is available.
> 
> Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build")
> Signed-off-by: Fabiano Rosas<farosas@suse.de>
> ---
>   target/arm/Kconfig | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 2/3] target/arm: Select CONFIG_ARM_V7M when TCG is enabled

[Philippe Mathieu-Daudé]

On 8/5/23 20:16, Fabiano Rosas wrote:
> We cannot allow this config to be disabled at the moment as not all of
> the relevant code is protected by it.
> 
> Commit 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a
> KVM-only build") moved the CONFIGs of several boards to Kconfig, so it
> is now possible that nothing selects ARM_V7M (e.g. when doing a
> --without-default-devices build).
> 
> Return the CONFIG_ARM_V7M entry to a state where it is always selected
> whenever TCG is available.
> 
> Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build")
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   target/arm/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index 3fffdcb61b..5947366f6e 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -1,6 +1,7 @@
>   config ARM
>       bool
>       select ARM_COMPATIBLE_SEMIHOSTING if TCG
> +    select ARM_V7M if TCG

Probably worth a comment mentioning this is temporarily
required until <some magic happens>, so we won't forgot
to remove it.

>   
>   config AARCH64
>       bool

Re: [PATCH v3 2/3] target/arm: Select CONFIG_ARM_V7M when TCG is enabled

[Paolo Bonzini]

On 5/9/23 16:49, Philippe Mathieu-Daudé wrote:
> On 8/5/23 20:16, Fabiano Rosas wrote:
>> We cannot allow this config to be disabled at the moment as not all of
>> the relevant code is protected by it.
>>
>> Commit 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a
>> KVM-only build") moved the CONFIGs of several boards to Kconfig, so it
>> is now possible that nothing selects ARM_V7M (e.g. when doing a
>> --without-default-devices build).
>>
>> Return the CONFIG_ARM_V7M entry to a state where it is always selected
>> whenever TCG is available.
>>
>> Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a 
>> KVM-only build")
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>   target/arm/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
>> index 3fffdcb61b..5947366f6e 100644
>> --- a/target/arm/Kconfig
>> +++ b/target/arm/Kconfig
>> @@ -1,6 +1,7 @@
>>   config ARM
>>       bool
>>       select ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_V7M if TCG
> 
> Probably worth a comment mentioning this is temporarily
> required until <some magic happens>, so we won't forgot
> to remove it.

Yeah, this one should in principle be defined by the boards, but 
m_helper.c is included unconditionally instead of having some kind of 
stub for A-only boards.

Related to this is the (right now unconditional, later on only "if TCG") 
"select ARM_GICV3_TCG" that needs to be added under ARM_GIC.

Paolo

[PATCH v3 3/3] tests/qtest: Don't run cdrom boot tests if no accelerator is present

[Fabiano Rosas]

On a build configured with: --disable-tcg --enable-xen it is possible
to produce a QEMU binary with no TCG nor KVM support. Skip the cdrom
boot tests if that's the case.

Fixes: 0c1ae3ff9d ("tests/qtest: Fix tests when no KVM or TCG are present")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/cdrom-test.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 26a2400181..31d3bacd8c 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -130,6 +130,11 @@ static void test_cdboot(gconstpointer data)
 
 static void add_x86_tests(void)
 {
+    if (!qtest_has_accel("tcg") && !qtest_has_accel("kvm")) {
+        g_test_skip("No KVM or TCG accelerator available, skipping boot tests");
+        return;
+    }
+
     qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
     qtest_add_data_func("cdrom/boot/virtio-scsi",
                         "-device virtio-scsi -device scsi-cd,drive=cdr "
@@ -176,6 +181,11 @@ static void add_x86_tests(void)
 
 static void add_s390x_tests(void)
 {
+    if (!qtest_has_accel("tcg") && !qtest_has_accel("kvm")) {
+        g_test_skip("No KVM or TCG accelerator available, skipping boot tests");
+        return;
+    }
+
     qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
     qtest_add_data_func("cdrom/boot/virtio-scsi",
                         "-device virtio-scsi -device scsi-cd,drive=cdr "
-- 
2.35.3

Re: [PATCH v3 3/3] tests/qtest: Don't run cdrom boot tests if no accelerator is present

[Thomas Huth]

On 08/05/2023 20.16, Fabiano Rosas wrote:
> On a build configured with: --disable-tcg --enable-xen it is possible
> to produce a QEMU binary with no TCG nor KVM support. Skip the cdrom
> boot tests if that's the case.
> 
> Fixes: 0c1ae3ff9d ("tests/qtest: Fix tests when no KVM or TCG are present")
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/cdrom-test.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 26a2400181..31d3bacd8c 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -130,6 +130,11 @@ static void test_cdboot(gconstpointer data)
>   
>   static void add_x86_tests(void)
>   {
> +    if (!qtest_has_accel("tcg") && !qtest_has_accel("kvm")) {
> +        g_test_skip("No KVM or TCG accelerator available, skipping boot tests");
> +        return;
> +    }
> +
>       qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
>       qtest_add_data_func("cdrom/boot/virtio-scsi",
>                           "-device virtio-scsi -device scsi-cd,drive=cdr "
> @@ -176,6 +181,11 @@ static void add_x86_tests(void)
>   
>   static void add_s390x_tests(void)
>   {
> +    if (!qtest_has_accel("tcg") && !qtest_has_accel("kvm")) {
> +        g_test_skip("No KVM or TCG accelerator available, skipping boot tests");
> +        return;
> +    }
> +
>       qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
>       qtest_add_data_func("cdrom/boot/virtio-scsi",
>                           "-device virtio-scsi -device scsi-cd,drive=cdr "

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v3 0/3] target/arm: disable-tcg and without-default-devices fixes

[Peter Maydell]

On Mon, 8 May 2023 at 19:16, Fabiano Rosas <farosas@suse.de> wrote:
>
> Changed the cdrom test to apply to only the x86 and s390x cdrom boot
> tests.
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/860488769
>
> v2:
> https://lore.kernel.org/r/20230505123524.23401-1-farosas@suse.de
>
> v1:
> https://lore.kernel.org/r/20230503193833.29047-1-farosas@suse.de
>
> Here's the fix for the cdrom test failure that we discussed in the
> list, plus 2 fixes for the ---without-default-devices build.
>
> When I moved the boards CONFIGs from default.mak to Kconfig, it became
> possible (due to --without-default-devices) to disable the CONFIGs for
> all the boards that require ARM_V7M. That breaks the build because
> ARM_V7M is required to be always set.



Applied to target-arm.next, thanks.

-- PMM