From owner-dev-commits-src-main@freebsd.org Tue Feb 23 22:56:03 2021 Return-Path: Delivered-To: dev-commits-src-main@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 9A678557889; Tue, 23 Feb 2021 22:56:03 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.9]) (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 4DlZCk4R2jz4nnm; Tue, 23 Feb 2021 22:56:02 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.229.168]) by shaw.ca with ESMTPA id Egb0lVloy2SWTEgb2lPcq9; Tue, 23 Feb 2021 15:56:00 -0700 X-Authority-Analysis: v=2.4 cv=fdJod2cF c=1 sm=1 tr=0 ts=60358800 a=7AlCcx2GqMg+lh9P3BclKA==:117 a=7AlCcx2GqMg+lh9P3BclKA==:17 a=xqWC_Br6kY4A:10 a=kj9zAlcOel0A:10 a=qa6Q16uM49sA:10 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=EkcXrb_YAAAA:8 a=VxmjJ2MpAAAA:8 a=iGbCS3zQAAAA:8 a=1SrTYSZmx2jzKtmugbUA:9 a=umdqf8azl6mVhff5:21 a=aaFSTmKjo1xzFNRr:21 a=CjuIK1q_8ugA:10 a=3RcraWcUxpgA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 a=LK5xJRSDVpKd5WXXoEvA:22 a=7gXAzLPJhVmCkEl4_tsf:22 a=oRZS8w7TM5j8Y1SnqK-S:22 Received: from slippy.cwsent.com (slippy [IPv6:fc00:1:1:1::5b]) by spqr.komquats.com (Postfix) with ESMTPS id 85A46D2F; Tue, 23 Feb 2021 14:55:58 -0800 (PST) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.16.1/8.16.1) with ESMTP id 11NMtwxL013572; Tue, 23 Feb 2021 14:55:58 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <202102232255.11NMtwxL013572@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: <202102232251.11NMp7Dl013494@slippy.cwsent.com> References: <202102232119.11NLJw17099523@gitrepo.freebsd.org> <202102232251.11NMp7Dl013494@slippy.cwsent.com> Comments: In-reply-to Cy Schubert message dated "Tue, 23 Feb 2021 14:51:07 -0800." Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Tue, 23 Feb 2021 14:55:58 -0800 X-CMAE-Envelope: MS4xfL3NJOhwCziznn2mUpMoNclUYK0PUe2869z96Tzmezp2r3GVYfHoV4Ruclg96V30oh3NKeBmvzpAmaHHfKJZsFX2yWNBg6QWFmkz56cOrc+2TkC7oQaL MnjA0z33u6yfrjF19KZME5/K2HAOEHZi9ZlwjUpi0jZxjQdZ9DZY9YiKVy5OlKgsGPWckvhNtIU2CyZ+OVIdE4XIgI7wVySFA1LuKaOsxXaew7ISm0PhvWxz RBK8sRQYyZ4Ms9EYzqOkICQeh6EuM3t8qMIHEniqIgjrE1IGn0mux+0UuXYtc4dYVZ0Npc8i/V1V373QwzZ4+08fGzP8lxf44bk/o+5vids= X-Rspamd-Queue-Id: 4DlZCk4R2jz4nnm X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of cy.schubert@cschubert.com has no SPF policy when checking 64.59.134.9) smtp.mailfrom=cy.schubert@cschubert.com X-Spamd-Result: default: False [0.30 / 15.00]; HAS_REPLYTO(0.00)[Cy.Schubert@cschubert.com]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; MV_CASE(0.50)[]; RWL_MAILSPIKE_GOOD(0.00)[64.59.134.9:from]; RCVD_COUNT_THREE(0.00)[4]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RBL_DBL_DONT_QUERY_IPS(0.00)[64.59.134.9:from]; ASN(0.00)[asn:6327, ipnet:64.59.128.0/20, country:CA]; R_DKIM_NA(0.00)[]; RCVD_IN_DNSWL_LOW(-0.10)[64.59.134.9:from]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; REPLYTO_EQ_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[cschubert.com: no valid DMARC record]; AUTH_NA(1.00)[]; NEURAL_SPAM_SHORT(1.00)[1.000]; SPAMHAUS_ZRD(0.00)[64.59.134.9:from:127.0.2.255]; RCVD_TLS_LAST(0.00)[]; R_SPF_NA(0.00)[no SPF record]; MAILMAN_DEST(0.00)[dev-commits-src-all,dev-commits-src-main] X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Feb 2021 22:56:03 -0000 Nevermind. I see you found it. -- 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. 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 > > 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_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 > > > > +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. > > >