From owner-dev-commits-src-all@freebsd.org Tue Feb 23 22:51:21 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 407025570CF; Tue, 23 Feb 2021 22:51:21 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-so.shaw.ca (smtp-out-so.shaw.ca [64.59.136.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DlZ6J5ktSz4nS0; Tue, 23 Feb 2021 22:51:20 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.229.168]) by shaw.ca with ESMTPA id EgWTlIZPCHmS3EgWUlo6Iz; Tue, 23 Feb 2021 15:51:19 -0700 X-Authority-Analysis: v=2.4 cv=MaypB7zf c=1 sm=1 tr=0 ts=603586e7 a=7AlCcx2GqMg+lh9P3BclKA==:117 a=7AlCcx2GqMg+lh9P3BclKA==:17 a=xqWC_Br6kY4A:10 a=kj9zAlcOel0A:10 a=qa6Q16uM49sA:10 a=6I5d2MoRAAAA:8 a=iGbCS3zQAAAA:8 a=YxBL1-UpAAAA:8 a=EkcXrb_YAAAA:8 a=DvCTxDGqdahi0-rcVXsA:9 a=m1hu7YsS2IuEFSGk:21 a=da-7ecNw1VAO5s_I:21 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=oRZS8w7TM5j8Y1SnqK-S:22 a=Ia-lj3WSrqcvXOmTRaiG:22 a=LK5xJRSDVpKd5WXXoEvA:22 Received: from slippy.cwsent.com (slippy [IPv6:fc00:1:1:1::5b]) by spqr.komquats.com (Postfix) with ESMTPS id 3ACCCD23; Tue, 23 Feb 2021 14:51:07 -0800 (PST) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.16.1/8.16.1) with ESMTP id 11NMp7Dl013494; Tue, 23 Feb 2021 14:51:07 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <202102232251.11NMp7Dl013494@slippy.cwsent.com> X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.7.1 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Allan Jude 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 In-reply-to: <202102232119.11NLJw17099523@gitrepo.freebsd.org> References: <202102232119.11NLJw17099523@gitrepo.freebsd.org> Comments: In-reply-to Allan Jude message dated "Tue, 23 Feb 2021 21:19:58 +0000." Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Tue, 23 Feb 2021 14:51:07 -0800 X-CMAE-Envelope: MS4xfNoa64ZQnJ5pAQdhtP0wViBvtDMwQiqCMEYf1wgBdUZIGWjp/th+fNqSC781vgR9qwphEVXOUZYD/tH80t0q/9qVrXmvnU65hvRwC/7O5TkFXJeSod1L cPQRpUmGj7EvV2PTlAEf3RPP2WnbRONughdAFSzonXgfyQlDj/68QGMFSeVDMup62YKzX0GaB5WRSl8fHm247ia5S2uodHGYh7qtKyWMUU9sGl7MMFjy085b FsOFllQ7B/TvKnXzSNDDRmPB+O3wHtkycSqFSaJrhJ/gAt5h5EaHIdv8S7terdiS9ES1APm3cPvFDHbfKGInboMX+wvFHLdjLttppFafQ6I= X-Rspamd-Queue-Id: 4DlZ6J5ktSz4nS0 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Feb 2021 22:51:21 -0000 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 > AuthorDate: 2021-02-23 21:17:37 +0000 > Commit: Allan Jude > 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 > > +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 FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org The need of the many outweighs the greed of the few.