[PATCH 0/1] smbios_build_type_8_table should use T8_BASE.

[PATCH 0/1] smbios_build_type_8_table should use T8_BASE.

[Felix Wu]

It should use T8_BASE instead of T0_BASE.

Felix Wu (1):
  SMBIOS type 8 should use T8_BASE.

 hw/smbios/smbios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.43.0.275.g3460e3d667-goog

[PATCH 1/1] SMBIOS type 8 should use T8_BASE.

[Felix Wu]

---
 hw/smbios/smbios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 2a90601ac5..7dda84b284 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -591,6 +591,7 @@ bool smbios_skip_table(uint8_t type, bool required_table)
 #define T2_BASE 0x200
 #define T3_BASE 0x300
 #define T4_BASE 0x400
+#define T8_BASE 0x800
 #define T11_BASE 0xe00
 
 #define T16_BASE 0x1000
@@ -775,7 +776,7 @@ static void smbios_build_type_8_table(void)
     struct type8_instance *t8;
 
     QTAILQ_FOREACH(t8, &type8, next) {
-        SMBIOS_BUILD_TABLE_PRE(8, T0_BASE + instance, true);
+        SMBIOS_BUILD_TABLE_PRE(8, T8_BASE + instance, true);
 
         SMBIOS_TABLE_SET_STR(8, internal_reference_str, t8->internal_reference);
         SMBIOS_TABLE_SET_STR(8, external_reference_str, t8->external_reference);
-- 
2.43.0.275.g3460e3d667-goog

Re: [PATCH 1/1] SMBIOS type 8 should use T8_BASE.

[Igor Mammedov]

On Thu, 11 Jan 2024 19:25:22 +0000
Felix Wu <flwu@google.com> wrote:

it is missing Signed-off tag a minimum, and also commit message should describe in more detail
what's wrong and what's breaks and how it's being fixed with references to spec preferably. 

please see https://www.qemu.org/docs/master/devel/submitting-a-patch.html
for requirements to commit message of the patch.

> ---
>  hw/smbios/smbios.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 2a90601ac5..7dda84b284 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -591,6 +591,7 @@ bool smbios_skip_table(uint8_t type, bool required_table)
>  #define T2_BASE 0x200
>  #define T3_BASE 0x300
>  #define T4_BASE 0x400
> +#define T8_BASE 0x800
>  #define T11_BASE 0xe00
>  
>  #define T16_BASE 0x1000
> @@ -775,7 +776,7 @@ static void smbios_build_type_8_table(void)
>      struct type8_instance *t8;
>  
>      QTAILQ_FOREACH(t8, &type8, next) {
> -        SMBIOS_BUILD_TABLE_PRE(8, T0_BASE + instance, true);
> +        SMBIOS_BUILD_TABLE_PRE(8, T8_BASE + instance, true);

we can't do that unconditionally as it will break live migration
where firmware reading this might get part of broken tables (on source host)
and 2nd part of fixed ones (from target host).

So we need to use T0_base for old machine types and T8_BASE for
default/new machine types.
for example see how 'pcmc->smbios_uuid_encoded' is used.
unless it's x86 specific, this should be done for affected machine types.
  
>          SMBIOS_TABLE_SET_STR(8, internal_reference_str, t8->internal_reference);
>          SMBIOS_TABLE_SET_STR(8, external_reference_str, t8->external_reference);

Re: [PATCH 1/1] SMBIOS type 8 should use T8_BASE.

[Igor Mammedov]

On Wed, 17 Jan 2024 09:38:47 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Thu, 11 Jan 2024 19:25:22 +0000
> Felix Wu <flwu@google.com> wrote:
> 
> it is missing Signed-off tag a minimum, and also commit message should describe in more detail
> what's wrong and what's breaks and how it's being fixed with references to spec preferably. 
> 
> please see https://www.qemu.org/docs/master/devel/submitting-a-patch.html
> for requirements to commit message of the patch.
>

on top of that pls consider adding test case for it in
tests/qtest/bios-tables-test.c

> > ---
> >  hw/smbios/smbios.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > index 2a90601ac5..7dda84b284 100644
> > --- a/hw/smbios/smbios.c
> > +++ b/hw/smbios/smbios.c
> > @@ -591,6 +591,7 @@ bool smbios_skip_table(uint8_t type, bool required_table)
> >  #define T2_BASE 0x200
> >  #define T3_BASE 0x300
> >  #define T4_BASE 0x400
> > +#define T8_BASE 0x800
> >  #define T11_BASE 0xe00
> >  
> >  #define T16_BASE 0x1000
> > @@ -775,7 +776,7 @@ static void smbios_build_type_8_table(void)
> >      struct type8_instance *t8;
> >  
> >      QTAILQ_FOREACH(t8, &type8, next) {
> > -        SMBIOS_BUILD_TABLE_PRE(8, T0_BASE + instance, true);
> > +        SMBIOS_BUILD_TABLE_PRE(8, T8_BASE + instance, true);  
> 
> we can't do that unconditionally as it will break live migration
> where firmware reading this might get part of broken tables (on source host)
> and 2nd part of fixed ones (from target host).
> 
> So we need to use T0_base for old machine types and T8_BASE for
> default/new machine types.
> for example see how 'pcmc->smbios_uuid_encoded' is used.
> unless it's x86 specific, this should be done for affected machine types.
>   
> >          SMBIOS_TABLE_SET_STR(8, internal_reference_str, t8->internal_reference);
> >          SMBIOS_TABLE_SET_STR(8, external_reference_str, t8->external_reference);  
>