Re: [dmidecode] [PATCH v2 1/1] dmioem: Decode HPE OEM Record 238

[Jerry Hoemann]

On Tue, May 31, 2022 at 09:34:22AM +0200, Jean Delvare wrote:
> On Mon, 30 May 2022 12:30:09 -0600, Jerry Hoemann wrote:
> > Decode HPE OEM Record 238: USB Port Connector Correlation Record.
> > 
> > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> > ---
> >  dmioem.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> > 
> > diff --git a/dmioem.c b/dmioem.c
> > index 61569a6..9f38a2f 100644
> > --- a/dmioem.c
> > +++ b/dmioem.c
> > @@ -299,6 +299,57 @@ static void dmi_hp_203_devloc(const char *fname, 
> > unsigned int code)
> >     pr_attr(fname, "%s", str);
> >  }
> >  
> > +static void dmi_hp_238_loc(const char *fname, unsigned int code)
> > +{
> > +   static const char *str = "Reserved";
> > +   static const char *location[] = {
> > +           "Internal", /* 0x00 */
> > +           "Front of Server",
> > +           "Rear of Server",
> > +           "Embedded internal SD Card",
> > +           "iLO USB",
> > +           "HP NAND Controller (USX 2065 or other)",
> > +           "Reserved",
> > +           "Debug Port", /* 0x07 */
> > +   };
> > +
> > +   if (code < ARRAY_SIZE(location))
> > +           str = location[code];
> > +
> > +   pr_attr(fname, "%s", str);
> > +}
> > +
> > +static void dmi_hp_238_flags(const char *fname, unsigned int code)
> > +{
> > +   static const char *str = "Reserved";
> > +   static const char *flags[] = {
> > +           "Not Shared", /* 0x00 */
> > +           "Shared with physical switch",
> > +           "Shared with automatic control", /* 0x02 */
> > +   };
> > +
> > +   if (code < ARRAY_SIZE(flags))
> > +           str = flags[code];
> > +
> > +   pr_attr(fname, "%s", str);
> > +}
> > +
> > +static void dmi_hp_238_speed(const char *fname, unsigned int code)
> > +{
> > +   static const char *str = "Reserved";
> > +   static const char *speed[] = {
> > +           "Reserved", /* 0x00 */
> > +           "USB 1.1 Full Speed",
> > +           "USB 2.0 High Speed",
> > +           "USB 3.0 Super Speed" /* 0x03 */
> > +   };
> > +
> > +   if (code < ARRAY_SIZE(speed))
> > +           str = speed[code];
> > +
> > +   pr_attr(fname, "%s", str);
> > +}
> > +
> >  static int dmi_decode_hp(const struct dmi_header *h)
> >  {
> >     u8 *data = h->data;
> > @@ -591,6 +642,44 @@ static int dmi_decode_hp(const struct dmi_header *h)
> >                     }
> >                     break;
> >  
> > +           case 238:
> > +                   /*
> > +                    * Vendor Specific: HPE USB Port Connector Correlation 
> > Record
> > +                    *
> > +                    * Offset |  Name      | Width | Description
> > +                    * ---------------------------------------
> > +                    *  0x00  | Type       | BYTE  | 0xEE, HP Device 
> > Correlation Record
> > +                    *  0x01  | Length     | BYTE  | Length of structure
> > +                    *  0x02  | Handle     | WORD  | Unique handle
> > +                    *  0x04  | Hand Assoc | WORD  | Handle to map to Type 8
> > +                    *  0x06  | Parent Bus | BYTE  | PCI Bus number of USB 
> > controller of this port
> > +                    *  0x07  | Par Dev/Fun| BYTE  | PCI Dev/Fun of USB 
> > Controller of this port
> > +                    *  0x08  | Location   | BYTE  | Enumerated value of 
> > location of USB port
> > +                    *  0x09  | Flags      | WORD  | USB Shared Management 
> > Port
> > +                    *  0x0B  | Port Inst  | BYTE  | Instance number for 
> > this type of USB port
> > +                    *  0x0C  | Parent Hub | BYTE  | Instance number of 
> > internal Hub
> > +                    *  0x0D  | Port Speed | BYTE  | Enumerated value of 
> > speed configured by BIOS
> > +                    *  0x0E  | Device Path| STRING| UEFI Device Path of 
> > USB endpoint
> > +                    */
> > +                   if (gen < G9) return 0;
> > +                   pr_handle_name("%s Proliant USB Port Connector 
> > Correlation Record", company);
> > +                   if (h->length < 0x0F) break;
> > +                   if (!(opt.flags & FLAG_QUIET))
> > +                           pr_attr("Associated Handle", "0x%04X", 
> > WORD(data + 0x4));
> > +                   pr_attr("PCI Bus of Parent USB", "0x%04X", data[0x6]);
> > +                   pr_attr("PCI Device of Parent USB", "0x%04X", data[0x7] 
> > >> 3);
> > +                   pr_attr("PCI Function of Parent USB", "0x%04X", 
> > data[0x7] & 0x7);
> > +                   dmi_hp_238_loc("Location", data[0x8]);
> > +                   dmi_hp_238_flags("Management Port", data[0x9]);
> 
> This field is documented as WORD size, so 16 bits. Here you only pass
> the first byte and ignore the second (at offset 0xA). Result should be
> the same for now because DMI uses little-endian byte ordering and only
> 2 flags are defined, however it will break if more flags are added in
> the future.
> 
> This is the reason why I suggested passing WORD(data + 0x9) as the
> parameter during my previous review.
> 
> Everything else is OK now, so I can fix that myself before committing
> if that's OK with you.

I'm sorry.  I missed that.  You're correct.  It should be:

        dmi_hp_238_flags("Management port", WORD(data + 0x9));

> 
> > +                   pr_attr("Port Instance", "%d", data[0xB]);
> > +                   if (data[0xC] != 0xFE)
> > +                           pr_attr("Parent Hub Port Instance", "%d", 
> > data[0xC]);
> > +                   else
> > +                           pr_attr("Parent Hub Port Instance", "N/A");
> > +                   dmi_hp_238_speed("Port Speed Capability", data[0xD]);
> > +                   pr_attr("Device Path", "%s", dmi_string(h, data[0xE]));
> > +                   break;
> > +
> >             case 240:
> >                     /*
> >                      * Vendor Specific: HPE Proliant Inventory Record
> 
> Thanks,
> -- 
> Jean Delvare
> SUSE L3 Support

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------