Message-ID: <aa8bc916-9363-2de7-2997-4ce470562b2c@codeaurora.org>
Date:   Thu, 3 Nov 2016 12:58:10 -0400
From:   Sinan Kaya <okaya@...eaurora.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     cov@...eaurora.org, Tomasz Nowicki <tn@...ihalf.com>,
        will.deacon@....com, catalin.marinas@....com, rafael@...nel.org,
        Lorenzo.Pieralisi@....com, arnd@...db.de, hanjun.guo@...aro.org,
        jchandra@...adcom.com, dhdang@....com, ard.biesheuvel@...aro.org,
        robert.richter@...iumnetworks.com, mw@...ihalf.com,
        Liviu.Dudau@....com, ddaney@...iumnetworks.com,
        wangyijing@...wei.com, msalter@...hat.com,
        linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linaro-acpi@...ts.linaro.org, jcm@...hat.com,
        andrea.gallo@...aro.org, jeremy.linton@....com,
        liudongdong3@...wei.com, gabriele.paoloni@...wei.com,
        jhugo@...eaurora.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2] PCI: QDF2432 32 bit config space accessors


On 11/3/2016 10:00 AM, Bjorn Helgaas wrote:
> On Wed, Nov 02, 2016 at 12:36:16PM -0400, Sinan Kaya wrote:
>> Hi Bjorn,
>>
>> On 11/2/2016 12:08 PM, Bjorn Helgaas wrote:
>>> On Tue, Nov 01, 2016 at 07:06:31AM -0600, cov@...eaurora.org wrote:
>>>> Hi Bjorn,
>>>>
>>>> On 2016-10-31 15:48, Bjorn Helgaas wrote:
>>>>> On Wed, Sep 21, 2016 at 06:38:05PM -0400, Christopher Covington wrote:
>>>>>> The Qualcomm Technologies QDF2432 SoC does not support accesses
>>>>>> smaller
>>>>>> than 32 bits to the PCI configuration space. Register the appropriate
>>>>>> quirk.
>>>>>>
>>>>>> Signed-off-by: Christopher Covington <cov@...eaurora.org>
>>>>>
>>>>> Hi Christopher,
>>>>>
>>>>> Can you rebase this against v4.9-rc1?  It no longer applies to my tree.
>>>>
>>>> I apologize for not being clearer. This patch depends on:
>>>>
>>>> PCI/ACPI: Extend pci_mcfg_lookup() responsibilities
>>>> PCI/ACPI: Check platform-specific ECAM quirks
>>>>
>>>> These patches from Tomasz Nowicki were previously in your pci/ecam-v6
>>>> branch, but that seems to have come and gone. How would you like to
>>>> proceed?
>>>
>>> Oh yes, that's right, I forgot that connection.  I'm afraid I kind of
>>> dropped the ball on that thread, so I went back and read through it
>>> again.
>>>
>>> I *think* the current state is:
>>>
>>>   - I'm OK with the first two patches that add the quirk
>>>     infrastructure.
>>>
>>>   - My issue with the last three patches that add ThunderX quirks is
>>>     that there's no generic description of the ECAM address space.
>>>
>>> So if I understand correctly, your Qualcomm patch depends only on the
>>> first two patches.
>>>
>>> Then the question is how the Qualcomm ECAM address space is described.
>>> Your quirk overrides the default pci_generic_ecam_ops with the
>>> &pci_32b_ops, but it doesn't touch the address space part, so I assume
>>> the bus ranges and corresponding address space in your MCFG is
>>> correct.  So far, so good.
>>
>> Qualcomm ECAM space includes both the root port and the endpoint address
>> space with a single contiguous 256 MB address space described in MCFG table.
>> There is no need to describe additional resources like PNP0C02.
> 
> This is the crucial point I have failed to communicate clearly: the
> PNP0C02 resource is *always* required, even if the MCFG is correct.
> 

Interesting...

It looks like there is a lot of lessons learnt here from history.

I think this requirement is only true if your system DDR space and PCIe
space overlaps in the memory map. I understand that Intel systems allow
sharing of these two memory ranges. An OS could potentially reclaim this
address range.

If there is no overlap and PCI is not enabled, there can't be any SW entity
to reclaim this space. 

Did I miss something?

> The reason is that MCFG is a PCI-specific table, and it should be
> possible to boot a kernel with no PCI support.  That kernel will not
> look at the MCFG.  The PCI hardware will still be present and will
> still consume the ECAM space, so the OS must be able to discover that
> the ECAM space is not available for other devices.
> 
> The usual way to for the OS to discover that would be via the _CRS of
> a PNP0A03 or PNP0A08 host bridge device.  _CRS is what I mean by a
> "generic" way to describe this address space, because the ACPI core
> can interpret _CRS for all ACPI devices, even if the kernel doesn't
> contain drivers for all of those devices.
> 
> It turns out that we can't use the _CRS of host bridges because of the
> Producer/Consumer bit screwup [1].  So the fallback is to include the
> ECAM space in the _CRS of a PNP0C02 device.  This is what the PCI
> Firmware spec r3.0, Table 4-2, footnote 2 is talking about.
> 
> Bjorn
> 
> [1] The original ACPI spec intent was that Consumer resources would be
> space like ECAM that is consumed directly by the bridge, and Producer
> resources would be the windows forwarded down to PCI.  But BIOSes
> didn't use the Producer/Consumer bit consistently, so we have to
> assume that all resources in host bridge _CRS are windows, which
> leaves us no way to describe the Consumer resources.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Powered by blists - more mailing lists