Re: [PATCH] x86/PCI: Disable E820 reserved region clipping for Clevo NL4XLU laptops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bjorn,

On 12/2/22 22:58, Bjorn Helgaas wrote:
> On Wed, Oct 12, 2022 at 10:23:12AM +0200, Hans de Goede wrote:
>> On 10/11/22 19:40, Bjorn Helgaas wrote:
>>> On Mon, Oct 10, 2022 at 05:02:06PM +0200, Hans de Goede wrote:
>>>> Clevo NL4XLU barebones have the same E820 reservation covering
>>>> the entire _CRS 32-bit window issue as the Lenovo *IIL* and
>>>> Clevo X170KM-G models, relevant dmesg bits (with pci=no_e820):
>>>> ...
>>>> Add a no_e820 quirk for these models to fix the touchpad not working
>>>> (due to Linux being unable to assign a PCI BAR for the i2c-controller).
>>>
>>> I do plan to apply this, but a little food for thought below.
>>>
>>> I explored this issue a little bit with the ACPI/UEFI folks (see
>>> https://members.uefi.org/wg/aswg/mail/thread/9265 if you have access).  
>>>
>>> One aspect I had glossed over earlier is that on most recent machines,
>>> the "E820 map" Linux uses is actually constructed internally by Linux
>>> based on the UEFI memory map, and that construction conflates several
>>> EFI types into E820_TYPE_RESERVED; see
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/efi/libstub/x86-stub.c?id=v5.19#n576
> 
> Critical error on my part here: the E820 map is passed to Linux by the
> bootloader or the EFI stub, NOT constructed by Linux.
> 
>>> From a response in the ACPI/UEFI discussion:
>>>
>>>   The reason EfiMemoryMappedIO[1] exist in the UEFI memory map is to
>>>   request a virtual mapping for UEFI Runtime Services.
>>>   ...
>>>   Thus the EfiMemoryMappedIO entries just exist to pass up the
>>>   EFI_MEMORY_RUNTIME attribute in the UEFI Memory Map. This is the part
>>>   of the contract for UEFI Runtime Service to use virtual mappings
>>>   provided by the OS. So from an OS point of view EfiMemoryMappedIO has
>>>   no other purpose.
>>>   
>>>   [1] UEFI: Table 7-5 Memory Type Usage before ExitBootServices() "Used
>>>   by system firmware to request that a memory-mapped IO region be
>>>   mapped by the OS to a virtual address so it can be accessed by EFI
>>>   runtime services."
> 
>>> I'm a little leery of changing that UEFI->E820 conversion because of
>>> other possible implications, but it may be that omitting
>>> EfiMemoryMappedIO entries from the E820 map and keeping the original
>>> "avoid E820 regions" (4dc2287c1805) would also solve this problem.
>>
>> Actually during my many attempts to fix this I did write a patch
>> adding a new E820_TYPE_MMIO to the generated e820-memmap which
>> would only show up in the EFI -> E820 entry generation case
>> and then used that to not exclude that E820 region, see
>> this RFC series:
>>
>> https://lore.kernel.org/linux-pci/20220214151759.98267-1-hdegoede@xxxxxxxxxx/
> 
> Yes :)  I tried something similar and of course it didn't work because
> it didn't change the E820 map coming from the bootloader.

Ah that is a good point, which I did not realize when I was working
on this back then.

Note it depends on the bootloader actually, systemd-boot uses
the in-kernel-EFI-stub and Fedora's grub is patched to also use
the in-kernel-EFI-stub.

But your solution should work regardless of the boot path (1), which
is good.

1) As long as some form of EFI booting is used.

> 
>> I also did another series which used the EfiMemoryMappedIO type as
>> an input to heuristics to automatically set pci=no_e820, see:
>>
>> https://lore.kernel.org/linux-pci/20220228105259.230903-1-hdegoede@xxxxxxxxxx/
>>
>> IIRC that patch eventually got replaced by a similar but simpler
>> heuristic from you. Which IIRC eventually got dropped again because
>> it was causing regressions on some models again.
>>
>> So we ended up with the current set pci=no_e820 using DMI based quirks +
>> try to enable it for all BIOS-es with date >= 2023 approach,
>> with the plan to do DMI quirks setting pci=use_e820 if any (buggy)
>> 2023 BIOS-es show up which need this.
> 
> I gave up on figuring out what went wrong in this path.
> 
> Anyway, I've had the patch below in -next for a couple weeks, but I
> plan to drop it and replace it with the series here:
> https://lore.kernel.org/linux-pci/20221202211838.1061278-1-helgaas@xxxxxxxxxx/

As I mentioned in the email-thread about that patch-series (and there
now is dmesg E820 output to confirm this) your generic fix will
unfortunately only work when people boot in EFI mode. It will still
be good to have the generic fix of course.

But maybe we should also add this quirk to make sure these
Clevo-s also work properly when booted in BIOS CSM mode ?

Regards,

Hans





> 
> Bjorn
> 
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216565
>>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>>> ---
>>>>  arch/x86/pci/acpi.c | 13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>>>> index 2f82480fd430..45ef65d31a40 100644
>>>> --- a/arch/x86/pci/acpi.c
>>>> +++ b/arch/x86/pci/acpi.c
>>>> @@ -189,6 +189,19 @@ static const struct dmi_system_id pci_crs_quirks[] __initconst = {
>>>>  			DMI_MATCH(DMI_BOARD_NAME, "X170KM-G"),
>>>>  		},
>>>>  	},
>>>> +
>>>> +	/*
>>>> +	 * Clevo NL4XLU barebones have the same E820 reservation covering
>>>> +	 * the entire _CRS 32-bit window issue as the Lenovo *IIL* models.
>>>> +	 * See https://bugzilla.kernel.org/show_bug.cgi?id=216565
>>>> +	 */
>>>> +	{
>>>> +		.callback = set_no_e820,
>>>> +		.ident = "Clevo NL4XLU Barebone",
>>>> +		.matches = {
>>>> +			DMI_MATCH(DMI_BOARD_NAME, "NL4XLU"),
>>>> +		},
>>>> +	},
>>>>  	{}
>>>>  };
>>>>  
>>>> -- 
>>>> 2.37.3
>>>>
>>>
>>
> 




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux