[PATCH v2 17/20] smbios: clear smbios_type4_count before building tables

Igor Mammedov posted 20 patches 8 months, 3 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Song Gao <gaosong@loongson.cn>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>
There is a newer version of this series
[PATCH v2 17/20] smbios: clear smbios_type4_count before building tables
Posted by Igor Mammedov 8 months, 3 weeks ago
it will help to keep type 4 tables accounting correct in case
SMBIOS tables are built multiple times.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
 hw/smbios/smbios.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index bf5c7a8885..b64d3bc227 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -981,6 +981,7 @@ static bool smbios_get_tables_ep(MachineState *ms,
            ep_type == SMBIOS_ENTRY_POINT_TYPE_64);
 
     g_free(smbios_tables);
+    smbios_type4_count = 0;
     smbios_tables = g_memdup2(usr_blobs, usr_blobs_len);
     smbios_tables_len = usr_blobs_len;
     smbios_table_max = usr_table_max;
-- 
2.39.3
Re: [PATCH v2 17/20] smbios: clear smbios_type4_count before building tables
Posted by Ani Sinha 8 months, 3 weeks ago

> On 05-Mar-2024, at 21:27, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> it will help to keep type 4 tables accounting correct in case
> SMBIOS tables are built multiple times.


I suggest you arrange this before patch 15 where you are actually calling smbios_get_tables_ep() multiple times. That way there is no window where things can break between patches.

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> hw/smbios/smbios.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index bf5c7a8885..b64d3bc227 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -981,6 +981,7 @@ static bool smbios_get_tables_ep(MachineState *ms,
>            ep_type == SMBIOS_ENTRY_POINT_TYPE_64);
> 
>     g_free(smbios_tables);
> +    smbios_type4_count = 0;

Nit: Can you put this before g_free() because gfree(smbios_tables) and smbios_tables = memdup2() etc are related. This is kind of coming in between.

>     smbios_tables = g_memdup2(usr_blobs, usr_blobs_len);
>     smbios_tables_len = usr_blobs_len;
>     smbios_table_max = usr_table_max;
> -- 
> 2.39.3
> 
Re: [PATCH v2 17/20] smbios: clear smbios_type4_count before building tables
Posted by Igor Mammedov 8 months, 3 weeks ago
On Wed, 6 Mar 2024 13:17:39 +0530
Ani Sinha <anisinha@redhat.com> wrote:

> > On 05-Mar-2024, at 21:27, Igor Mammedov <imammedo@redhat.com> wrote:
> > 
> > it will help to keep type 4 tables accounting correct in case
> > SMBIOS tables are built multiple times.  
> 
> 
> I suggest you arrange this before patch 15 where you are actually calling smbios_get_tables_ep() multiple times. That way there is no window where things can break between patches.

it doesn't break in patch by patch test, because auto is not in use yet.
It could be moved but, I'd rather not respin series for the sake of reordering.

Michael can you reorder it before 15/20 when applying?

> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> > ---
> > hw/smbios/smbios.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > index bf5c7a8885..b64d3bc227 100644
> > --- a/hw/smbios/smbios.c
> > +++ b/hw/smbios/smbios.c
> > @@ -981,6 +981,7 @@ static bool smbios_get_tables_ep(MachineState *ms,
> >            ep_type == SMBIOS_ENTRY_POINT_TYPE_64);
> > 
> >     g_free(smbios_tables);
> > +    smbios_type4_count = 0;  
> 
> Nit: Can you put this before g_free() because gfree(smbios_tables) and smbios_tables = memdup2() etc are related. This is kind of coming in between.
cleanup is not related to memdup, the later works fine without cleanup.
I'd prefer to keep it as is.

> 
> >     smbios_tables = g_memdup2(usr_blobs, usr_blobs_len);
> >     smbios_tables_len = usr_blobs_len;
> >     smbios_table_max = usr_table_max;
> > -- 
> > 2.39.3
> >   
>