Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel

Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel

[Claudio Fontana]

On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
> 
> Only enable the following ARMv6 CPUs when TCG is available:
> 
>   - ARM1136
>   - ARM1176
>   - ARM11MPCore
>   - Cortex-M0
> 
> The following machines are no more built when TCG is disabled:
> 
>   - kzm                  ARM KZM Emulation Baseboard (ARM1136)
>   - microbit             BBC micro:bit (Cortex-M0)
>   - n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
>   - n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
>   - realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/devices/arm-softmmu.mak | 2 --
>  hw/arm/realview.c                       | 2 +-
>  tests/qtest/cdrom-test.c                | 2 +-
>  hw/arm/Kconfig                          | 6 ++++++
>  target/arm/Kconfig                      | 4 ++++
>  5 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
> index 0aad35da0c4..175530595ce 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -10,9 +10,7 @@ CONFIG_ARM_VIRT=y
>  CONFIG_CUBIEBOARD=y
>  CONFIG_EXYNOS4=y
>  CONFIG_HIGHBANK=y
> -CONFIG_FSL_IMX31=y
>  CONFIG_MUSCA=y
> -CONFIG_NSERIES=y
>  CONFIG_STELLARIS=y
>  CONFIG_REALVIEW=y
>  CONFIG_VEXPRESS=y
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 2dcf0a4c23e..0606d22da14 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -463,8 +463,8 @@ static void realview_machine_init(void)
>  {
>      if (tcg_builtin()) {
>          type_register_static(&realview_eb_type);
> +        type_register_static(&realview_eb_mpcore_type);
>      }
> -    type_register_static(&realview_eb_mpcore_type);
>      type_register_static(&realview_pb_a8_type);
>      type_register_static(&realview_pbx_a9_type);
>  }
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 1f1bc26fa7a..cb0409c5a11 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -224,8 +224,8 @@ int main(int argc, char **argv)
>          const char *armmachines[] = {
>  #ifdef CONFIG_TCG
>              "realview-eb",
> -#endif /* CONFIG_TCG */
>              "realview-eb-mpcore",
> +#endif /* CONFIG_TCG */
>              "realview-pb-a8",
>              "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
>              "vexpress-a9", "virt", NULL
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 560442bfc5c..6c4bce4d637 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -123,6 +123,8 @@ config NETDUINOPLUS2
>  
>  config NSERIES
>      bool
> +    default y if TCG && ARM
> +    select ARM_V6
>      select OMAP
>      select TMP105   # tempature sensor
>      select BLIZZARD # LCD/TV controller
> @@ -401,6 +403,8 @@ config FSL_IMX25
>  
>  config FSL_IMX31
>      bool
> +    default y if TCG && ARM
> +    select ARM_V6
>      select SERIAL
>      select IMX
>      select IMX_I2C
> @@ -478,11 +482,13 @@ config FSL_IMX6UL
>  
>  config MICROBIT
>      bool
> +    default y if TCG && ARM
>      select NRF51_SOC
>  
>  config NRF51_SOC
>      bool
>      select I2C
> +    select ARM_V6
>      select ARM_V7M
>      select UNIMP
>  
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index 9b3635617dc..fbb7bba9018 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -14,6 +14,10 @@ config ARM_V5
>      bool
>      depends on TCG && ARM
>  
> +config ARM_V6
> +    bool
> +    depends on TCG && ARM
> +
>  config ARM_V7M
>      bool
>      select PTIMER
> 

Added Cc: Eduardo,

Looks good to me in general,

Acked-by: Claudio Fontana <cfontana@suse.de>

I am just wondering about that if (tcg_builtin()) / (or _available() as I suggest elsewhere), 
should we instead use the build system already at this stage, so no such check is necessary?

It could be a successive change, but then tcg_builtin() would be introduced, only to become useless after the proper refactoring is done,
and the build system is used to select the right modules to compile, which would do the registration.

Ciao,

Claudio

Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
>
> Only enable the following ARMv6 CPUs when TCG is available:
>
>   - ARM1136
>   - ARM1176
>   - ARM11MPCore
>   - Cortex-M0
>
> The following machines are no more built when TCG is disabled:
>
>   - kzm                  ARM KZM Emulation Baseboard (ARM1136)
>   - microbit             BBC micro:bit (Cortex-M0)
>   - n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
>   - n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
>   - realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/devices/arm-softmmu.mak | 2 --
>  hw/arm/realview.c                       | 2 +-
>  tests/qtest/cdrom-test.c                | 2 +-
>  hw/arm/Kconfig                          | 6 ++++++
>  target/arm/Kconfig                      | 4 ++++
>  5 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
> index 0aad35da0c4..175530595ce 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -10,9 +10,7 @@ CONFIG_ARM_VIRT=y
>  CONFIG_CUBIEBOARD=y
>  CONFIG_EXYNOS4=y
>  CONFIG_HIGHBANK=y
> -CONFIG_FSL_IMX31=y
>  CONFIG_MUSCA=y
> -CONFIG_NSERIES=y
>  CONFIG_STELLARIS=y
>  CONFIG_REALVIEW=y
>  CONFIG_VEXPRESS=y
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 2dcf0a4c23e..0606d22da14 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -463,8 +463,8 @@ static void realview_machine_init(void)
>  {
>      if (tcg_builtin()) {
>          type_register_static(&realview_eb_type);
> +        type_register_static(&realview_eb_mpcore_type);
>      }
> -    type_register_static(&realview_eb_mpcore_type);
>      type_register_static(&realview_pb_a8_type);
>      type_register_static(&realview_pbx_a9_type);
>  }

This confuses me - are we even able to run a realview image under KVM?
Surely the whole of realview should be TCG only?

The rest looks fine to me though:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v6 00/11] Support disabling TCG on ARM (part 2)

