On Wed, Feb 24, 2016 at 06:46:09AM +0000, Gabriele Paoloni wrote: > > Hi Bjorn, many thanks for replying > > > -----Original Message----- > > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] > > Sent: 24 February 2016 09:14 > > To: Gabriele Paoloni > > Cc: 'Mark Rutland'; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong > > (C); Linuxarm; qiujiang; 'bhelgaas@xxxxxxxxxx'; 'arnd@xxxxxxxx'; > > 'Lorenzo.Pieralisi@xxxxxxx'; 'tn@xxxxxxxxxxxx'; 'linux- > > pci@xxxxxxxxxxxxxxx'; 'linux-kernel@xxxxxxxxxxxxxxx'; xuwei (O); > > 'linux-acpi@xxxxxxxxxxxxxxx'; 'jcm@xxxxxxxxxx'; zhangjukuo; Liguozhu > > (Kenneth); 'linux-arm-kernel@xxxxxxxxxxxxxxxxxxx' > > Subject: Re: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support for > > HiSilicon SoCs Host Controllers > > > > On Tue, Feb 23, 2016 at 02:47:22AM +0000, Gabriele Paoloni wrote: > > > > -----Original Message----- > > > > From: Gabriele Paoloni > > > > Sent: 10 February 2016 22:45 > > > > To: Mark Rutland > > > > Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); Linuxarm; > > > > qiujiang; bhelgaas@xxxxxxxxxx; arnd@xxxxxxxx; > > Lorenzo.Pieralisi@xxxxxxx; > > > > tn@xxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux- > > > > kernel@xxxxxxxxxxxxxxx; xuwei (O); linux-acpi@xxxxxxxxxxxxxxx; > > > > jcm@xxxxxxxxxx; zhangjukuo; Liguozhu (Kenneth); linux-arm- > > > > kernel@xxxxxxxxxxxxxxxxxxx > > > > Subject: RE: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support > > for > > > > HiSilicon SoCs Host Controllers > > > > > > > > > -----Original Message----- > > > > > From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel- > > > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Mark Rutland > > > > > Sent: 10 February 2016 11:13 > > > > > To: Gabriele Paoloni > > > > > Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C); > > Linuxarm; > > > > > qiujiang; bhelgaas@xxxxxxxxxx; arnd@xxxxxxxx; > > > > > Lorenzo.Pieralisi@xxxxxxx; tn@xxxxxxxxxxxx; linux- > > pci@xxxxxxxxxxxxxxx; > > > > > linux-kernel@xxxxxxxxxxxxxxx; xuwei (O); linux- > > acpi@xxxxxxxxxxxxxxx; > > > > > jcm@xxxxxxxxxx; zhangjukuo; Liguozhu (Kenneth); linux-arm- > > > > > kernel@xxxxxxxxxxxxxxxxxxx > > > > > Subject: Re: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support > > for > > > > > HiSilicon SoCs Host Controllers > > > > > > > > > > On Wed, Feb 10, 2016 at 09:52:36AM +0000, Gabriele Paoloni wrote: > > > > > > Hi Mark > > > > > > > > > > > > > On Tue, Feb 09, 2016 at 05:34:20PM +0000, Gabriele Paoloni > > wrote: > > > > > > > > From: gabriele paoloni <gabriele.paoloni@xxxxxxxxxx> > > > > > > > > +/* > > > > > > > > + * Retrieve rc_dbi base and size from _DSD > > > > > > > > + * Name (_DSD, Package () { > > > > > > > > + * ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), > > > > > > > > + * Package () { > > > > > > > > + * Package () {"rc-dbi", Package () { 0x0, 0xb0080000, > > 0x0, > > > > > 0x10000 > > > > > > > }}, > > > > > > > > + * } > > > > > > > > + * }) > > > > > > > > + */ > > > > > > > > > > > > > > As above, this does not look right. ACPI has standard > > mechanisms > > > > > for > > > > > > > describing addresses. Making something up like this is not a > > good > > > > > idea. > > > > > > > > > > > > I am quite new to ACPI, may I ask you to explain a bit? > > > > > > > > > > ACPI has standard mechanisms for describing certain resources, > > and > > > > > these > > > > > should not be described in _DSD. Memory or IO address regions are > > > > such > > > > > resources (in _CRS, IIRC), and should not be described in _DSD. > > > > > > > > Hi Mark, > > > > > > > > In my case I think in need to look into the MCFG object as the > > problem > > > > I have is RC using a different range than the rest of the hierarchy. > > > > > > > > I'll investigate this and try to come with a solution in v4 > > > > > > I have looked into this and in our case we cannot use the > > > standard MCFG object to pass the RC config space addresses. > > > > > > The reason is that in our HW we have the config base addresses of the > > > root complex ports that are less than 0x100000 byte distant one from > > > the other as we only map the first 0x10000 bytes. > > > > The ECAM spec requires 4096 bytes per function, 8 functions per > > device, 32 devices per bus, which means you need 0x100000 bytes of > > address space per bus. If your device doesn't supply that, it doesn't > > really implement ECAM, and you probably can't use the standard ways of > > describing ECAM (MCFG, _CBA). > > Correct, our host bridge is non ECAM only for the RC bus config space; > for any other bus underneath the root bus we support ECAM access, so > we need a quirk for the RC config rd/wr MCFG can specify a bus number range, so you might be able to use it to describe the ECAM space for buses below the root bus, e.g., [bus 01-ff]. > > > Now the MCFG acpi framework always fix the MCFG resource size to > > 0x100000 > > > for each bus; therefore if we pass our RC addresses through MCFG we > > end > > > up with a resource conflict. > > > > > > To give you a practical example we are in a situation where we have: > > > > > > port0: [0x00000000b0080000 - 0x00000000b0080000 + 0x10000] > > > port1: [0x00000000b0090000 - 0x00000000b0090000 + 0x10000] > > > port2: [0x00000000b00A0000 - 0x00000000b00A0000 + 0x10000] > > > port3: [0x00000000b00B0000 - 0x00000000b00B0000 + 0x10000] > > > > > > So if we pass the base addresses through MCFG the resources > > > will overlap as MCFG will consider 0x100000 size for each base > > > address of the root complex (only the RC bus uses that address) > > > > > > So far I do not see many option other than using _DSD to pass > > > these RC config base addresses. > > > > I don't want to take over Mark's discussion, but I really do not think > > _DSD is the correct way to fix this. _CRS is like a generalized PCI > > BAR. A PCI device is only allowed to respond to address space > > reported via a BAR. An ACPI device is only allowed to respond to > > address space reported via _CRS. Those are important rules because > > they mean we can manage address space with generic code instead of > > device-specific code. > > From what I see in > http://lxr.free-electrons.com/source/drivers/acpi/pci_root.c#L723 > acpi_pci_probe_root_resources() parses the _CRS method and > it only works for MEM and IO resources, so I don't think it is > right to pass a config space address by _CRS or to modify the > current ACPI framework to support this. Config space addresses are made up of [bus, device, function, register offset], and you're right that _CRS doesn't contain those addresses directly; _CRS only describes memory, I/O, and bus number ranges. But part of the function of a host bridge is to convert memory or I/O accesses on the primary (CPU) side to PCI config accesses on the secondary (PCI) side, and this CPU-side memory or I/O region should be reported via _CRS. I think the relevant spec is the PCI Firmware Spec, r3.0, sec 4.1.2. Note 2 in that section says the address range of an MMCFG region must be reserved by declaring a motherboard resource, i.e., included in the _CRS of a PNP0C02 or similar device. > On the other side, since this is an exception only for the config > space address of our host controller (as said before all the buses > below the root one support ECAM), I think that it is right to put > this address as a device specific data (in fact the rest of the > config space addresses will be parsed from MCFG). A kernel with no support for your host controller (and thus no knowledge of its _DSD) should still be able to operate the rest of the system correctly. That means we must have a generic way to learn what address space is consumed by the host controller so we don't try to assign it to other devices. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html