[v1] hvmloader: add new SMBIOS tables (7,8,9,26,27,28)

[PATCH 1/3] hvmloader: fix code style violations

Posted by Petr Beneš 4 months ago

From: Petr Beneš <w1benny@gmail.com>

Preparatory commit. No functional change.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 tools/firmware/hvmloader/smbios.c | 56 +++++++++++++++----------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c
index 97a054e9e3..fc3cdc9a25 100644
--- a/tools/firmware/hvmloader/smbios.c
+++ b/tools/firmware/hvmloader/smbios.c
@@ -378,18 +378,18 @@ static void *
 smbios_type_0_init(void *start, const char *xen_version,
                    uint32_t xen_major_version, uint32_t xen_minor_version)
 {
-    struct smbios_type_0 *p = (struct smbios_type_0 *)start;
+    struct smbios_type_0 *p = start;
     static const char *smbios_release_date = __SMBIOS_DATE__;
     const char *s;
     void *pts;
     uint32_t length;
 
     pts = get_smbios_pt_struct(0, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE0;
-        return (start + length);
+        return start + length;
     }
 
     memset(p, 0, sizeof(*p));
@@ -438,17 +438,17 @@ smbios_type_1_init(void *start, const char *xen_version,
                    uint8_t uuid[16])
 {
     char uuid_str[37];
-    struct smbios_type_1 *p = (struct smbios_type_1 *)start;
+    struct smbios_type_1 *p = start;
     const char *s;
     void *pts;
     uint32_t length;
 
     pts = get_smbios_pt_struct(1, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE1;
-        return (start + length);
+        return start + length;
     }
 
     memset(p, 0, sizeof(*p));
@@ -496,7 +496,7 @@ smbios_type_1_init(void *start, const char *xen_version,
 static void *
 smbios_type_2_init(void *start)
 {
-    struct smbios_type_2 *p = (struct smbios_type_2 *)start;
+    struct smbios_type_2 *p = start;
     const char *s;
     uint8_t *ptr;
     void *pts;
@@ -504,7 +504,7 @@ smbios_type_2_init(void *start)
     unsigned int counter = 0;
 
     pts = get_smbios_pt_struct(2, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE2;
@@ -517,7 +517,7 @@ smbios_type_2_init(void *start)
                 *((uint16_t*)ptr) = SMBIOS_HANDLE_TYPE3;
         }
 
-        return (start + length);
+        return start + length;
     }
 
     memset(p, 0, sizeof(*p));
@@ -591,18 +591,18 @@ smbios_type_2_init(void *start)
 static void *
 smbios_type_3_init(void *start)
 {
-    struct smbios_type_3 *p = (struct smbios_type_3 *)start;
+    struct smbios_type_3 *p = start;
     const char *s;
     void *pts;
     uint32_t length;
     uint32_t counter = 0;
 
     pts = get_smbios_pt_struct(3, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE3;
-        return (start + length);
+        return start + length;
     }
     
     memset(p, 0, sizeof(*p));
@@ -653,7 +653,7 @@ smbios_type_4_init(
     void *start, unsigned int cpu_number, char *cpu_manufacturer)
 {
     char buf[80]; 
-    struct smbios_type_4 *p = (struct smbios_type_4 *)start;
+    struct smbios_type_4 *p = start;
     uint32_t eax, ebx, ecx, edx;
 
     memset(p, 0, sizeof(*p));
@@ -704,7 +704,7 @@ smbios_type_4_init(
 static void *
 smbios_type_11_init(void *start) 
 {
-    struct smbios_type_11 *p = (struct smbios_type_11 *)start;
+    struct smbios_type_11 *p = start;
     char path[20];
     const char *s;
     int i;
@@ -712,11 +712,11 @@ smbios_type_11_init(void *start)
     uint32_t length;
 
     pts = get_smbios_pt_struct(11, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE11;
-        return (start + length);
+        return start + length;
     }
 
     p->header.type = 11;
@@ -754,7 +754,7 @@ smbios_type_11_init(void *start)
 static void *
 smbios_type_16_init(void *start, uint32_t memsize, int nr_mem_devs)
 {
-    struct smbios_type_16 *p = (struct smbios_type_16*)start;
+    struct smbios_type_16 *p = start;
 
     memset(p, 0, sizeof(*p));
 
@@ -779,7 +779,7 @@ static void *
 smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
 {
     char buf[16];
-    struct smbios_type_17 *p = (struct smbios_type_17 *)start;
+    struct smbios_type_17 *p = start;
     
     memset(p, 0, sizeof(*p));
 
@@ -814,7 +814,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
 static void *
 smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
 {
-    struct smbios_type_19 *p = (struct smbios_type_19 *)start;
+    struct smbios_type_19 *p = start;
     
     memset(p, 0, sizeof(*p));
 
@@ -836,7 +836,7 @@ smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
 static void *
 smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
 {
-    struct smbios_type_20 *p = (struct smbios_type_20 *)start;
+    struct smbios_type_20 *p = start;
 
     memset(p, 0, sizeof(*p));
 
@@ -862,18 +862,18 @@ smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
 static void *
 smbios_type_22_init(void *start)
 {
-    struct smbios_type_22 *p = (struct smbios_type_22 *)start;
+    struct smbios_type_22 *p = start;
     static const char *smbios_release_date = __SMBIOS_DATE__;
     const char *s;
     void *pts;
     uint32_t length;
 
     pts = get_smbios_pt_struct(22, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE22;
-        return (start + length);
+        return start + length;
     }
 
     s = xenstore_read(HVM_XS_SMBIOS_DEFAULT_BATTERY, "0");
@@ -927,7 +927,7 @@ smbios_type_22_init(void *start)
 static void *
 smbios_type_32_init(void *start)
 {
-    struct smbios_type_32 *p = (struct smbios_type_32 *)start;
+    struct smbios_type_32 *p = start;
 
     memset(p, 0, sizeof(*p));
 
@@ -946,16 +946,16 @@ smbios_type_32_init(void *start)
 static void *
 smbios_type_39_init(void *start)
 {
-    struct smbios_type_39 *p = (struct smbios_type_39 *)start;
+    struct smbios_type_39 *p = start;
     void *pts;
     uint32_t length;
 
     pts = get_smbios_pt_struct(39, &length);
-    if ( (pts != NULL)&&(length > 0) )
+    if ( pts != NULL && length > 0 )
     {
         memcpy(start, pts, length);
         p->header.handle = SMBIOS_HANDLE_TYPE39;
-        return (start + length);
+        return start + length;
     }
 
     /* Only present when passed in */
@@ -998,7 +998,7 @@ smbios_type_vendor_oem_init(void *start)
 static void *
 smbios_type_127_init(void *start)
 {
-    struct smbios_type_127 *p = (struct smbios_type_127 *)start;
+    struct smbios_type_127 *p = start;
 
     memset(p, 0, sizeof(*p));
 
-- 
2.34.1

Re: [PATCH 1/3] hvmloader: fix code style violations

Posted by Jan Beulich 4 months ago

On 02.07.2025 01:45, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> Preparatory commit. No functional change.
> 
> Signed-off-by: Petr Beneš <w1benny@gmail.com>

There are style violations that you deal with here, yes. But you go beyond
that. At which point I agree with Frediano that you want to call out at
least what you do beyond bringing code (more?) in line with ./CODING_STYLE.
The subject may also want to reflect that's it's not just violations you
sort.

Jan

Re: [PATCH 1/3] hvmloader: fix code style violations

Posted by Frediano Ziglio 4 months ago

On Wed, Jul 2, 2025 at 12:45 AM Petr Beneš <w1benny@gmail.com> wrote:
>
> From: Petr Beneš <w1benny@gmail.com>
>
> Preparatory commit. No functional change.
>

Can you describe which specific code styles this commit is supposed to fix?

> Signed-off-by: Petr Beneš <w1benny@gmail.com>
> ---
>  tools/firmware/hvmloader/smbios.c | 56 +++++++++++++++----------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c
> index 97a054e9e3..fc3cdc9a25 100644
> --- a/tools/firmware/hvmloader/smbios.c
> +++ b/tools/firmware/hvmloader/smbios.c
> @@ -378,18 +378,18 @@ static void *
>  smbios_type_0_init(void *start, const char *xen_version,
>                     uint32_t xen_major_version, uint32_t xen_minor_version)
>  {
> -    struct smbios_type_0 *p = (struct smbios_type_0 *)start;
> +    struct smbios_type_0 *p = start;
>      static const char *smbios_release_date = __SMBIOS_DATE__;
>      const char *s;
>      void *pts;
>      uint32_t length;
>
>      pts = get_smbios_pt_struct(0, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE0;
> -        return (start + length);
> +        return start + length;
>      }
>
>      memset(p, 0, sizeof(*p));
> @@ -438,17 +438,17 @@ smbios_type_1_init(void *start, const char *xen_version,
>                     uint8_t uuid[16])
>  {
>      char uuid_str[37];
> -    struct smbios_type_1 *p = (struct smbios_type_1 *)start;
> +    struct smbios_type_1 *p = start;
>      const char *s;
>      void *pts;
>      uint32_t length;
>
>      pts = get_smbios_pt_struct(1, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE1;
> -        return (start + length);
> +        return start + length;
>      }
>
>      memset(p, 0, sizeof(*p));
> @@ -496,7 +496,7 @@ smbios_type_1_init(void *start, const char *xen_version,
>  static void *
>  smbios_type_2_init(void *start)
>  {
> -    struct smbios_type_2 *p = (struct smbios_type_2 *)start;
> +    struct smbios_type_2 *p = start;
>      const char *s;
>      uint8_t *ptr;
>      void *pts;
> @@ -504,7 +504,7 @@ smbios_type_2_init(void *start)
>      unsigned int counter = 0;
>
>      pts = get_smbios_pt_struct(2, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE2;
> @@ -517,7 +517,7 @@ smbios_type_2_init(void *start)
>                  *((uint16_t*)ptr) = SMBIOS_HANDLE_TYPE3;
>          }
>
> -        return (start + length);
> +        return start + length;
>      }
>
>      memset(p, 0, sizeof(*p));
> @@ -591,18 +591,18 @@ smbios_type_2_init(void *start)
>  static void *
>  smbios_type_3_init(void *start)
>  {
> -    struct smbios_type_3 *p = (struct smbios_type_3 *)start;
> +    struct smbios_type_3 *p = start;
>      const char *s;
>      void *pts;
>      uint32_t length;
>      uint32_t counter = 0;
>
>      pts = get_smbios_pt_struct(3, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE3;
> -        return (start + length);
> +        return start + length;
>      }
>
>      memset(p, 0, sizeof(*p));
> @@ -653,7 +653,7 @@ smbios_type_4_init(
>      void *start, unsigned int cpu_number, char *cpu_manufacturer)
>  {
>      char buf[80];
> -    struct smbios_type_4 *p = (struct smbios_type_4 *)start;
> +    struct smbios_type_4 *p = start;
>      uint32_t eax, ebx, ecx, edx;
>
>      memset(p, 0, sizeof(*p));
> @@ -704,7 +704,7 @@ smbios_type_4_init(
>  static void *
>  smbios_type_11_init(void *start)
>  {
> -    struct smbios_type_11 *p = (struct smbios_type_11 *)start;
> +    struct smbios_type_11 *p = start;
>      char path[20];
>      const char *s;
>      int i;
> @@ -712,11 +712,11 @@ smbios_type_11_init(void *start)
>      uint32_t length;
>
>      pts = get_smbios_pt_struct(11, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE11;
> -        return (start + length);
> +        return start + length;
>      }
>
>      p->header.type = 11;
> @@ -754,7 +754,7 @@ smbios_type_11_init(void *start)
>  static void *
>  smbios_type_16_init(void *start, uint32_t memsize, int nr_mem_devs)
>  {
> -    struct smbios_type_16 *p = (struct smbios_type_16*)start;
> +    struct smbios_type_16 *p = start;
>
>      memset(p, 0, sizeof(*p));
>
> @@ -779,7 +779,7 @@ static void *
>  smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>  {
>      char buf[16];
> -    struct smbios_type_17 *p = (struct smbios_type_17 *)start;
> +    struct smbios_type_17 *p = start;
>
>      memset(p, 0, sizeof(*p));
>
> @@ -814,7 +814,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>  static void *
>  smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
>  {
> -    struct smbios_type_19 *p = (struct smbios_type_19 *)start;
> +    struct smbios_type_19 *p = start;
>
>      memset(p, 0, sizeof(*p));
>
> @@ -836,7 +836,7 @@ smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
>  static void *
>  smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
>  {
> -    struct smbios_type_20 *p = (struct smbios_type_20 *)start;
> +    struct smbios_type_20 *p = start;
>
>      memset(p, 0, sizeof(*p));
>
> @@ -862,18 +862,18 @@ smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
>  static void *
>  smbios_type_22_init(void *start)
>  {
> -    struct smbios_type_22 *p = (struct smbios_type_22 *)start;
> +    struct smbios_type_22 *p = start;
>      static const char *smbios_release_date = __SMBIOS_DATE__;
>      const char *s;
>      void *pts;
>      uint32_t length;
>
>      pts = get_smbios_pt_struct(22, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE22;
> -        return (start + length);
> +        return start + length;
>      }
>
>      s = xenstore_read(HVM_XS_SMBIOS_DEFAULT_BATTERY, "0");
> @@ -927,7 +927,7 @@ smbios_type_22_init(void *start)
>  static void *
>  smbios_type_32_init(void *start)
>  {
> -    struct smbios_type_32 *p = (struct smbios_type_32 *)start;
> +    struct smbios_type_32 *p = start;
>
>      memset(p, 0, sizeof(*p));
>
> @@ -946,16 +946,16 @@ smbios_type_32_init(void *start)
>  static void *
>  smbios_type_39_init(void *start)
>  {
> -    struct smbios_type_39 *p = (struct smbios_type_39 *)start;
> +    struct smbios_type_39 *p = start;
>      void *pts;
>      uint32_t length;
>
>      pts = get_smbios_pt_struct(39, &length);
> -    if ( (pts != NULL)&&(length > 0) )
> +    if ( pts != NULL && length > 0 )
>      {
>          memcpy(start, pts, length);
>          p->header.handle = SMBIOS_HANDLE_TYPE39;
> -        return (start + length);
> +        return start + length;
>      }
>
>      /* Only present when passed in */
> @@ -998,7 +998,7 @@ smbios_type_vendor_oem_init(void *start)
>  static void *
>  smbios_type_127_init(void *start)
>  {
> -    struct smbios_type_127 *p = (struct smbios_type_127 *)start;
> +    struct smbios_type_127 *p = start;
>
>      memset(p, 0, sizeof(*p));
>

Frediano

[PATCH 1/3] hvmloader: fix code style violations
[PATCH 2/3] hvmloader: fix SMBIOS table length checks
[PATCH 3/3] hvmloader: add new SMBIOS tables (7,8,9,26,27,28)