svn commit: r297954 - in head/sys: boot/efi/loader/arch/amd64 boot/i386/libi386 x86/acpica

[John Baldwin]

On Thursday, April 14, 2016 10:53:56 AM Warner Losh wrote:
> On Thu, Apr 14, 2016 at 10:40 AM, John Baldwin <jhb at freebsd.org> wrote:
> 
> > On Thursday, April 14, 2016 04:59:51 AM Warner Losh wrote:
> > > Author: imp
> > > Date: Thu Apr 14 04:59:51 2016
> > > New Revision: 297954
> > > URL: https://svnweb.freebsd.org/changeset/base/297954
> > >
> > > Log:
> > >   Deprecate using hints.acpi.0.rsdp to communicate the RSDP to the
> > >   system. This uses the hints mechnanism. This mostly works today
> > >   because when there's no static hints (the default), this value can be
> > >   fetched from the hint. When there is a static hints file, the hint
> > >   passed from the boot loader to the kernel is ignored, but for the BIOS
> > >   case we're able to find it anyway. However, with UEFI, the fallback
> > >   doesn't work, so we get a panic instead.
> > >
> > >   Switch to acpi.rsdp and use TUNABLE_ULONG_FETCH instead. Continue to
> > >   generate the old values to allow for transitions. In addition, fall
> > >   back to the old method if the new method isn't present.
> > >
> > >   Add comments about all this.
> >
> > I still think this is a bit of a band-aid.  At least acpidump in userland
> > also
> > uses the hints I believe.  I still think a better long term approach is to
> > fix static hints + loader one way or another.
> >
> 
> Sure, but this data never should have been passed as a hint.

One could argue it is POLA to change it now.  You don't know that other
userland utilities outside of the tree aren't already using it as well.
There should be a clear benefit to changing the name (vs just adding an
additional alias).

> > One approach would be to teach the hint code to "merge" dynamic and static
> > hints by looking in the dynamic hints first.  However, while that is fairly
> > trivial for resource_*_value() that request values by name, it would be a
> > PITA
> > to handle the part of the hints API that iterates to find all the possible
> > device names (though this could perhaps iterate over dynamic then static).
> >
> 
> Yes, we could do that, or we could not abuse 'hints' mechanism and instead
> realize it can and will be overridden and use a different method to pass
> this
> critical data. I like module data better than the environment, but was short
> on time. That's where this should be passed in, and a sysctl to export it to
> userland should be done instead of acpidump reading hints. I freely admit
> that
> my kenv hack just made things better without fully fixing the problem.

kenv is easier to see as a developer for simple values (i.e. scalars as
opposed to tables like SMAP or the EFI memory map) rather than module data.
It is also easier to work with in scripts, etc.  There are many places using
the smbios kenv values that way because they are easily accessible.

However, I also don't really see hints and kenv as that separate I guess.
I think treating one particular subset of the kenv as magic relative to the
rest of kenv is the more inconsistent approach.

> The second approach would be to desupport using static hints for kernels
> > booted via the loader and relegate it to something that only systems using
> > uboot or the like use.  Anything in static hints can be moved into either
> > /boot/device.hints, /boot/loader.conf{,.local} or into a /boot/foo.hints
> > that is added to the list of loader conf files in
> > /boot/loader.conf{,.local}.
> > I would add a big, fat WARNING message (like we have for WITNESS) that
> > prints
> > out during kernel boot if we detect static hints being used when a kernel
> > environment is present.
> >
> 
> Seems like an over-reaction to using the wrong mechanism

One could say breaking POLA with an incomplete change is an over-reaction. :)

> > I probably lean towards the second.  We never shipped a release that used
> > static hints for a kernel on platforms where /boot/loader is supported
> > (GENERIC on i386 only used static hints for about a month after the hints
> > code was added pre-5.0 before the hints moved out to /boot/device.hints
> > instead).  Moving your static hints out into a loader config file means
> > that
> > you also have more flexibility to during recovery situations as you could,
> > at least in theory, adjust the values pre-boot from the loader.
> 
> 
> I see no reason to desupport anything.

Except for things already using the existing hints?  I think you're just
catering to the problem in front of you right now, not thinking about the
more general case.

-- 
John Baldwin