2022-04-16 02:15:31

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 0/3] x86/PCI: Log E820 clipping

From: Bjorn Helgaas <[email protected]>

This is still work-in-progress on the issue of PNP0A03 _CRS methods that
are buggy or not interpreted correctly by Linux.

The previous try at:
https://lore.kernel.org/r/[email protected]
caused regressions on some Chromebooks:
https://lore.kernel.org/r/[email protected]

This v2 drops the commit that caused the Chromebook regression, so it also
doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
Barebones.

The point of this v2 update is to split the logging patch into (1) a pure
logging addition and (2) the change to only clip PCI windows, which was
previously hidden inside the logging patch and not well documented.

Bjorn Helgaas (3):
x86/PCI: Eliminate remove_e820_regions() common subexpressions
x86: Log resource clipping for E820 regions
x86/PCI: Clip only host bridge windows for E820 regions

arch/x86/include/asm/e820/api.h | 5 +++++
arch/x86/kernel/resource.c | 23 ++++++++++++++++-------
arch/x86/pci/acpi.c | 5 +++++
3 files changed, 26 insertions(+), 7 deletions(-)

--
2.25.1


2022-04-16 02:16:47

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 1/3] x86/PCI: Eliminate remove_e820_regions() common subexpressions

From: Bjorn Helgaas <[email protected]>

Add local variables to reduce repetition later. No functional change
intended.

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Helgaas <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
---
arch/x86/kernel/resource.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/resource.c b/arch/x86/kernel/resource.c
index 9b9fb7882c20..8ffe68437744 100644
--- a/arch/x86/kernel/resource.c
+++ b/arch/x86/kernel/resource.c
@@ -27,12 +27,14 @@ static void remove_e820_regions(struct resource *avail)
{
int i;
struct e820_entry *entry;
+ u64 e820_start, e820_end;

for (i = 0; i < e820_table->nr_entries; i++) {
entry = &e820_table->entries[i];
+ e820_start = entry->addr;
+ e820_end = entry->addr + entry->size - 1;

- resource_clip(avail, entry->addr,
- entry->addr + entry->size - 1);
+ resource_clip(avail, e820_start, e820_end);
}
}

--
2.25.1

2022-04-19 18:27:43

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:
> On 1/1/70 01:00, Bjorn Helgaas wrote:
> > This is still work-in-progress on the issue of PNP0A03 _CRS methods that
> > are buggy or not interpreted correctly by Linux.
> >
> > The previous try at:
> > https://lore.kernel.org/r/[email protected]
> > caused regressions on some Chromebooks:
> > https://lore.kernel.org/r/[email protected]
> >
> > This v2 drops the commit that caused the Chromebook regression, so it also
> > doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
> > Barebones.
> >
> > The point of this v2 update is to split the logging patch into (1) a pure
> > logging addition and (2) the change to only clip PCI windows, which was
> > previously hidden inside the logging patch and not well documented.
> >
> > Bjorn Helgaas (3):
> > x86/PCI: Eliminate remove_e820_regions() common subexpressions
> > x86: Log resource clipping for E820 regions
> > x86/PCI: Clip only host bridge windows for E820 regions
>
> Thanks, the entire series looks good to me:
>
> Reviewed-by: Hans de Goede <[email protected]>

Thank you!

> So what is the plan to actually fix the issue seen on some Lenovo models
> and Clevo Barebones ? As I mentioned previously I think that since all
> our efforts have failed so far that we should maybe reconsider just
> using DMI quirks to ignore the E820 reservation windows for host bridges
> on affected models ?

I have been resisting DMI quirks but I'm afraid there's no other way.
I think the web we've gotten into, where vendors have used E820 to
interact with _CRS in incompatible and undocumented ways, is not
sustainable.

I'm not aware of any spec that says the OS should use E820 to clip
things out of _CRS, so I think the long term plan should be to
decouple them by default.

Straw man:

