Re: [PATCH v1 3/9] Add init and realize funciontality for RDT device.

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Fri, 19 Jul 2024 16:29:23 +0000
Hendrik Wuethrich <whendrik@google.com> wrote:

> From: ‪Hendrik Wüthrich <whendrik@google.com>
> 
> Add code to initialize all necessary state for the RDT device.
> 
> Signed-off-by: Hendrik Wüthrich <whendrik@google.com>
> ---
>  hw/i386/rdt.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/hw/i386/rdt.c b/hw/i386/rdt.c
> index cf246ab835..259dafc963 100644
> --- a/hw/i386/rdt.c
> +++ b/hw/i386/rdt.c
> @@ -62,10 +62,38 @@ static void rdt_init(Object *obj)
>  
>  static void rdt_realize(DeviceState *dev, Error **errp)
>  {
> +    CPUState *cs = first_cpu;
> +

No blank line here.

> +    RDTState *rdtDev = RDT(dev);

blank line here.

> +    rdtDev->rdtInstances = g_array_new(false, true, sizeof(RDTStateInstance));
> +    g_array_set_size(rdtDev->rdtInstances, cs->nr_cores);
> +    CPU_FOREACH(cs) {
> +        RDTStateInstance *rdt = &g_array_index(rdtDev->rdtInstances, RDTStateInstance, cs->cpu_index);
> +
No blank line here. Also wrap this long line somewhere.

> +        X86CPU *cpu = X86_CPU(cs);
blank line here.

> +        rdt->rdtstate = rdtDev;
> +        cpu->rdt = rdt;
> +
> +        rdt->monitors = g_array_new(false, true, sizeof(RDTMonitor));
> +        rdt->rdtstate->allocations = g_array_new(false, true, sizeof(RDTAllocation));
> +
> +        g_array_set_size(rdt->monitors, rdtDev->rmids);
> +        g_array_set_size(rdt->rdtstate->allocations, rdtDev->rmids);

Are these g_array's going to change size? If not, why go through this dance when
a simple pointer will do?


> +    }
>  }
>  
>  static void rdt_finalize(Object *obj)
>  {
> +    CPUState *cs;
> +    RDTState *rdt = RDT(obj);
> +
> +    CPU_FOREACH(cs) {
> +        RDTStateInstance *rdtInstance = &g_array_index(rdt->rdtInstances, RDTStateInstance, cs->cpu_index);

Long line. Wrap that.  Worth running checkpatch which I think
will moan about things like this.



> +        g_array_free(rdtInstance->monitors, true);
> +        g_array_free(rdtInstance->rdtstate->allocations, true);
> +    }
> +
> +    g_array_free(rdt->rdtInstances, true);
>  }
>  
>  static void rdt_class_init(ObjectClass *klass, void *data)