Re: Info: mapping multiple BARs. Your kernel is fine.

Rafael J. Wysocki
2014-03-20T20:55:06Z

On Thursday, March 20, 2014 10:45:52 AM Bjorn Helgaas wrote:
> On Wed, Mar 19, 2014 at 9:03 PM, Zhang, Rui <rui.zhang@intel.com> wrote:
>
> > I've talked with Yan Zheng, and I was told that this resource "0xfed10000 - 0xfed15fff"
> > is got from PCI device register directly, which is not in its BAR range.
> > Thus IMO, it is impossible for PNP layer to be aware of this resource.
>
> Slow down, this isn't quite correct. The *base* address (0xfed10000)
> is from a PCI config register (MCHBAR, at PCI config offset 0x48) [1].
> This is a device-dependent register, so the PCI core knows neither
> the base nor the size.
>
> The device consumes address space that is not reported via the
> architected PCI mechanism, so the only way to report that space is via
> the PNP0C02 ACPI device. The BIOS has to determine the base and size
> based on its knowledge of the hardware. On this hardware, per the
> spec in [1], the region described by MCHBAR is 32KB in size.
>
> The 0x6000 (24KB) size of the region above comes from the driver and
> is actually less than what the device consumes. It is legal for a
> driver to request only the area it requires, but the entire area
> consumed by the device should be reported via the PNP0C02 device. The
> fact that PNP0C02 reports 16KB but the device actually consumes 32KB
> is a BIOS defect. This probably happened because previous versions of
> this chip consumed only 16KB, and the BIOS didn't get updated for the
> change.
>
> > BTW, about drivers/pnp/system.c, if its ONLY purpose is to prevent those
> > resources from being allocated to uninitialized PCI devices, then IMO,
> > the best way to do this is make PCI bus handle those PNP0C01/PNP0C02
> > resources directly, probably via a platform callback, say,
> > 1. make drivers/pnp/system.c a no-op for PNPACPI, by checking pnp_dev->protocol.
> > 2. introduce acpi_check_reserved_resource() to parsing PNP0C01/PNP0C02 resources.
> > 3. in PCI bus, invoke acpi_check_reserved_resource() when assigning
> > resources to PCI devices.
>
> The purpose of system.c is indeed to prevent resources from being
> allocated to other devices. This is really a question for Rafael, but
> in my opinion this function (reserving resources of PNP/ACPI devices
> to prevent their allocation to other devices) should be done for *all*
> PNP and ACPI devices, not just the PNP0C01/PNP0C02 devices handled by
> system.c.
>
> So I think the best solution would be to move that into the ACPI core
> somehow so it happens for all devices.

Well, I think you got to the bottom of this, but that's something we can
do long-term. Still, we need to find a short-term solution for the
particular issue at hand.

> If we had that, we could get
> rid of system.c altogether, and we wouldn't have to do anything
> special in PCI. This is much easier to say than to do, however,
> because there are all kinds of issues with legacy resource
> reservations, and we currently can't really deal with overlapping
> resources.

Indeed.

All above said, appended is the relevant piece of the DSDT from the machine
in question (and that is in the PCI host bridge scope).

So we have a PCI device with an ACPI object called LPC which has a child
called SIO and the _HID of that child is "PNP0C02".

I'm not sure if the way system.c handles this is correct in this particular
case to be honest.

Device (LPC)
{
Name (_ADR, 0x001F0000)
Name (_S3D, 0x03)
Name (RID, 0x00)
Device (SIO)
{
Name (_HID, EisaId ("PNP0C02"))
Name (_UID, 0x00)
Name (SCRS, ResourceTemplate ()
{
IO (Decode16,
0x0010, // Range Minimum
0x0010, // Range Maximum
0x01, // Alignment
0x10, // Length
)
IO (Decode16,
0x0090, // Range Minimum
0x0090, // Range Maximum
0x01, // Alignment
0x10, // Length
)
IO (Decode16,
0x0024, // Range Minimum
0x0024, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x0028, // Range Minimum
0x0028, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x002C, // Range Minimum
0x002C, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x0030, // Range Minimum
0x0030, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x0034, // Range Minimum
0x0034, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x0038, // Range Minimum
0x0038, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x003C, // Range Minimum
0x003C, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x00A4, // Range Minimum
0x00A4, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x00A8, // Range Minimum
0x00A8, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x00AC, // Range Minimum
0x00AC, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x00B0, // Range Minimum
0x00B0, // Range Maximum
0x01, // Alignment
0x06, // Length
)
IO (Decode16,
0x00B8, // Range Minimum
0x00B8, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x00BC, // Range Minimum
0x00BC, // Range Maximum
0x01, // Alignment
0x02, // Length
)
IO (Decode16,
0x0050, // Range Minimum
0x0050, // Range Maximum
0x01, // Alignment
0x04, // Length
)
IO (Decode16,
0x0072, // Range Minimum
0x0072, // Range Maximum
0x01, // Alignment
0x06, // Length
)
IO (Decode16,
0x0400, // Range Minimum
0x0400, // Range Maximum
0x01, // Alignment
0x80, // Length
)
IO (Decode16,
0x0500, // Range Minimum
0x0500, // Range Maximum
0x01, // Alignment
0x80, // Length
)
IO (Decode16,
0x0800, // Range Minimum
0x0800, // Range Maximum
0x01, // Alignment
0x10, // Length
)
IO (Decode16,
0x15E0, // Range Minimum
0x15E0, // Range Maximum
0x01, // Alignment
0x10, // Length
)
IO (Decode16,
0x1600, // Range Minimum
0x1600, // Range Maximum
0x01, // Alignment
0x80, // Length
)
Memory32Fixed (ReadWrite,
0xF8000000, // Address Base
0x04000000, // Address Length
)
Memory32Fixed (ReadWrite,
0x00000000, // Address Base
0x00001000, // Address Length
_Y26)
Memory32Fixed (ReadWrite,
0xFED1C000, // Address Base
0x00004000, // Address Length
)
Memory32Fixed (ReadWrite,
0xFED10000, // Address Base
0x00004000, // Address Length
)
Memory32Fixed (ReadWrite,
0xFED18000, // Address Base
0x00001000, // Address Length
)
Memory32Fixed (ReadWrite,
0xFED19000, // Address Base
0x00001000, // Address Length
)
Memory32Fixed (ReadWrite,
0xFED45000, // Address Base
0x00007000, // Address Length
)
})
CreateDWordField (SCRS, \_SB.PCI0.LPC.SIO._Y26._BAS, TRMB)
Method (_CRS, 0, NotSerialized)
{
Store (\TBAB, TRMB)
If (LEqual (\_SB.PCI0.LPC.TPM._STA (), 0x0F))
{
Return (SCRS)
}
Else
{
Subtract (SizeOf (SCRS), 0x02, Local0)
Name (BUF0, Buffer (Local0) {})
Add (Local0, SizeOf (\_SB.PCI0.LPC.TPM.BUF1), Local0)
Name (BUF1, Buffer (Local0) {})
Store (SCRS, BUF0)
Concatenate (BUF0, \_SB.PCI0.LPC.TPM.BUF1, BUF1)
Return (BUF1)
}
}
}