On Tue, Apr 08, 2025 at 04:52:26PM +0200, Jean Delvare wrote:
> On Thu, 2025-04-03 at 19:37 -0600, Jerry Hoemann wrote:
> > Add PCIe Riser.
> >
> > Signed-off-by: Jerry Hoemann <jerry.hoem...@hpe.com>
> > ---
> > dmioem.c | 40 ++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 38 insertions(+), 2 deletions(-)
> >
> > diff --git a/dmioem.c b/dmioem.c
> > index 94be918..97a1c46 100644
> > --- a/dmioem.c
> > +++ b/dmioem.c
> > @@ -962,6 +962,27 @@ static void dmi_hp_245_pcie_riser(const struct
> > dmi_header *h)
> > pr_attr("Riser Name", dmi_string(h, data[0x08]));
> > }
> >
> > +static void dmi_hp_245_pcie_mhs_riser(const struct dmi_header *h)
> > +{
> > + u8 *data = h->data;
> > + u8 i, count;
> > + int len = h->length;
> > +
> > + pr_attr("Board Type", "PCIe Riser (MHS Platform)");
> > + if (h->length < 0x0C) return;
>
> I think length 0x0B is valid? If count == 0. Or if the minimal legal
> value for count is 1 then the minimum record length would be 0x0D?
For the degenerated case, i.e. count=0, then 0x0B would be correct
length case.
As this is for a PCIe riser, I was assuming there would be at least one
slot. In this case 0x0C is correct as there would always at least one
slot ID at offset 0x0B.
But changing to initial length check of 0x0B to accommodate a
zero slot case is more complete.
Done.
>
> > + pr_attr("Riser ID", "%d", data[0x05]);
> > + if (data[0x06])
> > + pr_attr("Firmware Version", "%x.%x", data[0x06], data[0x07]);
> > + pr_attr("Downgradable", "%s", data[0x08] & 0x01 ? "Yes" : "No");
> > + pr_attr("Riser Name", dmi_string(h, data[0x09]));
> > + count = data[0x0A];
> > + pr_attr("Slot Count", "%d", count);
> > + pr_list_start("Slot IDs", NULL);
> > + for (i = 0; (i < count) && ((0x0B + i) < len); i++)
> > + pr_list_item("0x%x", data[0x0B + i]);
> > + pr_list_end();
> > +}
> > + (...)
>
> Everything else looks good.
>
> Thanks,
> --
> Jean Delvare
> SUSE L3 Support
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