[Claudio Fontana]

On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
> Cover from Samuel Ortiz from (part 1) [1]:
> 
>   This patchset allows for building and running ARM targets with TCG
>   disabled. [...]
> 
>   The rationale behind this work comes from the NEMU project where
>   we're trying to only support x86 and ARM 64-bit architectures,
>   without including the TCG code base. We can only do so if we can
>   build and run ARM binaries with TCG disabled.
> 
> Peter mentioned in v5 [6] that since 32-bit host has been removed,
> we have to remove v7 targets. This is not done in this series, as
> linking succeeds, and there is enough material to review (no need
> to spend time on that extra patch if the current approach is not
> accepted).
> 
> CI: https://gitlab.com/philmd/qemu/-/pipelines/249272441
> 
> v6:
> - rebased on "target/arm/Kconfig" series
> - introduce/use tcg_builtin() for realview machines
> 
> v5:
> - addressed Paolo/Richard/Thomas review comments from v4 [5].
> 
> v4 almost 2 years later... [2]:
> - Rebased on Meson
> - Addressed Richard review comments
> - Addressed Claudio review comments
> 
> v3 almost 18 months later [3]:
> - Rebased
> - Addressed Thomas review comments
> - Added Travis-CI job to keep building --disable-tcg on ARM
> 
> v2 [4]:
> - Addressed review comments from Richard and Thomas from v1 [1]
> 
> Regards,
> 
> Phil.
> 
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html
> [2]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg689168.html
> [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg641796.html
> [4]: https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05003.html
> [5]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg746041.html
> [6]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg777669.html
> 
> Based-on: <20210131111316.232778-1-f4bug@amsat.org>
>           "target: Provide target-specific Kconfig"
> 
> Philippe Mathieu-Daudé (9):
>   sysemu/tcg: Introduce tcg_builtin() helper
>   exec: Restrict TCG specific headers
>   target/arm: Restrict ARMv4 cpus to TCG accel
>   target/arm: Restrict ARMv5 cpus to TCG accel
>   target/arm: Restrict ARMv6 cpus to TCG accel
>   target/arm: Restrict ARMv7 R-profile cpus to TCG accel
>   target/arm: Restrict ARMv7 M-profile cpus to TCG accel
>   target/arm: Reorder meson.build rules
>   .travis.yml: Add a KVM-only Aarch64 job
> 
> Samuel Ortiz (1):
>   target/arm: Do not build TCG objects when TCG is off
> 
> Thomas Huth (1):
>   target/arm: Make m_helper.c optional via CONFIG_ARM_V7M
> 
>  default-configs/devices/aarch64-softmmu.mak |  1 -
>  default-configs/devices/arm-softmmu.mak     | 27 --------
>  include/exec/helper-proto.h                 |  2 +
>  include/sysemu/tcg.h                        |  2 +
>  target/arm/cpu.h                            | 12 ----
>  hw/arm/realview.c                           |  7 +-
>  target/arm/cpu_tcg.c                        |  4 +-
>  target/arm/helper.c                         |  7 --
>  target/arm/m_helper-stub.c                  | 73 +++++++++++++++++++++
>  tests/qtest/cdrom-test.c                    |  6 +-
>  .travis.yml                                 | 32 +++++++++
>  hw/arm/Kconfig                              | 38 +++++++++++
>  target/arm/Kconfig                          | 17 +++++
>  target/arm/meson.build                      | 28 +++++---
>  14 files changed, 196 insertions(+), 60 deletions(-)
>  create mode 100644 target/arm/m_helper-stub.c
> 

Looking at this series, just my 2 cents on how I would suggest to go forward:
I could again split my series in two parts, with only the TCG Ops in the first part.

Then this series could be merged, enabling --disable-tcg for ARM,

then I could extend the second part of my series to include ARM as well.

Wdyt? (Probably Richard?)

Thanks,

Claudio

Re: [PATCH v6 00/11] Support disabling TCG on ARM (part 2)

[Philippe Mathieu-Daudé]

On 1/31/21 3:40 PM, Claudio Fontana wrote:
> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>> Cover from Samuel Ortiz from (part 1) [1]:
>>
>>   This patchset allows for building and running ARM targets with TCG
>>   disabled. [...]
>>
>>   The rationale behind this work comes from the NEMU project where
>>   we're trying to only support x86 and ARM 64-bit architectures,
>>   without including the TCG code base. We can only do so if we can
>>   build and run ARM binaries with TCG disabled.
>>
>> Peter mentioned in v5 [6] that since 32-bit host has been removed,
>> we have to remove v7 targets. This is not done in this series, as
>> linking succeeds, and there is enough material to review (no need
>> to spend time on that extra patch if the current approach is not
>> accepted).
>>
>> CI: https://gitlab.com/philmd/qemu/-/pipelines/249272441
>>
>> v6:
>> - rebased on "target/arm/Kconfig" series
>> - introduce/use tcg_builtin() for realview machines
>>
>> v5:
>> - addressed Paolo/Richard/Thomas review comments from v4 [5].
>>
>> v4 almost 2 years later... [2]:
>> - Rebased on Meson
>> - Addressed Richard review comments
>> - Addressed Claudio review comments
>>
>> v3 almost 18 months later [3]:
>> - Rebased
>> - Addressed Thomas review comments
>> - Added Travis-CI job to keep building --disable-tcg on ARM
>>
>> v2 [4]:
>> - Addressed review comments from Richard and Thomas from v1 [1]
>>
>> Regards,
>>
>> Phil.
>>
>> [1]: https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html
>> [2]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg689168.html
>> [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg641796.html
>> [4]: https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05003.html
>> [5]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg746041.html
>> [6]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg777669.html
>>
>> Based-on: <20210131111316.232778-1-f4bug@amsat.org>
>>           "target: Provide target-specific Kconfig"
>>
>> Philippe Mathieu-Daudé (9):
>>   sysemu/tcg: Introduce tcg_builtin() helper
>>   exec: Restrict TCG specific headers
>>   target/arm: Restrict ARMv4 cpus to TCG accel
>>   target/arm: Restrict ARMv5 cpus to TCG accel
>>   target/arm: Restrict ARMv6 cpus to TCG accel
>>   target/arm: Restrict ARMv7 R-profile cpus to TCG accel
>>   target/arm: Restrict ARMv7 M-profile cpus to TCG accel
>>   target/arm: Reorder meson.build rules
>>   .travis.yml: Add a KVM-only Aarch64 job
>>
>> Samuel Ortiz (1):
>>   target/arm: Do not build TCG objects when TCG is off
>>
>> Thomas Huth (1):
>>   target/arm: Make m_helper.c optional via CONFIG_ARM_V7M
>>
>>  default-configs/devices/aarch64-softmmu.mak |  1 -
>>  default-configs/devices/arm-softmmu.mak     | 27 --------
>>  include/exec/helper-proto.h                 |  2 +
>>  include/sysemu/tcg.h                        |  2 +
>>  target/arm/cpu.h                            | 12 ----
>>  hw/arm/realview.c                           |  7 +-
>>  target/arm/cpu_tcg.c                        |  4 +-
>>  target/arm/helper.c                         |  7 --
>>  target/arm/m_helper-stub.c                  | 73 +++++++++++++++++++++
>>  tests/qtest/cdrom-test.c                    |  6 +-
>>  .travis.yml                                 | 32 +++++++++
>>  hw/arm/Kconfig                              | 38 +++++++++++
>>  target/arm/Kconfig                          | 17 +++++
>>  target/arm/meson.build                      | 28 +++++---
>>  14 files changed, 196 insertions(+), 60 deletions(-)
>>  create mode 100644 target/arm/m_helper-stub.c
>>
> 
> Looking at this series, just my 2 cents on how I would suggest to go forward:
> I could again split my series in two parts, with only the TCG Ops in the first part.
> 
> Then this series could be merged, enabling --disable-tcg for ARM,
> 
> then I could extend the second part of my series to include ARM as well.
> 
> Wdyt? (Probably Richard?)

¯\_(ツ)_/¯

I respun because Richard unqueue your series, and it looks
there is no big clashing.

Anyhow meanwhile peer review is useful, and thanks for yours ;)

> 
> Thanks,
> 
> Claudio
> 
> 
> 
> 

Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper

[Philippe Mathieu-Daudé]

On 1/31/21 3:18 PM, Claudio Fontana wrote:
> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>> Modules are registered early with type_register_static().
>>
>> We would like to call tcg_enabled() when registering QOM types,
> 
> 
> Hi Philippe,
> 
> could this not be controlled by meson at this stage?
> On X86, I register the tcg-specific types in tcg/* in modules that are only built for TCG.
> 
> Maybe tcg_builtin() is useful anyway, thinking long term at loadable modules,
> but there we are interested in whether tcg code is available or not, regardless of whether it's builtin,
> or needs to be loaded via a .so plugin..
> 
> maybe tcg_available()?

The alternatives I found:

- reorder things in vl.c?

- use ugly #ifdef'ry, see this patch:
  https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg08037.html

- this earlier approach I previously discarded:

-- >8 --
diff --git a/include/qom/object.h b/include/qom/object.h
index d378f13a116..30590c6fac3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -403,9 +403,12 @@ struct Object
  *   parent class initialization has occurred, but before the class itself
  *   is initialized.  This is the function to use to undo the effects of
  *   memcpy from the parent class to the descendants.
- * @class_data: Data to pass to the @class_init,
+ * @class_data: Data to pass to the @class_registerable, @class_init,
  *   @class_base_init. This can be useful when building dynamic
  *   classes.
+ * @registerable: This function is called when modules are registered,
+ *   prior to any class initialization. When present and returning %false,
+ *   the type is not registered, the class is not present (not usable).
  * @interfaces: The list of interfaces associated with this type.  This
  *   should point to a static array that's terminated with a zero filled
  *   element.
@@ -428,6 +431,7 @@ struct TypeInfo
     void (*class_base_init)(ObjectClass *klass, void *data);
     void *class_data;

+    bool (*registerable)(void *data);
     InterfaceInfo *interfaces;
 };

diff --git a/qom/object.c b/qom/object.c
index 2fa0119647c..0febaffa12e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -138,6 +138,10 @@ static TypeImpl *type_new(const TypeInfo *info)
 static TypeImpl *type_register_internal(const TypeInfo *info)
 {
     TypeImpl *ti;
+
+    if (info->registerable && !info->registerable(info->class_data)) {
+        return NULL;
+    }
     ti = type_new(info);

     type_table_add(ti);

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 990509d3852..1a2b1889da4 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -24,6 +24,7 @@
 #include "hw/loader.h"
 #include "hw/arm/boot.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/tcg.h"
 #include "qom/object.h"

 #define SMPBOOT_ADDR    0x300 /* this should leave enough space for
ATAGS */
@@ -368,18 +369,26 @@ static void raspi3b_machine_class_init(ObjectClass
*oc, void *data)
 };
 #endif /* TARGET_AARCH64 */

