Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH] PCI: Remove End-End TLP as PASID dependency
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        jean-philippe <jean-philippe@linaro.org>,
        kenneth-lee-2012@foxmail.com, Wangzhou <wangzhou1@hisilicon.com>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        Sinan Kaya <okaya@kernel.org>
References: <20200611174140.GA1597601@bjorn-Precision-5520>
From:   Zhangfei Gao <zhangfei.gao@linaro.org>
Message-ID: <c981ed09-5fe2-b0ca-b023-ba7e8882832a@linaro.org>
Date:   Sat, 13 Jun 2020 21:49:19 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <20200611174140.GA1597601@bjorn-Precision-5520>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020/6/12 上午1:41, Bjorn Helgaas wrote:
> [+cc Sinan]
>
> On Wed, Jun 10, 2020 at 12:18:14PM +0800, Zhangfei Gao wrote:
>> Some platform devices appear as PCI and have PCI cfg space,
>> but are actually on the AMBA bus.
>> They can support PASID via smmu stall feature, but does not
>> support tlp since they are not real pci devices.
>> So remove tlp as a PASID dependency.
> When you iterate on this, pay attention to things like:
>
>    - Wrap paragraphs to 75 columns or so, so they fill the whole line
>      but don't overflow when "git show" adds 4 spaces.
>
>    - Leave a blank line between paragraphs.
>
>    - Capitalize consistently: "SMMU", "PCI", "TLP".
>
>    - Provide references to relevant spec sections, e.g., for the SMMU
>      stall feature.
OK, Thanks Bjorn
>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
>> ---
>>   drivers/pci/ats.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
>> index 390e92f..8e31278 100644
>> --- a/drivers/pci/ats.c
>> +++ b/drivers/pci/ats.c
>> @@ -344,9 +344,6 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
>>   	if (WARN_ON(pdev->pasid_enabled))
>>   		return -EBUSY;
>>   
>> -	if (!pdev->eetlp_prefix_path)
>> -		return -EINVAL;
> No.  This would mean we might enable PASID on actual PCIe devices when
> it is not safe to do so, as Jean-Philippe pointed out.
>
> You cannot break actual PCIe devices just to make your non-PCIe device
> work.
>
> These devices do not support PASID as defined in the PCIe spec.  They
> might support something *like* PASID, and you might be able to make
> parts of the PCI core work with them, but you're going to have to deal
> with the parts that don't follow the PCIe spec on your own.  That
> might be quirks, it might be some sort of AMBA adaptation shim, I
> don't know.  But it's not the responsibility of the PCI core to adapt
> to them.
Understand now.
Will continue use quirk for this.

Thanks