On Wed, Oct 19, 2016 at 12:20 AM, John Baldwin <j...@freebsd.org> wrote:
> On Tuesday, October 18, 2016 11:44:52 PM Warner Losh wrote:
>> On Mon, Oct 17, 2016 at 11:40 AM, John Baldwin <j...@freebsd.org> wrote:
>> > On Friday, October 14, 2016 12:25:54 PM Warner Losh wrote:
>> >> On Oct 14, 2016 11:55 AM, "Doug Ambrisko" <ambri...@ambrisko.com> wrote:
>> >> >
>> >> > On Fri, Oct 14, 2016 at 10:33:15AM -0700, Ravi Pokala wrote:
>> >> > | -----Original Message-----
>> >> > | > From: <owner-src-committ...@freebsd.org> on behalf of Doug Ambrisko
>> >> > <
>> >> ambri...@ambrisko.com>
>> >> > | > Date: 2016-10-14, Friday at 10:27
>> >> > | > To: Warner Losh <i...@bsdimp.com>
>> >> > | > Cc: Doug Ambrisko <ambri...@freebsd.org>, src-committers <
>> >> src-committ...@freebsd.org>, "svn-src-all@freebsd.org" <
>> >> svn-src-all@freebsd.org>, "svn-src-h...@freebsd.org" <
>> >> svn-src-h...@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 <machine/bus.h>
>> >> > #include <machine/resource.h>
>> >> > #include <sys/rman.h>
>> >> > +#include <sys/sysctl.h>
>> >> >
>> >> > #include <vm/vm.h>
>> >> > #include <vm/vm_param.h>
>> >> > @@ -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.
Agreed. That sort of thing isn't exported via a kenv today anyway.
> 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.
Understood. And I aagree. I think there's useful info that we should
put in sysctl as well. It's also useful to have it in the sysctl -a
output. The sysctl info I'm planning is about the system, and not any
of these tables.
One of my systems would export these as sysctls:
smbios.bios.reldate="07/05/2013"
smbios.bios.vendor="American Megatrends Inc."
smbios.bios.version="3.00"
smbios.chassis.maker="Supermicro"
smbios.chassis.serial="0123456789"
smbios.chassis.tag="To Be Filled By O.E.M."
smbios.chassis.version="0123456789"
smbios.memory.enabled="268435456"
smbios.planar.location="To be filled by O.E.M."
smbios.planar.maker="Supermicro"
smbios.planar.product="X9SRH-7F/7TF"
smbios.planar.serial="VM13CS028237"
smbios.planar.tag="To be filled by O.E.M."
smbios.planar.version="0123456789"
smbios.socket.enabled="1"
smbios.socket.populated="1"
smbios.system.family="To be filled by O.E.M."
smbios.system.maker="Supermicro"
smbios.system.product="X9SRH-7F/7TF"
smbios.system.serial="0123456789"
smbios.system.sku="To be filled by O.E.M."
smbios.system.uuid="00000000-0000-0000-0000-002590e4d0ec"
smbios.system.version="0123456789"
smbios.version="2.7"
though some of them are silly due to the BIOS writer being silly...
Warner
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
