From owner-svn-src-head@freebsd.org Wed Oct 19 06:35:30 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CCE6DC18B07; Wed, 19 Oct 2016 06:35:30 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8E8C2E9B; Wed, 19 Oct 2016 06:35:30 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 4FE6B10AF8F; Wed, 19 Oct 2016 02:35:29 -0400 (EDT) From: John Baldwin To: Warner Losh Cc: Doug Ambrisko , "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , src-committers , Doug Ambrisko , Ravi Pokala Subject: Re: svn commit: r307326 - head/sys/boot/efi/loader Date: Tue, 18 Oct 2016 23:20:14 -0700 Message-ID: <3841347.BNPZXPm7N3@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-PRERELEASE; KDE/4.14.10; amd64; ; ) In-Reply-To: References: <201610141710.u9EHArlL089412@repo.freebsd.org> <1950201.IjTl3rpdGP@ralph.baldwin.cx> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Wed, 19 Oct 2016 02:35:29 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Oct 2016 06:35:30 -0000 On Tuesday, October 18, 2016 11:44:52 PM Warner Losh wrote: > On Mon, Oct 17, 2016 at 11:40 AM, John Baldwin wrote: > > On Friday, October 14, 2016 12:25:54 PM Warner Losh wrote: > >> On Oct 14, 2016 11:55 AM, "Doug Ambrisko" wrote: > >> > > >> > On Fri, Oct 14, 2016 at 10:33:15AM -0700, Ravi Pokala wrote: > >> > | -----Original Message----- > >> > | > From: on behalf of Doug Ambrisko < > >> ambrisko@ambrisko.com> > >> > | > Date: 2016-10-14, Friday at 10:27 > >> > | > To: Warner Losh > >> > | > Cc: Doug Ambrisko , src-committers < > >> src-committers@freebsd.org>, "svn-src-all@freebsd.org" < > >> svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" < > >> svn-src-head@freebsd.org> > >> > | > Subject: Re: svn commit: r307326 - head/sys/boot/efi/loader > >> > | > > >> > | > On Fri, Oct 14, 2016 at 11:16:02AM -0600, Warner Losh wrote: > >> > | > | Love the functionality, but don't like using the 'hint' namespace > >> for > >> > | > | this. Can we change it now before too many things depend on it? We > >> had > >> > | > | similar issues in ACPI and moved it to the 'acpi' space. Can we move > >> > | > | this to the 'smbios' space please? > >> > | > | > >> > | > | The reason is that 'hint' is special and sometimes filtered out, so > >> it > >> > | > | is a poor choice to export data from the boot loader to the kernel. > >> > | > > >> > | > The reason I picked hint was it could be put /boot/device.hints > >> > | > to make it work as well and that it was a hint. Other standards in > >> the > >> > | > future might use other methods. Looking back over the email I had > >> > | > with John he had suggested hint.smbios.0.anchor to make this look > >> > | > different. This code had been hanging around for so long I forgot > >> > | > about that and we were using hint.smbios.0.mem in our shipping code > >> base. > >> > | > > >> > | > However, I hope that nothing would use this except for smbios(4) and > >> > | > for people to make smbios(4) useful for this info. > >> > | > >> > | Doug's looking at me when he says that. :-) > >> > | > >> > | We talked about this last night at BAFUG; right now, even if smbios(4) > >> > | is able to find the SMBIOS info -- it currently only looks at the > >> > | aforementioned 0xf0000 - 0xfffff range, so it can't find it on UEFI -- > >> > | smbios(4) doesn't actually provide any interface for that information. > >> > | Doug and I have talked about making smbios(4) useful, by parsing the > >> > | data and providing KPIs and APIs, for years now; I think I'll *finally* > >> > | have the time and motivation to do so "soon". > >> > > >> > I've actually talked to a few people. However, your the first to > >> > step up. This needs to be designed and will take some time and > >> > review. I would hope that except for smbios(4), nothing else would > >> > use this kenv but there is nothing to prevent that :-( I could name > >> > it super_secret_dont_use. > >> > > >> > BTW, to get you started this patch prevents smbios(4) from blowing chunks > >> > when it gets a anchor in high memory and works on legacy machines. > >> > > >> > --- /sys/x86/bios/smbios.c 2013-10-01 14:28:25.000000000 -0700 > >> > +++ ./smbios.c 2016-04-11 11:58:03.234969000 -0700 > >> > @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD: release/9.2.0/sys/x8 > >> > #include > >> > #include > >> > #include > >> > +#include > >> > > >> > #include > >> > #include > >> > @@ -59,7 +60,7 @@ struct smbios_softc { > >> > }; > >> > > >> > #define RES2EPS(res) ((struct smbios_eps > >> *)rman_get_virtual(res)) > >> > -#define ADDR2EPS(addr) ((struct smbios_eps > >> *)BIOS_PADDRTOVADDR(addr)) > >> > +#define ADDR2EPS(addr) ((struct smbios_eps *)PHYS_TO_DMAP(addr)) > >> > > >> > static devclass_t smbios_devclass; > >> > > >> > @@ -71,19 +72,26 @@ static int smbios_modevent (module_t, in > >> > > >> > static int smbios_cksum (struct smbios_eps *); > >> > > >> > +static unsigned long addr; > >> > +static SYSCTL_NODE(_hw, OID_AUTO, smbios, CTLFLAG_RD, 0, > >> > + "SMBIOS driver parameters"); > >> > +SYSCTL_LONG(_hw_smbios, OID_AUTO, mem, CTLFLAG_RW, > >> > + &addr, 0, ""); > >> > + > >> > static void > >> > smbios_identify (driver_t *driver, device_t parent) > >> > { > >> > device_t child; > >> > - u_int32_t addr; > >> > int length; > >> > int rid; > >> > > >> > if (!device_is_alive(parent)) > >> > return; > >> > > >> > - addr = bios_sigsearch(SMBIOS_START, SMBIOS_SIG, SMBIOS_LEN, > >> > - SMBIOS_STEP, SMBIOS_OFF); > >> > + if (resource_long_value("smbios", 0, "mem", &addr) != 0 || > >> > + addr == 0) > >> > + addr = bios_sigsearch(SMBIOS_START, SMBIOS_SIG, > >> SMBIOS_LEN, > >> > + SMBIOS_STEP, SMBIOS_OFF); > >> > if (addr != 0) { > >> > rid = 0; > >> > length = ADDR2EPS(addr)->length; > >> > > >> > Note I don't plan to commit this since it doesn't really do much and we > >> > need a lot more. > >> > >> I was planning on exporting all smbios stuff via sysctl. > > > > I'm a bit hesitant to do all the type parsing in the kernel vs userland. > > However, I think having smbios(4) export a /dev/smbios that you can either > > read() or mmap() to access the table would be very convenient and let you > > keep the bits to parse the table in userland (and not require root if we > > allow read-only access to mortals on /dev/foo). > > I'd support allowing this also, but we have them hidden in kenv how. > Moving them to sysctl is trivial and adds no additional risk to the > kernel. Having a /dev/smbios that also exports them and having > userland tools to parse nodes that the loader doesn't export is also a > good idea, but I don't think that would replace what I have in mind. > sysctl is super easy to get data from in shell and other scripts. > /dev/smbios would require another program to parse the blob that's > exported. I think it would be cool to have that as well and wouldn't > interfere with what I was planning on doing. Not everything is as sensible to export via sysctl, though a subset of things might be. I have patches to pciconf to let it parse the smbios table for describing physical slots and which new-bus devices are associated with physical slots (unfortunately the tables themselves aren't very reliable in my experience to date), and that sort of thing probably belongs in userland. dmidecode could be patched to use /dev/smbios rather than grovelling around in /dev/mem rather easily. Having a tool to parse the table isn't the end of the world. We do this with other tables (e.g. the SMAP and EFI memory maps are exported as "raw" sysctl nodes that the sysctl binary knows how to format, but that is still done in userland). acpidump -t, mptable, pirtool, etc. are all done in userland as well. -- John Baldwin