[v1] qom: Constify class_data

[PATCH 03/10] target/xtensa: Finalize config in xtensa_register_core()

Posted by Philippe Mathieu-Daudé 12 months ago

Only modify XtensaConfig within xtensa_register_core(),
when the class is registered, not when it is initialized.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Cc: Max Filippov <jcmvbkbc@gmail.com>
---
 target/xtensa/helper.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index 2978c471c1f..c4735989714 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
 {
     CPUClass *cc = CPU_CLASS(oc);
     XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
-    XtensaConfig *config = data;
+    const XtensaConfig *config = data;
 
-    xtensa_finalize_config(config);
     xcc->config = config;
 
     /*
@@ -189,12 +188,15 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
 
 void xtensa_register_core(XtensaConfigList *node)
 {
+    XtensaConfig *config = g_memdup2(node->config, sizeof(config));
     TypeInfo type = {
         .parent = TYPE_XTENSA_CPU,
         .class_init = xtensa_core_class_init,
-        .class_data = (void *)node->config,
+        .class_data = config,
     };
 
+    xtensa_finalize_config(config);
+
     node->next = xtensa_cores;
     xtensa_cores = node;
     type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"), node->config->name);
-- 
2.47.1

Re: [PATCH 03/10] target/xtensa: Finalize config in xtensa_register_core()

Posted by Richard Henderson 12 months ago

On 2/10/25 02:25, Philippe Mathieu-Daudé wrote:
> Only modify XtensaConfig within xtensa_register_core(),
> when the class is registered, not when it is initialized.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> ---
>   target/xtensa/helper.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
> index 2978c471c1f..c4735989714 100644
> --- a/target/xtensa/helper.c
> +++ b/target/xtensa/helper.c
> @@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
>   {
>       CPUClass *cc = CPU_CLASS(oc);
>       XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
> -    XtensaConfig *config = data;
> +    const XtensaConfig *config = data;
>   
> -    xtensa_finalize_config(config);
>       xcc->config = config;
>   
>       /*
> @@ -189,12 +188,15 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
>   
>   void xtensa_register_core(XtensaConfigList *node)
>   {
> +    XtensaConfig *config = g_memdup2(node->config, sizeof(config));

Why are you introducing a new copy?
Previously we finalized in place.


r~

>       TypeInfo type = {
>           .parent = TYPE_XTENSA_CPU,
>           .class_init = xtensa_core_class_init,
> -        .class_data = (void *)node->config,
> +        .class_data = config,
>       };
>   
> +    xtensa_finalize_config(config);
> +
>       node->next = xtensa_cores;
>       xtensa_cores = node;
>       type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"), node->config->name);

Re: [PATCH 03/10] target/xtensa: Finalize config in xtensa_register_core()

Posted by Max Filippov 12 months ago

On Mon, Feb 10, 2025 at 2:26 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Only modify XtensaConfig within xtensa_register_core(),
> when the class is registered, not when it is initialized.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  target/xtensa/helper.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
> index 2978c471c1f..c4735989714 100644
> --- a/target/xtensa/helper.c
> +++ b/target/xtensa/helper.c
> @@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
>      XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
> -    XtensaConfig *config = data;
> +    const XtensaConfig *config = data;
>
> -    xtensa_finalize_config(config);

It was here to only do potentially expensive finalization once for the
actually used core, but I guess there's nothing that expensive there.

>      xcc->config = config;
>
>      /*
> @@ -189,12 +188,15 @@ static void xtensa_core_class_init(ObjectClass *oc, void *data)
>
>  void xtensa_register_core(XtensaConfigList *node)
>  {
> +    XtensaConfig *config = g_memdup2(node->config, sizeof(config));

The structures pointed to by the node->config are not const, I'm not sure
why the pointer is const. I'd say that rather than making a copy here the
XtensaConfigList should lose the const qualifier in the config definition.

>      TypeInfo type = {
>          .parent = TYPE_XTENSA_CPU,
>          .class_init = xtensa_core_class_init,
> -        .class_data = (void *)node->config,
> +        .class_data = config,
>      };
>
> +    xtensa_finalize_config(config);
> +
>      node->next = xtensa_cores;
>      xtensa_cores = node;
>      type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"), node->config->name);

-- 
Thanks.
-- Max