Re: [PATCH 1/1] x86/pci: Skip early E820 check for ECAM region
From: Bjorn Helgaas
Date: Wed Apr 17 2024 - 17:10:03 EST
On Wed, Apr 17, 2024 at 11:47:27PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 17, 2024 at 11:40 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> > Reported-by: Mateusz Kaduk <mateusz.kaduk@xxxxxxxxx>
>
> > Reported-by: Arul <...>
> > Reported-by: Imcarneiro91 <...>
> > Reported-by: Aman <...>
>
> Isn't bugzilla public enough? You may take emails from there, no?
Good question. I think email addresses in bugzilla are only visible
to logged-in users, so I wanted permission before publishing them. I
got that from Mateusz and am hoping to hear from the others because I
want everybody to get credit for their contribution.
> > + /*
> > + * 946f2ee5c731 ("Check that MCFG points to an e820
> > + * reserved area") added this E820 check in 2006 to work
> > + * around BIOS defects.
> > + *
> > + * Per PCI Firmware r3.3, sec 4.1.2, ECAM space must be
> > + * reserved by a PNP0C02 resource, but it need not be
> > + * mentioned in E820. Before the ACPI interpreter is
> > + * available, we can't check for PNP0C02 resources, so
> > + * there's no reliable way to verify the region in this
> > + * early check. Keep it only for the old machines that
> > + * motivated 946f2ee5c731.
> > + */
>
> > + if (dmi_get_bios_year() < 2016 && raw_pci_ops)
>
> I probably missed something, but where does 2016 come from?
> (I've been following the bz discussion)
I made it up based on the fact that 946f2ee5c731 was added in 2006,
and I just added 10 years. I would love to get rid of the E820 checks
altogether because they're really completely bogus and an ongoing
headache, but I don't know the details of the machines that
946f2ee5c731 fixed. I'm open to other suggestions. The Lenovo BIOS
is from 2023, so it would have to be something earlier than that:
DMI: LENOVO 83AG/LNVNB161216, BIOS MHCN40WW 12/15/2023
> > if (pci_mmcfg_running_state)
> > return true;
> >
> > - /* Don't try to do this check unless configuration
> > - type 1 is available. how about type 2 ?*/
> > - if (raw_pci_ops)
> > - return is_mmconf_reserved(e820__mapped_all, cfg, dev,
> > - "E820 entry");
> > -
> > return false;
>
> Not strictly related to this patch, but now it can simply
>
> return pci_mmcfg_running_state;
Good point, changed locally.
> In any case, LGTM,
> Reviewed-by: Andy Shevchenko <andy@xxxxxxxxxx>
Thanks!
Bjorn