- Disable E820 clipping by default.

- Add a quirk to enable E820 clipping for machines older than X,
e.g., 2023, to avoid breaking machines that currently work.

- Add quirks to disable E820 clipping for individual machines like
the Lenovo and Clevos that predate X, but E820 clipping breaks
them.

- Add quirks to enable E820 clipping for individual machines like
the Chromebooks (and probably machines we don't know about yet)
that have devices that consume part of _CRS but are not
enumerable.

- Communicate this to OEMs to try to prevent future machines that
need quirks.

Bjorn

2022-04-19 18:48:53

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

Hi,

On 4/19/22 17:03, Bjorn Helgaas wrote:
> On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:
>> On 1/1/70 01:00, Bjorn Helgaas wrote:
>>> This is still work-in-progress on the issue of PNP0A03 _CRS methods that
>>> are buggy or not interpreted correctly by Linux.
>>>
>>> The previous try at:
>>> https://lore.kernel.org/r/[email protected]
>>> caused regressions on some Chromebooks:
>>> https://lore.kernel.org/r/[email protected]
>>>
>>> This v2 drops the commit that caused the Chromebook regression, so it also
>>> doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
>>> Barebones.
>>>
>>> The point of this v2 update is to split the logging patch into (1) a pure
>>> logging addition and (2) the change to only clip PCI windows, which was
>>> previously hidden inside the logging patch and not well documented.
>>>
>>> Bjorn Helgaas (3):
>>> x86/PCI: Eliminate remove_e820_regions() common subexpressions
>>> x86: Log resource clipping for E820 regions
>>> x86/PCI: Clip only host bridge windows for E820 regions
>>
>> Thanks, the entire series looks good to me:
>>
>> Reviewed-by: Hans de Goede <[email protected]>
>
> Thank you!
>
>> So what is the plan to actually fix the issue seen on some Lenovo models
>> and Clevo Barebones ? As I mentioned previously I think that since all
>> our efforts have failed so far that we should maybe reconsider just
>> using DMI quirks to ignore the E820 reservation windows for host bridges
>> on affected models ?
>
> I have been resisting DMI quirks but I'm afraid there's no other way.

Well there is the first match adjacent windows returned by _CRS and
only then do the "covers whole region" exception check. I still
think that would work at least for the chromebook regression...

So do you want me to give that a try; or shall I write a patch
using DMI quirks. And if we go the DMI quirks, what about
matching cmdline arguments? If we add matching cmdline arguments,
which seems to be the sensible thing to do then to allow users
to test if they need the quirk, then we basically end up with my
first attempt at fixing this from 6 months ago:

https://lore.kernel.org/linux-pci/[email protected]/

> I think the web we've gotten into, where vendors have used E820 to
> interact with _CRS in incompatible and undocumented ways, is not
> sustainable.
>
> I'm not aware of any spec that says the OS should use E820 to clip
> things out of _CRS, so I think the long term plan should be to
> decouple them by default.

Right and AFAICT the reason Windows is getting away with this is
the same as with the original Dell _CRS has overlap with
physical RAM issue (1), Linux assigns address to unassigneds BAR-s
starting with the lowest available address in the bridge window,
where as Windows assigns addresses from the highest available
address in the window. So the real fix here might very well be
to rework the BAR assignment code to switch to fill the window
from the top rather then from the bottom. AFAICT all issues where
excluding _E820 reservations have helped are with _E820 - bridge
window overlaps at the bottom of the window.

IOW these are really all bugs in the _CRS method for the bridge,
which Windows does not hit because it never actually uses
the lowest address(es) of the _CRS returned window.

Regards,

Hans



1) At least I read in either a bugzilla, or email thread about
this that Windows allocating bridge window space from the top
was assumed to be why Windows was not impacted.





