Re: [PATCH v6 40/47] hw/arm/xlnx-versal: add versal2 SoC

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 26/9/25 09:07, Luc Michel wrote:
> Add the Versal Gen 2 (versal2) version of the Versal SoC family.
> This version embeds up to 8 Cortex-A78AE cores (split into 4 clusters)
> and 10 Cortex-R52 cores (split into 5 clusters). The similarities
> between versal and versal2 in term of architecture allow to reuse the
> VersalMap structure to almost fully describe the implemented parts of
> versal2.
> 
> The versal2 eFuse device differs quite a lot from the versal one and is
> left as future work.
> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>   include/hw/arm/xlnx-versal.h |  17 ++-
>   hw/arm/xlnx-versal.c         | 207 ++++++++++++++++++++++++++++++++---
>   2 files changed, 209 insertions(+), 15 deletions(-)
> 
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index b6cc71f7209..e1fb1f4cf5b 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -1,7 +1,7 @@
>   /*
> - * Model of the Xilinx Versal
> + * AMD/Xilinx Versal family SoC model.
>    *
>    * Copyright (c) 2018 Xilinx Inc.
>    * Copyright (c) 2025 Advanced Micro Devices, Inc.
>    * Written by Edgar E. Iglesias
>    *
> @@ -20,10 +20,11 @@
>   
>   #define TYPE_XLNX_VERSAL_BASE "xlnx-versal-base"
>   OBJECT_DECLARE_TYPE(Versal, VersalClass, XLNX_VERSAL_BASE)
>   
>   #define TYPE_XLNX_VERSAL "xlnx-versal"
> +#define TYPE_XLNX_VERSAL2 "xlnx-versal2"
>   
>   struct Versal {
>       /*< private >*/
>       SysBusDevice parent_obj;
>   
> @@ -70,6 +71,20 @@ hwaddr versal_get_reserved_mmio_addr(Versal *s);
>   
>   int versal_get_num_cpu(VersalVersion version);
>   int versal_get_num_can(VersalVersion version);
>   int versal_get_num_sdhci(VersalVersion version);
>   
> +static inline const char *versal_get_class(VersalVersion version)
> +{

Not a blocker, but inlining isn't really justified here.

> +    switch (version) {
> +    case VERSAL_VER_VERSAL:
> +        return TYPE_XLNX_VERSAL;
> +
> +    case VERSAL_VER_VERSAL2:
> +        return TYPE_XLNX_VERSAL2;
> +
> +    default:
> +        g_assert_not_reached();
> +    }
> +}