From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Mario Limonciello <Mario_Limonciello@dell.com>
Cc: Peter Jones <pjones@redhat.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Len Brown <lenb@kernel.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Andy Lutomirski <luto@amacapital.net>
Subject: Re: [PATCH] ACPI: don't show an error when we're not in charge of PCIe hotplug.
Date: Wed, 22 Jun 2016 00:51:08 +0200 [thread overview]
Message-ID: <CAJZ5v0ji0DpVsNbnKQpWTdJS-d9y+dh-BVGGfJ_o5fHGe8qeiA@mail.gmail.com> (raw)
In-Reply-To: <cf3d90a8598241ff9b6e28b8fb037633@ausx13mpc120.AMER.DELL.COM>
On Tue, Jun 21, 2016 at 8:01 PM, <Mario_Limonciello@dell.com> wrote:
>> -----Original Message-----
>> From: Peter Jones [mailto:pjones@redhat.com]
>> Sent: Tuesday, June 21, 2016 10:19 AM
>> To: Rafael J. Wysocki <rafael@kernel.org>
>> Cc: ACPI Devel Maling List <linux-acpi@vger.kernel.org>; Limonciello, Mario
>> <Mario_Limonciello@Dell.com>; Linux Kernel Mailing List <linux-
>> kernel@vger.kernel.org>; Len Brown <lenb@kernel.org>; Rafael J . Wysocki
>> <rjw@rjwysocki.net>; Andy Lutomirski <luto@amacapital.net>
>> Subject: Re: [PATCH] ACPI: don't show an error when we're not in charge of
>> PCIe hotplug.
>>
>> (Sorry for the slow response - it's deadline time over here.)
>>
>> On Thu, Jun 16, 2016 at 04:56:57PM +0200, Rafael J. Wysocki wrote:
>> > On Thu, Jun 16, 2016 at 2:12 AM, Rafael J. Wysocki <rafael@kernel.org>
>> wrote:
>> > > On Thu, Jun 16, 2016 at 12:15 AM, Peter Jones <pjones@redhat.com>
>> wrote:
>> > >> Right now when booting, on many laptops the firmware manages the
>> PCIe
>> > >> bus. As a result, when we call the _OSC ACPI method, it returns an
>> > >> error code. Unfortunately the errors are not very articulate.
>> > >
>> > > What exactly do you mean here?
>> > >
>> > >> As a result, we show:
>> > >>
>> > >> ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-fe])
>> > >> acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM
>> Segments MSI]
>> > >> \_SB_.PCI0 (33DB4D5B-1FF7-401C-9657-7441C03DD766): _OSC invalid
>> UUID
>> > >> _OSC request data: 1 1f 0
>> > >
>> > > So _OSC told us that the UUID was invalid, didn't it?
>> >
>> > BTW, the above messages are KERN_DEBUG, so at least in theory they
>> > shouldn't be visible in production runs.
>> >
>> > Maybe the bug to fix is that they show up when they aren't supposed to?
>>
>> No - the workflow that I am really trying to remedy is this:
>>
>> 1) S3 resume sometimes isn't working on some laptop you've got.
>> 2) start looking at debug messages
>> 3) this shows an error, so it looks like it's probably the problem
>> 4) go fishing for red herring
>> 5) if you happen to know who maintains the DSDT for the platform in
>> question, eventually work out that this is working as intended and
>> the bug is someplace else.
>> 5b) if you don't know that person, eventually work out that it /might/
>> be someplace else...
>>
>> So the idea was to make it look more like an indication of status, and
>> less like an error that's causing unrelated problems.
>>
>> When I talked to Mario at Dell (Cc'd), it wasn't clear to us that
>> there's a way to distinguish the between the UUID being
>> invalid/malformed, being merely unsupported, or being supported in some
>> configurations but not the current one. In this particular DSDT, the
>> machine doesn't support the OS controlling any of this if USB-C /
>> thunderbolt are enabled. The DSDT is clearly written with the belief
>> that you have to completely disable the handling for that UUID in this
>> case, and googling for this looks like it's not the only one written
>> with that belief.
>>
>> Reading the spec (v6.1, sections 6.2.11.3 and 6.2.11.4), it seems
>> plausible that you can express this instead by handling the UUID but
>> choosing each individual query/status bit in the way that accomplishes
>> the OS doing nothing with the response. So it may well be that that's
>> just more code that vendors have thought wasn't necessary (or wasn't
>> correct for some reason.)
>>
>> Mario, want to jump in on your thinking here?
>>
>> --
>> Peter
>
> After talking to the team, I was told this particular implementation to not let
> OS take control when acting on that specific UUID based upon a variable
> (NEXP in this case) came from Intel RC code.
>
> That's probably why this is all across a lot of platforms, including non-Dell.
>
> At least in the context of the laptop Peter noticed this on (Dell XPS 13 9350)
> NEXP is set in GNVS based upon Thunderbolt capability.
>
> As for why they return unrecognized UUID instead of just masking all the
> capabilities bits? It's the same net functional result. If the vendor provided
> RC code doesn't caused WCHK problems or functional problems it's hard to
> make a case for why it needs to be changed by the OEM.
>
> I think that Peter's patch is appropriate to message this is specifically
> what's going on.
No, it may hide real (ie. non-intentional) bugs in _OSC, so it is not
appropriate.
Debug-level messages really should not hurt anyone (and should never
show up in production anyway).
We can slightly tone down the "_OSC failed (%s); disabling ASPM\n"
message in negotiate_os_control() in drivers/acpi/pci_root.c if you
think it's too strong and that's it.
Thanks,
Rafael
next prev parent reply other threads:[~2016-06-21 22:51 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 22:15 [PATCH] ACPI: don't show an error when we're not in charge of PCIe hotplug Peter Jones
2016-06-16 0:12 ` Rafael J. Wysocki
2016-06-16 14:56 ` Rafael J. Wysocki
2016-06-21 15:18 ` Peter Jones
2016-06-21 18:01 ` Mario_Limonciello
2016-06-21 18:07 ` Andy Lutomirski
2016-06-21 22:54 ` Rafael J. Wysocki
2016-06-21 22:51 ` Rafael J. Wysocki [this message]
2016-06-22 19:43 ` Mario_Limonciello
2016-06-22 20:53 ` Andy Lutomirski
2016-06-22 22:47 ` Rafael J. Wysocki
2016-06-23 15:38 ` Mario_Limonciello
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJZ5v0ji0DpVsNbnKQpWTdJS-d9y+dh-BVGGfJ_o5fHGe8qeiA@mail.gmail.com \
--to=rafael@kernel.org \
--cc=Mario_Limonciello@dell.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=pjones@redhat.com \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).