Date: Tue, 23 Feb 2021 14:55:58 -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: <202102232255.11NMtwxL013572@slippy.cwsent.com> In-Reply-To: <202102232251.11NMp7Dl013494@slippy.cwsent.com> References: <202102232119.11NLJw17099523@gitrepo.freebsd.org> <202102232251.11NMp7Dl013494@slippy.cwsent.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Nevermind. I see you found it. -- 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. In message <202102232251.11NMp7Dl013494@slippy.cwsent.com>, Cy Schubert writes: > 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=ba6e37e47f41484fc61cc034619267 > b8 > > 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_D > EF > > ); > > > > 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, v > oi > > 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, vo > id > > *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_call > ba > > 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_cal > lb > > 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?202102232255.11NMtwxL013572>