> Straw man:
>
> - Disable E820 clipping by default.
>
> - Add a quirk to enable E820 clipping for machines older than X,
> e.g., 2023, to avoid breaking machines that currently work.
>
> - Add quirks to disable E820 clipping for individual machines like
> the Lenovo and Clevos that predate X, but E820 clipping breaks
> them.
>
> - Add quirks to enable E820 clipping for individual machines like
> the Chromebooks (and probably machines we don't know about yet)
> that have devices that consume part of _CRS but are not
> enumerable.
>
> - Communicate this to OEMs to try to prevent future machines that
> need quirks.
>
> Bjorn
>

2022-04-20 00:44:02

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

Hi All,

On 1/1/70 01:00, Bjorn Helgaas wrote:
> This is still work-in-progress on the issue of PNP0A03 _CRS methods that
> are buggy or not interpreted correctly by Linux.
>
> The previous try at:
> https://lore.kernel.org/r/[email protected]
> caused regressions on some Chromebooks:
> https://lore.kernel.org/r/[email protected]
>
> This v2 drops the commit that caused the Chromebook regression, so it also
> doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
> Barebones.
>
> The point of this v2 update is to split the logging patch into (1) a pure
> logging addition and (2) the change to only clip PCI windows, which was
> previously hidden inside the logging patch and not well documented.
>
> Bjorn Helgaas (3):
> x86/PCI: Eliminate remove_e820_regions() common subexpressions
> x86: Log resource clipping for E820 regions
> x86/PCI: Clip only host bridge windows for E820 regions

Thanks, the entire series looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

For the series.

So what is the plan to actually fix the issue seen on some Lenovo models
and Clevo Barebones ? As I mentioned previously I think that since all
our efforts have failed so far that we should maybe reconsider just
using DMI quirks to ignore the E820 reservation windows for host bridges
on affected models ?

That or implement 3 of:

> So I think the progression is:
>
> 1) Remove anything mentioned in E820 from _CRS (4dc2287c1805 [7]).
> This worked around some issues on Dell systems.
>
> 2) Remove things mentioned in E820 unless they cover the entire
> window (5949965ec934 [8])
>
> 3) Coalesce adjacent _CRS windows, *then* remove things mentioned in
> E820 unless they cover the entire (coalesced) window (current
> proposal)

Regards,

Hans

2022-04-20 15:12:17

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

On Tue, Apr 19, 2022 at 05:16:44PM +0200, Hans de Goede wrote:
> Hi,
>
> On 4/19/22 17:03, Bjorn Helgaas wrote:
> > On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:
> >> On 1/1/70 01:00, Bjorn Helgaas wrote:
> >>> This is still work-in-progress on the issue of PNP0A03 _CRS methods that
> >>> are buggy or not interpreted correctly by Linux.
> >>>
> >>> The previous try at:
> >>> https://lore.kernel.org/r/[email protected]
> >>> caused regressions on some Chromebooks:
> >>> https://lore.kernel.org/r/[email protected]
> >>>
> >>> This v2 drops the commit that caused the Chromebook regression, so it also
> >>> doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
> >>> Barebones.
> >>>
> >>> The point of this v2 update is to split the logging patch into (1) a pure
> >>> logging addition and (2) the change to only clip PCI windows, which was
> >>> previously hidden inside the logging patch and not well documented.
> >>>
> >>> Bjorn Helgaas (3):
> >>> x86/PCI: Eliminate remove_e820_regions() common subexpressions
> >>> x86: Log resource clipping for E820 regions
> >>> x86/PCI: Clip only host bridge windows for E820 regions
> >>
> >> Thanks, the entire series looks good to me:
> >>
> >> Reviewed-by: Hans de Goede <[email protected]>
> >
> > Thank you!
> >
> >> So what is the plan to actually fix the issue seen on some Lenovo models
> >> and Clevo Barebones ? As I mentioned previously I think that since all
> >> our efforts have failed so far that we should maybe reconsider just
> >> using DMI quirks to ignore the E820 reservation windows for host bridges
> >> on affected models ?
> >
> > I have been resisting DMI quirks but I'm afraid there's no other way.
>
> Well there is the first match adjacent windows returned by _CRS and
> only then do the "covers whole region" exception check. I still
> think that would work at least for the chromebook regression...

Without a crystal clear strategy, I think we're going to be tweaking
the algorithm forever as the _CRS/E820 mix changes. That's why I
think that in the long term, a "use _CRS only, with quirks for
exceptions" strategy will be simplest.

> So do you want me to give that a try; or shall I write a patch
> using DMI quirks. And if we go the DMI quirks, what about
> matching cmdline arguments? If we add matching cmdline arguments,
> which seems to be the sensible thing to do then to allow users
> to test if they need the quirk, then we basically end up with my
> first attempt at fixing this from 6 months ago:
>
> https://lore.kernel.org/linux-pci/[email protected]/

So I think we should go ahead with DMI quirks instead of trying to
make the algorithm smarter, and yes, I think we will need commandline
arguments, probably one to force E820 clipping for future machines,
and one to disable it for old machines.

> > I think the web we've gotten into, where vendors have used E820 to
> > interact with _CRS in incompatible and undocumented ways, is not
> > sustainable.
> >
> > I'm not aware of any spec that says the OS should use E820 to clip
> > things out of _CRS, so I think the long term plan should be to
> > decouple them by default.
>
> Right and AFAICT the reason Windows is getting away with this is
> the same as with the original Dell _CRS has overlap with
> physical RAM issue (1), Linux assigns address to unassigneds BAR-s
> starting with the lowest available address in the bridge window,
> where as Windows assigns addresses from the highest available
> address in the window.

Right, I agree. I'm guessing Chromebooks don't get tested with
Windows at all, so we don't even have that level of testing to help.

> So the real fix here might very well be
> to rework the BAR assignment code to switch to fill the window
> from the top rather then from the bottom. AFAICT all issues where
> excluding _E820 reservations have helped are with _E820 - bridge
> window overlaps at the bottom of the window.
>
> IOW these are really all bugs in the _CRS method for the bridge,
> which Windows does not hit because it never actually uses
> the lowest address(es) of the _CRS returned window.

