Hi Bjorn, many thanks for replying

-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org]
Sent: 24 February 2016 09:14
To: Gabriele Paoloni
Cc: 'Mark Rutland'; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
(C); Linuxarm; qiujiang; 'bhelgaas at google.com'; 'arnd at arndb.de';
'Lorenzo.Pieralisi at arm.com'; 'tn at semihalf.com'; 'linux-
pci at vger.kernel.org'; 'linux-kernel at vger.kernel.org'; xuwei (O);
'linux-acpi at vger.kernel.org'; 'jcm at redhat.com'; zhangjukuo; Liguozhu
(Kenneth); 'linux-arm-kernel at lists.infradead.org'
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:

quoted

quoted

-----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 at google.com; arnd at arndb.de;

Lorenzo.Pieralisi at arm.com;

quoted

quoted

tn at semihalf.com; linux-pci at vger.kernel.org; linux-
kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth); linux-arm-
kernel at lists.infradead.org
Subject: RE: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support

for

quoted

quoted

HiSilicon SoCs Host Controllers

quoted

-----Original Message-----
From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
owner at vger.kernel.org] On Behalf Of Mark Rutland
Sent: 10 February 2016 11:13
To: Gabriele Paoloni
Cc: Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong (C);

Linuxarm;

quoted

quoted

quoted

qiujiang; bhelgaas at google.com; arnd at arndb.de;
Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-

pci at vger.kernel.org;

quoted

quoted

quoted

linux-kernel at vger.kernel.org; xuwei (O); linux-

acpi at vger.kernel.org;

quoted

quoted

quoted

jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth); linux-arm-
kernel at lists.infradead.org
Subject: Re: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support

for

quoted

quoted

quoted

HiSilicon SoCs Host Controllers

On Wed, Feb 10, 2016 at 09:52:36AM +0000, Gabriele Paoloni wrote:

quoted

Hi Mark

quoted

On Tue, Feb 09, 2016 at 05:34:20PM +0000, Gabriele Paoloni

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

From: gabriele paoloni <redacted>
+/*
+ * 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,

quoted

quoted

quoted

0x10000

quoted

quoted

}},

quoted

+ *	}
+ *	})
+ */

As above, this does not look right. ACPI has standard

mechanisms

quoted

quoted

quoted

for

quoted

quoted

describing addresses. Making something up like this is not a

good

quoted

quoted

quoted

idea.

quoted

I am quite new to ACPI, may I ask you to explain a bit?

ACPI has standard mechanisms for describing certain resources,

and

quoted

quoted

quoted

these
should not be described in _DSD. Memory or IO address regions are

such

quoted

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

quoted

quoted

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  

quoted

Now the MCFG acpi framework always fix the MCFG resource size to

0x100000

quoted

for each bus; therefore if we pass our RC addresses through MCFG we

end

quoted

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.

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).
I thought the purpose of the quirks from Nowicki patchset were meant
to handle this sort of special cases...


Thanks and Regards

Gab

Bjorn