+static bool raspi_machine_requiring_tcg_accel(void *data)
+{
+    return tcg_builtin();
+}
+
 static const TypeInfo raspi_machine_types[] = {
     {
         .name           = MACHINE_TYPE_NAME("raspi0"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi0_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi1ap"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi1ap_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi2b"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi2b_machine_class_init,
 #ifdef TARGET_AARCH64
     }, {
---

> 
> Ciao,
> 
> Claudio
> 
>> but tcg_enabled() returns tcg_allowed which is a runtime property
>> initialized later (See commit 2f181fbd5a9 which introduced the
>> MachineInitPhase in "hw/qdev-core.h" representing the different
>> phases of machine initialization and commit 0427b6257e2 which
>> document the initialization order).
>>
>> As we are only interested if the TCG accelerator is builtin,
>> regardless of being enabled, introduce the tcg_builtin() helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Cc: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/sysemu/tcg.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
>> index 00349fb18a7..6ac5c2ca89d 100644
>> --- a/include/sysemu/tcg.h
>> +++ b/include/sysemu/tcg.h
>> @@ -13,8 +13,10 @@ void tcg_exec_init(unsigned long tb_size, int splitwx);
>>  #ifdef CONFIG_TCG
>>  extern bool tcg_allowed;
>>  #define tcg_enabled() (tcg_allowed)
>> +#define tcg_builtin() 1
>>  #else
>>  #define tcg_enabled() 0
>> +#define tcg_builtin() 0
>>  #endif
>>  
>>  #endif
>>
> 

Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper

[Claudio Fontana]

On 1/31/21 4:23 PM, Philippe Mathieu-Daudé wrote:
> On 1/31/21 3:18 PM, Claudio Fontana wrote:
>> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>>> Modules are registered early with type_register_static().
>>>
>>> We would like to call tcg_enabled() when registering QOM types,
>>
>>
>> Hi Philippe,
>>
>> could this not be controlled by meson at this stage?
>> On X86, I register the tcg-specific types in tcg/* in modules that are only built for TCG.
>>
>> Maybe tcg_builtin() is useful anyway, thinking long term at loadable modules,
>> but there we are interested in whether tcg code is available or not, regardless of whether it's builtin,
>> or needs to be loaded via a .so plugin..
>>
>> maybe tcg_available()?
> 
> The alternatives I found:
> 
> - reorder things in vl.c?
> 
> - use ugly #ifdef'ry, see this patch:
>   https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg08037.html

Not sure it's that ugly,
if it is followed (or replaced by) exporting those pieces to separate files, which are only built by meson on CONFIG_TCG.

I did not try to do it, so you know best of course.

Ciao,

Claudio

> 
> - this earlier approach I previously discarded:
> 
> -- >8 --
> diff --git a/include/qom/object.h b/include/qom/object.h
> index d378f13a116..30590c6fac3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -403,9 +403,12 @@ struct Object
>   *   parent class initialization has occurred, but before the class itself
>   *   is initialized.  This is the function to use to undo the effects of
>   *   memcpy from the parent class to the descendants.
> - * @class_data: Data to pass to the @class_init,
> + * @class_data: Data to pass to the @class_registerable, @class_init,
>   *   @class_base_init. This can be useful when building dynamic
>   *   classes.
> + * @registerable: This function is called when modules are registered,
> + *   prior to any class initialization. When present and returning %false,
> + *   the type is not registered, the class is not present (not usable).
>   * @interfaces: The list of interfaces associated with this type.  This
>   *   should point to a static array that's terminated with a zero filled
>   *   element.
> @@ -428,6 +431,7 @@ struct TypeInfo
>      void (*class_base_init)(ObjectClass *klass, void *data);
>      void *class_data;
> 
> +    bool (*registerable)(void *data);
>      InterfaceInfo *interfaces;
>  };
> 
> diff --git a/qom/object.c b/qom/object.c
> index 2fa0119647c..0febaffa12e 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -138,6 +138,10 @@ static TypeImpl *type_new(const TypeInfo *info)
>  static TypeImpl *type_register_internal(const TypeInfo *info)
>  {
>      TypeImpl *ti;
> +
> +    if (info->registerable && !info->registerable(info->class_data)) {
> +        return NULL;
> +    }
>      ti = type_new(info);
> 
>      type_table_add(ti);
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 990509d3852..1a2b1889da4 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -24,6 +24,7 @@
>  #include "hw/loader.h"
>  #include "hw/arm/boot.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/tcg.h"
>  #include "qom/object.h"
> 
>  #define SMPBOOT_ADDR    0x300 /* this should leave enough space for
> ATAGS */
> @@ -368,18 +369,26 @@ static void raspi3b_machine_class_init(ObjectClass
> *oc, void *data)
>  };
>  #endif /* TARGET_AARCH64 */
> 
> +static bool raspi_machine_requiring_tcg_accel(void *data)
> +{
> +    return tcg_builtin();
> +}
> +
>  static const TypeInfo raspi_machine_types[] = {
>      {
>          .name           = MACHINE_TYPE_NAME("raspi0"),
>          .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = raspi_machine_requiring_tcg_accel,
>          .class_init     = raspi0_machine_class_init,
>      }, {
>          .name           = MACHINE_TYPE_NAME("raspi1ap"),
>          .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = raspi_machine_requiring_tcg_accel,
>          .class_init     = raspi1ap_machine_class_init,
>      }, {
>          .name           = MACHINE_TYPE_NAME("raspi2b"),
>          .parent         = TYPE_RASPI_MACHINE,
> +        .registerable   = raspi_machine_requiring_tcg_accel,
>          .class_init     = raspi2b_machine_class_init,
>  #ifdef TARGET_AARCH64
>      }, {
> ---
> 
>>
>> Ciao,
>>
>> Claudio
>>
>>> but tcg_enabled() returns tcg_allowed which is a runtime property
>>> initialized later (See commit 2f181fbd5a9 which introduced the
>>> MachineInitPhase in "hw/qdev-core.h" representing the different
>>> phases of machine initialization and commit 0427b6257e2 which
>>> document the initialization order).
>>>
>>> As we are only interested if the TCG accelerator is builtin,
>>> regardless of being enabled, introduce the tcg_builtin() helper.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  include/sysemu/tcg.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
>>> index 00349fb18a7..6ac5c2ca89d 100644
>>> --- a/include/sysemu/tcg.h
>>> +++ b/include/sysemu/tcg.h
>>> @@ -13,8 +13,10 @@ void tcg_exec_init(unsigned long tb_size, int splitwx);
>>>  #ifdef CONFIG_TCG
>>>  extern bool tcg_allowed;
>>>  #define tcg_enabled() (tcg_allowed)
>>> +#define tcg_builtin() 1
>>>  #else
>>>  #define tcg_enabled() 0
>>> +#define tcg_builtin() 0
>>>  #endif
>>>  
>>>  #endif
>>>
>>
> 

Re: [PATCH v6 02/11] exec: Restrict TCG specific headers

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Fixes when building with --disable-tcg on ARM:
>
>   In file included from target/arm/helper.c:16:
>   include/exec/helper-proto.h:42:10: fatal error: tcg-runtime.h: No such file or directory
>      42 | #include "tcg-runtime.h"
>         |          ^~~~~~~~~~~~~~~

I think the problem here is that we have stuff in helper.c which is
needed by non-TCG builds.

>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/exec/helper-proto.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/exec/helper-proto.h b/include/exec/helper-proto.h
> index 659f9298e8f..740bff3bb4d 100644
> --- a/include/exec/helper-proto.h
> +++ b/include/exec/helper-proto.h
> @@ -39,8 +39,10 @@ dh_ctype(ret) HELPER(name) (dh_ctype(t1), dh_ctype(t2), dh_ctype(t3), \
>  
>  #include "helper.h"
>  #include "trace/generated-helpers.h"
> +#ifdef CONFIG_TCG
>  #include "tcg-runtime.h"
>  #include "plugin-helpers.h"
> +#endif /* CONFIG_TCG */

If we are including helper-proto.h then we are defining helpers which
are (should be) TCG only.

>  
>  #undef IN_HELPER_PROTO


-- 
Alex Bennée

Re: [PATCH v6 06/11] target/arm: Restrict ARMv7 R-profile cpus to TCG accel

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>> KVM requires the target cpu to be at least ARMv8 architecture
>> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
>> "target/arm: Remove KVM support for 32-bit Arm hosts").
>> 
>> Beside, KVM only supports A-profile, thus won't be able to run
>> R-profile cpus.
>> 
>> Only enable the following ARMv7 R-Profile CPUs when TCG is available:
>> 
>>   - Cortex-R5
>>   - Cortex-R5F
>> 
>> The following machine is no more built when TCG is disabled:
>> 
>>   - xlnx-zcu102          Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5Fs
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  default-configs/devices/aarch64-softmmu.mak | 1 -
>>  hw/arm/Kconfig                              | 2 ++
>>  target/arm/Kconfig                          | 4 ++++
>>  3 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/default-configs/devices/aarch64-softmmu.mak b/default-configs/devices/aarch64-softmmu.mak
>> index 958b1e08e40..a4202f56817 100644
>> --- a/default-configs/devices/aarch64-softmmu.mak
>> +++ b/default-configs/devices/aarch64-softmmu.mak
>> @@ -3,6 +3,5 @@
>>  # We support all the 32 bit boards so need all their config
>>  include arm-softmmu.mak
>>  
>> -CONFIG_XLNX_ZYNQMP_ARM=y
>>  CONFIG_XLNX_VERSAL=y
>>  CONFIG_SBSA_REF=y
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 6c4bce4d637..4baf1f97694 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -360,8 +360,10 @@ config STM32F405_SOC
>>  
>>  config XLNX_ZYNQMP_ARM
>>      bool
>> +    default y if TCG && ARM
>
> The correct line is:
>
>       "default y if TCG && AARCH64"

Ahh yes, TIL we had some R-profile cores in QEMU ;-)

with the fix:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v6 03/11] target/arm: Restrict ARMv4 cpus to TCG accel

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
>
> Only enable the following ARMv4 CPUs when TCG is available:
>
>   - StrongARM (SA1100/1110)
>   - OMAP1510 (TI925T)
>
> The following machines are no more built when TCG is disabled:
>
>   - cheetah              Palm Tungsten|E aka. Cheetah PDA (OMAP310)
>   - sx1                  Siemens SX1 (OMAP310) V2
>   - sx1-v1               Siemens SX1 (OMAP310) V1
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v6 03/11] target/arm: Restrict ARMv4 cpus to TCG accel

[Claudio Fontana]

Hi,

I am trying to take these patches,
in the hope that they help with some of the test issues I am having with the kvm-only build,

but they fail with:

target/arm/Kconfig: does not exist in index

so I guess I need the "target/arm/Kconfig" series right, how can I find that one?

Thanks,

Claudio



On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
> 
> Only enable the following ARMv4 CPUs when TCG is available:
> 
>   - StrongARM (SA1100/1110)
>   - OMAP1510 (TI925T)
> 
> The following machines are no more built when TCG is disabled:
> 
>   - cheetah              Palm Tungsten|E aka. Cheetah PDA (OMAP310)
>   - sx1                  Siemens SX1 (OMAP310) V2
>   - sx1-v1               Siemens SX1 (OMAP310) V1
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/devices/arm-softmmu.mak | 2 --
>  hw/arm/Kconfig                          | 4 ++++
>  target/arm/Kconfig                      | 4 ++++
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
> index 0824e9be795..6ae964c14fd 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -14,8 +14,6 @@ CONFIG_INTEGRATOR=y
>  CONFIG_FSL_IMX31=y
>  CONFIG_MUSICPAL=y
>  CONFIG_MUSCA=y
> -CONFIG_CHEETAH=y
> -CONFIG_SX1=y
>  CONFIG_NSERIES=y
>  CONFIG_STELLARIS=y
>  CONFIG_REALVIEW=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index f3ecb73a3d8..f2957b33bee 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -31,6 +31,8 @@ config ARM_VIRT
>  
>  config CHEETAH
>      bool
> +    default y if TCG && ARM
> +    select ARM_V4
>      select OMAP
>      select TSC210X
>  
> @@ -249,6 +251,8 @@ config COLLIE
>  
>  config SX1
>      bool
> +    default y if TCG && ARM
> +    select ARM_V4
>      select OMAP
>  
>  config VERSATILE
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index ae89d05c7e5..811e1e81652 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -6,6 +6,10 @@ config AARCH64
>      bool
>      select ARM
>  
> +config ARM_V4
> +    bool
> +    depends on TCG && ARM
> +
>  config ARM_V7M
>      bool
>      select PTIMER
> 

Re: [PATCH v6 03/11] target/arm: Restrict ARMv4 cpus to TCG accel

[Philippe Mathieu-Daudé]

On 3/4/21 12:55 PM, Claudio Fontana wrote:
> Hi,
> 
> I am trying to take these patches,
> in the hope that they help with some of the test issues I am having with the kvm-only build,
> 
> but they fail with:
> 
> target/arm/Kconfig: does not exist in index
> 
> so I guess I need the "target/arm/Kconfig" series right, how can I find that one?

See the Based-on in the cover ;)
https://www.mail-archive.com/qemu-block@nongnu.org/msg79924.html

Regards,

Phil.