Date: Tue, 23 Feb 2021 14:51:07 -0800 From: Cy Schubert <Cy.Schubert@cschubert.com> To: Allan Jude <allanjude@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: ba6e37e47f41 - main - ipmi_smbios: Deduplicate smbios entry point discovery logic Message-ID: <202102232251.11NMp7Dl013494@slippy.cwsent.com> In-Reply-To: <202102232119.11NLJw17099523@gitrepo.freebsd.org> References: <202102232119.11NLJw17099523@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <202102232119.11NLJw17099523@gitrepo.freebsd.org>, Allan Jude writes : > The branch main has been updated by allanjude: > > URL: https://cgit.FreeBSD.org/src/commit/?id=ba6e37e47f41484fc61cc034619267b8 > 2ddd056c > > commit ba6e37e47f41484fc61cc034619267b82ddd056c > Author: Allan Jude <allanjude@FreeBSD.org> > AuthorDate: 2021-02-23 21:17:37 +0000 > Commit: Allan Jude <allanjude@FreeBSD.org> > CommitDate: 2021-02-23 21:17:37 +0000 > > ipmi_smbios: Deduplicate smbios entry point discovery logic > > Sponsored by: Ampere Computing LLC > Submitted by: Klara Inc. > Reviewed by: imp > Differential Revision: https://reviews.freebsd.org/D28743 > --- > sys/dev/ipmi/ipmi_isa.c | 1 + > sys/dev/ipmi/ipmi_pci.c | 1 + > sys/dev/ipmi/ipmi_smbios.c | 46 ++++++++++++++++---------------------------- > -- > sys/dev/ipmi/ipmi_smbus.c | 1 + > sys/dev/smbios/smbios.c | 21 +++++++++++++++++++++ > sys/dev/smbios/smbios.h | 2 ++ > 6 files changed, 42 insertions(+), 30 deletions(-) > > diff --git a/sys/dev/ipmi/ipmi_isa.c b/sys/dev/ipmi/ipmi_isa.c > index cdff305b07ec..2831b53e4586 100644 > --- a/sys/dev/ipmi/ipmi_isa.c > +++ b/sys/dev/ipmi/ipmi_isa.c > @@ -286,3 +286,4 @@ static driver_t ipmi_isa_driver = { > }; > > DRIVER_MODULE(ipmi_isa, isa, ipmi_isa_driver, ipmi_devclass, 0, 0); > +MODULE_DEPEND(ipmi_isa, smbios, 1, 1, 1); > diff --git a/sys/dev/ipmi/ipmi_pci.c b/sys/dev/ipmi/ipmi_pci.c > index d4598f9db873..1697a4c31c2a 100644 > --- a/sys/dev/ipmi/ipmi_pci.c > +++ b/sys/dev/ipmi/ipmi_pci.c > @@ -179,6 +179,7 @@ static driver_t ipmi_pci_driver = { > }; > > DRIVER_MODULE(ipmi_pci, pci, ipmi_pci_driver, ipmi_devclass, 0, 0); > +MODULE_DEPEND(ipmi_pci, smbios, 1, 1, 1); > > /* Native IPMI on PCI driver. */ > > diff --git a/sys/dev/ipmi/ipmi_smbios.c b/sys/dev/ipmi/ipmi_smbios.c > index 308a3b076ef7..735f404eec5f 100644 > --- a/sys/dev/ipmi/ipmi_smbios.c > +++ b/sys/dev/ipmi/ipmi_smbios.c > @@ -88,7 +88,7 @@ MTX_SYSINIT(ipmi_info, &ipmi_info_mtx, "ipmi info", MTX_DEF > ); > > static void ipmi_smbios_probe(struct ipmi_get_info *); > static int smbios_cksum(struct smbios_eps *); > -static void smbios_walk_table(uint8_t *, int, smbios_callback_t, > +static void smbios_walk_table(uint8_t *, vm_size_t, smbios_callback_t, > void *); > static void smbios_ipmi_info(struct smbios_structure_header *, void *); > > @@ -147,11 +147,12 @@ smbios_ipmi_info(struct smbios_structure_header *h, voi > d *arg) > } > > static void > -smbios_walk_table(uint8_t *p, int entries, smbios_callback_t cb, void *arg) > +smbios_walk_table(uint8_t *table, vm_size_t size, smbios_callback_t cb, void > *arg) > { > struct smbios_structure_header *s; > + uint8_t *p; > > - while (entries--) { > + for (p = table; p < table + size;) { > s = (struct smbios_structure_header *)p; > cb(s, arg); > > @@ -160,8 +161,11 @@ smbios_walk_table(uint8_t *p, int entries, smbios_callba > ck_t cb, void *arg) > * formatted area of this structure. > */ > p += s->length; > - while (!(p[0] == 0 && p[1] == 0)) > + while (!(p[0] == 0 && p[1] == 0)) { > p++; > + if (p >= table + size) > + return; > + } > > /* > * Skip over the double-nul to the start of the next > @@ -179,41 +183,23 @@ smbios_walk_table(uint8_t *p, int entries, smbios_callb > ack_t cb, void *arg) > static void > ipmi_smbios_probe(struct ipmi_get_info *info) > { > - struct smbios_eps *header; > void *table; > - u_int32_t addr; > + vm_paddr_t table_paddr; > + vm_size_t table_size; > + int err; > > bzero(info, sizeof(struct ipmi_get_info)); > > - /* Find the SMBIOS table header. */ > - addr = bios_sigsearch(SMBIOS_START, SMBIOS_SIG, SMBIOS_LEN, > - SMBIOS_STEP, SMBIOS_OFF); > - if (addr == 0) > + err = smbios_get_structure_table(&table_paddr, &table_size); > + if (err != 0) > return; > > - /* > - * Map the header. We first map a fixed size to get the actual > - * length and then map it a second time with the actual length so > - * we can verify the checksum. > - */ > - header = pmap_mapbios(addr, sizeof(struct smbios_eps)); > - table = pmap_mapbios(addr, header->length); > - pmap_unmapbios((vm_offset_t)header, sizeof(struct smbios_eps)); > - header = table; > - if (smbios_cksum(header) != 0) { > - pmap_unmapbios((vm_offset_t)header, header->length); > - return; > - } > + table = pmap_mapbios(table_paddr, table_size); > > - /* Now map the actual table and walk it looking for an IPMI entry. */ > - table = pmap_mapbios(header->structure_table_address, > - header->structure_table_length); > - smbios_walk_table(table, header->number_structures, smbios_ipmi_info, > - info); > + smbios_walk_table(table, table_size, smbios_ipmi_info, info); > > /* Unmap everything. */ > - pmap_unmapbios((vm_offset_t)table, header->structure_table_length); > - pmap_unmapbios((vm_offset_t)header, header->length); > + pmap_unmapbios((vm_offset_t)table, table_size); > } > > /* > diff --git a/sys/dev/ipmi/ipmi_smbus.c b/sys/dev/ipmi/ipmi_smbus.c > index 212248f0217e..bc7377e5aee8 100644 > --- a/sys/dev/ipmi/ipmi_smbus.c > +++ b/sys/dev/ipmi/ipmi_smbus.c > @@ -131,3 +131,4 @@ static driver_t ipmi_smbus_driver = { > > DRIVER_MODULE(ipmi_smbus, smbus, ipmi_smbus_driver, ipmi_devclass, 0, 0); > MODULE_DEPEND(ipmi_smbus, smbus, SMBUS_MINVER, SMBUS_PREFVER, SMBUS_MAXVER); > +MODULE_DEPEND(ipmi_smbus, smbios, 1, 1, 1); > diff --git a/sys/dev/smbios/smbios.c b/sys/dev/smbios/smbios.c > index 10589ed8d49d..00c7e2c98d2a 100644 > --- a/sys/dev/smbios/smbios.c > +++ b/sys/dev/smbios/smbios.c > @@ -51,6 +51,8 @@ __FBSDID("$FreeBSD$"); > #endif > #include <dev/smbios/smbios.h> > > +static struct smbios_softc *smbios; > + > /* > * System Management BIOS Reference Specification, v2.4 Final > * http://www.dmtf.org/standards/published_documents/DSP0134.pdf > @@ -179,6 +181,7 @@ smbios_attach (device_t dev) > bcd2bin(sc->eps->BCD_revision & 0x0f)); > printf("\n"); > > + smbios = sc; > return (0); > bad: > if (sc->res) > @@ -191,6 +194,7 @@ smbios_detach (device_t dev) > { > struct smbios_softc *sc; > > + smbios = NULL; > sc = device_get_softc(dev); > > if (sc->res) > @@ -199,6 +203,23 @@ smbios_detach (device_t dev) > return (0); > } > > +int > +smbios_get_structure_table(vm_paddr_t *table, vm_size_t *size) > +{ > + > + if (smbios == NULL) > + return (ENXIO); > + if (smbios->eps_64bit) { > + *table = smbios->eps3->structure_table_address; > + *size = smbios->eps3->structure_table_max_size; This had some undesired results. --- all_subdir_bios/smbios --- /opt/src/git-src/sys/dev/smbios/smbios.c:212:14: error: no member named 'eps_64bit' in 'struct smbios_softc' if (smbios->eps_64bit) { ~~~~~~ ^ /opt/src/git-src/sys/dev/smbios/smbios.c:213:20: error: no member named 'eps3' in 'struct smbios_softc'; did you mean 'eps'? *table = smbios->eps3->structure_table_address; ^~~~ eps /opt/src/git-src/sys/dev/smbios/smbios.c:66:22: note: 'eps' declared here struct smbios_eps * eps; ^ /opt/src/git-src/sys/dev/smbios/smbios.c:214:19: error: no member named 'eps3' in 'struct smbios_softc'; did you mean 'eps'? *size = smbios->eps3->structure_table_max_size; ^~~~ eps /opt/src/git-src/sys/dev/smbios/smbios.c:66:22: note: 'eps' declared here struct smbios_eps * eps; ^ /opt/src/git-src/sys/dev/smbios/smbios.c:214:25: error: no member named 'structure_table_max_size' in 'struct smbios_eps'; did you mean 'structure_table_address'? *size = smbios->eps3->structure_table_max_size; ^~~~~~~~~~~~~~~~~~~~~~~~ structure_table_address /opt/src/git-src/sys/dev/smbios/smbios.h:58:11: note: 'structure_table_address' declared here uint32_t structure_table_address; ^ > + } else { > + *table = smbios->eps->structure_table_address; > + *size = smbios->eps->structure_table_length; > + } > + return (0); > +} > + > + > static int > smbios_modevent (mod, what, arg) > module_t mod; > diff --git a/sys/dev/smbios/smbios.h b/sys/dev/smbios/smbios.h > index 6503cdb73c4c..554b882f1da4 100644 > --- a/sys/dev/smbios/smbios.h > +++ b/sys/dev/smbios/smbios.h > @@ -32,6 +32,8 @@ > #ifndef _SMBIOS_H_ > #define _SMBIOS_H_ > > +int smbios_get_structure_table(vm_paddr_t *table, vm_size_t *size); > + > /* > * System Management BIOS > */ > -- Cheers, Cy Schubert <Cy.Schubert@cschubert.com> FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org NTP: <cy@nwtime.org> Web: https://nwtime.org The need of the many outweighs the greed of the few.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202102232251.11NMp7Dl013494>