[PATCH] ACPI CMOS region support rev. 5

[Anthony Jenkins]

On 03/19/2015 09:49 AM, Ian Smith wrote:
> On Thu, 19 Mar 2015 09:11:08 -0400, Anthony Jenkins wrote:
>  > On 03/19/2015 04:10 AM, Ian Smith wrote:
>  > > On Wed, 18 Mar 2015 15:30:23 -0600, Warner Losh wrote:
>  > >  > > On Mar 18, 2015, at 10:06 AM, Anthony Jenkins <Anthony.B.Jenkins at att.net> wrote:
>  > >  > > 
>  > >  > > On 03/18/2015 11:29 AM, Warner Losh wrote:
>  > >  > >>> On Mar 17, 2015, at 7:02 AM, Anthony Jenkins <Anthony.B.Jenkins at att.net> wrote:
>  > >  > >>>> \Where else might ATRTC_VERBOSE be set otherwise?
>  > >  > >>> I'm picturing a (future?) config(5) knob, e.g.
>  > >  > >>> 
>  > >  > >>>   device atrtc
>  > >  > >>>   options ATRTC_VERBOSE=1
>  > >  > >>> 
>  > >  > >>> 
>  > >  > >>> so it can be set at compile time.
>  > >  > >> Why not just boot verbose? history has shown too many options like
>  > >  > >> this is hard to use.
>  > >
>  > > You can blame this on me :)  I agree about the option not being needed; 
>  > > the way it is you can just set sysctl hw.acpi.atrtc_verbose=0 to quell 
>  > > reports of successful access, if it turns out these are routine on some 
>  > > machines, especially outside of boot/suspend/resume contexts.
>  > >
>  > > However I'll still argue that, this being a new gadget and that we could 
>  > > use finding out which vendors want to read or write which locations in 
>  > > CMOS for whatever reason, at least while it's in head, we should log all 
>  > > access by default unless setting atrtc_verbose=0,
>  > 
>  > So the default verbosity of ACPI CMOS region accesses should be
>  > "verbose"?  I personally don't mind the default being "silent" and
>  > asking people triaging an ACPI problem to boot verbosely and send the
>  > logs (I think that's in the FreeBSD ACPI handbook anyway).
>
> Assuming they know that their problem is ACPI related, sure.
>
>  > > and in _any_ case we 
>  > > should be logging attempts to R/W out-of-bounds CMOS locations.
>  > 
>  > Error logs are always printed; they don't honor atrtc_verbose.
>
> That would be comforting.  However I was referring to rev. 5:
>
> +       if (bitwidth == 0 || bitwidth > 32 || (bitwidth & 0x07) ||
> +                       addr + bytewidth - 1 > 63) {
> +               ATRTC_DBG_PRINTF(1,
> +                       "Invalid bitwidth (%u) or addr (0x%08lx).\n",
> +                       bitwidth, addr);
> +               return AE_BAD_PARAMETER;
> +       }
> +       if (!acpi_check_rtc_access(func == ACPI_READ, addr, bytewidth)) {
> +               ATRTC_DBG_PRINTF(1, "Bad CMOS %s access at addr 0x%08lx.\n",
> +                       func == ACPI_READ ? "read" : "write", addr);
> +               return AE_BAD_PARAMETER;
> +       }

Ahh you're right, my bad.  These are /supposed/ to be simple printf()s
(or ATRTC_DBG_PRINTF(0, ...)s, but I think this would trigger a
"unsigned int >= 0 is always true" warning).

>  > >  > > I think I understand what you're saying... I also prefer fewer config(5)
>  > >  > > knobs.  So you're suggesting I determine (at runtime) the boot verbose
>  > >  > > setting (kenv(2) or however it's properly done) and dump the
>  > >  > > compile-time verbosity setting?
>  > >  > 
>  > >  > if (bootverbose)
>  > >  > 	do verbose things;
>  > >  > 
>  > >  > is how thatÿÿs done.
>  > >
>  > > Sure, and maybe successful access could be limited to bootverbose, and 
>  > > we could ask people whose boxes fail to boot/suspend/resume/whatever to 
>  > > boot verbose to reveal such as why Anthony's HP Envy either failed to 
>  > > suspend or immediately resumed - which isn't entirely clear, even with 
>  > > the messages - unless its ACPI AML succeeded in reading minute, hour and 
>  > > weekday, but I have a feeling we may see more of this sort of thing.
>  > 
>  > Now that I think about it, adding this ACPI CMOS region access should
>  > simply eliminate a class of failures where FreeBSD wasn't giving the
>  > BIOS access to CMOS.
>
> Do we have other examples than your HP Envy of such a class of failures?

Oh definitely!  You should be able to search for my name in this
newsgroup and find instances where I've manually provided the patch to
someone having suspend/resume issues and they've been resolved.  One has
subject "Impossible shutdown" circa 2014-June-26.

Anthony

>  > Logging /successful/  R/W accesses to CMOS by the
>  > BIOS (AML) won't really provide any useful info (IMHO), but the user can
>  > flip on bootverbose if she's curious.  If a user's box fails to
>  > boot/suspend/resume/whatever, we'll see any ACPI CMOS region access errors.
>
> Well I've made a case otherwise, likely too avidly; I'll leave it there.
>
> Thanks for flushing out this issue and doing something about it!
>
> cheers, Ian
> _______________________________________________
> freebsd-acpi at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"