Yes. We actually did try this
(https://git.kernel.org/linus/1af3c2e45e7a), but unfortunately we had
to revert it. Even more unfortunately, the revert
(https://git.kernel.org/linus/5e52f1c5e85f) doesn't have any details
about what went wrong.

> 1) At least I read in either a bugzilla, or email thread about
> this that Windows allocating bridge window space from the top
> was assumed to be why Windows was not impacted.
>
> > Straw man:
> >
> > - Disable E820 clipping by default.
> >
> > - Add a quirk to enable E820 clipping for machines older than X,
> > e.g., 2023, to avoid breaking machines that currently work.
> >
> > - Add quirks to disable E820 clipping for individual machines like
> > the Lenovo and Clevos that predate X, but E820 clipping breaks
> > them.
> >
> > - Add quirks to enable E820 clipping for individual machines like
> > the Chromebooks (and probably machines we don't know about yet)
> > that have devices that consume part of _CRS but are not
> > enumerable.
> >
> > - Communicate this to OEMs to try to prevent future machines that
> > need quirks.
> >
> > Bjorn
> >
>

2022-05-03 00:06:37

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

On Mon, May 02, 2022 at 02:24:26PM +0200, Hans de Goede wrote:
> On 4/19/22 18:45, Bjorn Helgaas wrote:
> > On Tue, Apr 19, 2022 at 05:16:44PM +0200, Hans de Goede wrote:
> >> On 4/19/22 17:03, Bjorn Helgaas wrote:
> >>> On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:

> >>>> So what is the plan to actually fix the issue seen on some
> >>>> Lenovo models and Clevo Barebones ? As I mentioned previously
> >>>> I think that since all our efforts have failed so far that we
> >>>> should maybe reconsider just using DMI quirks to ignore the
> >>>> E820 reservation windows for host bridges on affected models ?
> >>>
> >>> I have been resisting DMI quirks but I'm afraid there's no other
> >>> way.
> >>
> >> Well there is the first match adjacent windows returned by _CRS
> >> and only then do the "covers whole region" exception check. I
> >> still think that would work at least for the chromebook
> >> regression...
> >
> > Without a crystal clear strategy, I think we're going to be
> > tweaking the algorithm forever as the _CRS/E820 mix changes.
> > That's why I think that in the long term, a "use _CRS only, with
> > quirks for exceptions" strategy will be simplest.
>
> Looking at the amount of exception we already now about I'm not sure
> if that will work well.

It's possible that many quirks will be required. But I think in the
long run the value of the simplest, most obvious strategy is huge.
It's laid out in the spec already and it's the clearest way to
agreement between firmware and OS. When we trip over something, it's
very easy to determine whether _CRS is wrong or Linux is using it
wrong. If we have to bring in question of looking at E820 entries,
possibly merging them, using them or not based on overlaps ... that's
a much more difficult conversation without a clear resolution.

> > So I think we should go ahead with DMI quirks instead of trying to
> > make the algorithm smarter, and yes, I think we will need commandline
> > arguments, probably one to force E820 clipping for future machines,
> > and one to disable it for old machines.
>
> So what you are suggesting is to go back to a bios-date based approach
> (to determine old vs new machines) combined with DMI quirks to force
> E820 clipping on new machines which turn out to need it despite them
> being new ?

Yes. It's ugly but I think the 10-year outlook is better.

> I have the feeling that if we switch to top-down allocating
> that we can then switch to just using _CRS and that everything
> will then just work, because we then match what Windows is doing...

Yes, it might. But I'm not 100% comfortable because it basically
sweeps _CRS bugs under the rug, and we may trip over them as we do
more hotplug and (eventually) resource rebalancing. I think we need
to work toward getting _CRS more reliable.

Bjorn

2022-05-03 00:43:27

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

Hi,

Sorry for the late reply.

On 4/19/22 18:45, Bjorn Helgaas wrote:
> On Tue, Apr 19, 2022 at 05:16:44PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/19/22 17:03, Bjorn Helgaas wrote:
>>> On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:
>>>> On 1/1/70 01:00, Bjorn Helgaas wrote:
>>>>> This is still work-in-progress on the issue of PNP0A03 _CRS methods that
>>>>> are buggy or not interpreted correctly by Linux.
>>>>>
>>>>> The previous try at:
>>>>> https://lore.kernel.org/r/[email protected]
>>>>> caused regressions on some Chromebooks:
>>>>> https://lore.kernel.org/r/[email protected]
>>>>>
>>>>> This v2 drops the commit that caused the Chromebook regression, so it also
>>>>> doesn't fix the issue we were *trying* to fix on Lenovo Yoga and Clevo
>>>>> Barebones.
>>>>>
>>>>> The point of this v2 update is to split the logging patch into (1) a pure
>>>>> logging addition and (2) the change to only clip PCI windows, which was
>>>>> previously hidden inside the logging patch and not well documented.
>>>>>
>>>>> Bjorn Helgaas (3):
>>>>> x86/PCI: Eliminate remove_e820_regions() common subexpressions
>>>>> x86: Log resource clipping for E820 regions
>>>>> x86/PCI: Clip only host bridge windows for E820 regions
>>>>
>>>> Thanks, the entire series looks good to me:
>>>>
>>>> Reviewed-by: Hans de Goede <[email protected]>
>>>
>>> Thank you!
>>>
>>>> So what is the plan to actually fix the issue seen on some Lenovo models
>>>> and Clevo Barebones ? As I mentioned previously I think that since all
>>>> our efforts have failed so far that we should maybe reconsider just
>>>> using DMI quirks to ignore the E820 reservation windows for host bridges
>>>> on affected models ?
>>>
>>> I have been resisting DMI quirks but I'm afraid there's no other way.
>>
>> Well there is the first match adjacent windows returned by _CRS and
>> only then do the "covers whole region" exception check. I still
>> think that would work at least for the chromebook regression...
>
> Without a crystal clear strategy, I think we're going to be tweaking
> the algorithm forever as the _CRS/E820 mix changes. That's why I
> think that in the long term, a "use _CRS only, with quirks for
> exceptions" strategy will be simplest.

Looking at the amount of exception we already now about I'm
not sure if that will work well.


>
>> So do you want me to give that a try; or shall I write a patch
>> using DMI quirks. And if we go the DMI quirks, what about
>> matching cmdline arguments? If we add matching cmdline arguments,
>> which seems to be the sensible thing to do then to allow users
>> to test if they need the quirk, then we basically end up with my
>> first attempt at fixing this from 6 months ago:
>>
>> https://lore.kernel.org/linux-pci/[email protected]/
>
> So I think we should go ahead with DMI quirks instead of trying to
> make the algorithm smarter, and yes, I think we will need commandline
> arguments, probably one to force E820 clipping for future machines,
> and one to disable it for old machines.

So what you are suggesting is to go back to a bios-date based approach
(to determine old vs new machines) combined with DMI quirks to force
E820 clipping on new machines which turn out to need it despite them
being new ?

>
>>> I think the web we've gotten into, where vendors have used E820 to
>>> interact with _CRS in incompatible and undocumented ways, is not
>>> sustainable.
>>>
>>> I'm not aware of any spec that says the OS should use E820 to clip
>>> things out of _CRS, so I think the long term plan should be to
>>> decouple them by default.
>>
>> Right and AFAICT the reason Windows is getting away with this is
>> the same as with the original Dell _CRS has overlap with
>> physical RAM issue (1), Linux assigns address to unassigneds BAR-s
>> starting with the lowest available address in the bridge window,
>> where as Windows assigns addresses from the highest available
>> address in the window.
>
> Right, I agree. I'm guessing Chromebooks don't get tested with
> Windows at all, so we don't even have that level of testing to help.
>
>> So the real fix here might very well be
>> to rework the BAR assignment code to switch to fill the window
>> from the top rather then from the bottom. AFAICT all issues where
>> excluding _E820 reservations have helped are with _E820 - bridge
>> window overlaps at the bottom of the window.
>>
>> IOW these are really all bugs in the _CRS method for the bridge,
>> which Windows does not hit because it never actually uses
>> the lowest address(es) of the _CRS returned window.
>
> Yes. We actually did try this
> (https://git.kernel.org/linus/1af3c2e45e7a), but unfortunately we had
> to revert it. Even more unfortunately, the revert
> (https://git.kernel.org/linus/5e52f1c5e85f) doesn't have any details
> about what went wrong.

When I first started working on this I did read the entire old
email thread and IIRC this approach was reverted because the
e820 based approach was deemed to be a cleaner fix. Also the
single resource_alloc_from_bottom flag influenced all types
of resource allocations, not just PCI host bridge window
allocations.

Note that the current kernel no longer has the resource_alloc_from_bottom
flag. Still I think it might be worthwhile to give switching to
top-down allocating for host bridge window allocs a try. Maybe we
can make the desired allocation strategy a flag in the resource ?

I have the feeling that if we switch to top-down allocating
that we can then switch to just using _CRS and that everything
will then just work, because we then match what Windows is doing...

Regards,

Hans







>
>> 1) At least I read in either a bugzilla, or email thread about
>> this that Windows allocating bridge window space from the top
>> was assumed to be why Windows was not impacted.
>>
>>> Straw man:
>>>
>>> - Disable E820 clipping by default.
>>>
>>> - Add a quirk to enable E820 clipping for machines older than X,
>>> e.g., 2023, to avoid breaking machines that currently work.
>>>
>>> - Add quirks to disable E820 clipping for individual machines like
>>> the Lenovo and Clevos that predate X, but E820 clipping breaks
>>> them.
>>>
>>> - Add quirks to enable E820 clipping for individual machines like
>>> the Chromebooks (and probably machines we don't know about yet)
>>> that have devices that consume part of _CRS but are not
>>> enumerable.
>>>
>>> - Communicate this to OEMs to try to prevent future machines that
>>> need quirks.
>>>
>>> Bjorn
>>>
>>
>

2022-05-07 17:07:25

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] x86/PCI: Log E820 clipping

Hi,

On 5/2/22 22:32, Bjorn Helgaas wrote:
> On Mon, May 02, 2022 at 02:24:26PM +0200, Hans de Goede wrote:
>> On 4/19/22 18:45, Bjorn Helgaas wrote:
>>> On Tue, Apr 19, 2022 at 05:16:44PM +0200, Hans de Goede wrote:
>>>> On 4/19/22 17:03, Bjorn Helgaas wrote:
>>>>> On Tue, Apr 19, 2022 at 11:59:17AM +0200, Hans de Goede wrote:
>
>>>>>> So what is the plan to actually fix the issue seen on some
>>>>>> Lenovo models and Clevo Barebones ? As I mentioned previously
>>>>>> I think that since all our efforts have failed so far that we
>>>>>> should maybe reconsider just using DMI quirks to ignore the
>>>>>> E820 reservation windows for host bridges on affected models ?
>>>>>
>>>>> I have been resisting DMI quirks but I'm afraid there's no other
>>>>> way.
>>>>
>>>> Well there is the first match adjacent windows returned by _CRS
>>>> and only then do the "covers whole region" exception check. I
>>>> still think that would work at least for the chromebook
>>>> regression...
>>>
>>> Without a crystal clear strategy, I think we're going to be
>>> tweaking the algorithm forever as the _CRS/E820 mix changes.
>>> That's why I think that in the long term, a "use _CRS only, with
>>> quirks for exceptions" strategy will be simplest.
>>
>> Looking at the amount of exception we already now about I'm not sure
>> if that will work well.
>
> It's possible that many quirks will be required. But I think in the
> long run the value of the simplest, most obvious strategy is huge.
> It's laid out in the spec already and it's the clearest way to
> agreement between firmware and OS. When we trip over something, it's
> very easy to determine whether _CRS is wrong or Linux is using it
> wrong. If we have to bring in question of looking at E820 entries,
> possibly merging them, using them or not based on overlaps ... that's
> a much more difficult conversation without a clear resolution.
>
>>> So I think we should go ahead with DMI quirks instead of trying to
>>> make the algorithm smarter, and yes, I think we will need commandline
>>> arguments, probably one to force E820 clipping for future machines,
>>> and one to disable it for old machines.
>>
>> So what you are suggesting is to go back to a bios-date based approach
>> (to determine old vs new machines) combined with DMI quirks to force
>> E820 clipping on new machines which turn out to need it despite them
>> being new ?
>
> Yes. It's ugly but I think the 10-year outlook is better.

Ok, I've brushed off one of my earlier patches doing this and
added DMI quirks for the "Lenovo X1 Carbon 2nd gen" suspend
issue + the Asus C523NA / Google Coral Chromebook not booting
issues which we already know will get triggered by this based
on earlier testing.

I'll send this out right after this email.

Note this new patch is based on top of your:

https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=pci/resource

>> I have the feeling that if we switch to top-down allocating
>> that we can then switch to just using _CRS and that everything
>> will then just work, because we then match what Windows is doing...
>
> Yes, it might. But I'm not 100% comfortable because it basically
> sweeps _CRS bugs under the rug, and we may trip over them as we do
> more hotplug and (eventually) resource rebalancing. I think we need
> to work toward getting _CRS more reliable.

Ok.

Regards,

Hans