Re: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Bjorn Helgaas]

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).

> 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.

Bjorn