Re: [PATCH v2 5/5] PCI: qcom: Add support for configuring BDF to SID mapping for SM8250

[Stanimir Varbanov]

Hi Mani,

On 10/1/20 8:57 AM, Manivannan Sadhasivam wrote:
> Hi Stan,
> 
> On Thu, Oct 01, 2020 at 12:46:46AM +0300, Stanimir Varbanov wrote:
>> Hi Mani,
>>
>> On 9/30/20 6:09 PM, Manivannan Sadhasivam wrote:
>>> For SM8250, we need to write the BDF to SID mapping in PCIe controller
>>> register space for proper working. This is accomplished by extracting
>>> the BDF and SID values from "iommu-map" property in DT and writing those
>>> in the register address calculated from the hash value of BDF. In case
>>> of collisions, the index of the next entry will also be written.
>>
>> This describes what the patch is doing. But why? Is that done in the
>> other DWC low-level drivers or this is qcom specialty?
>>
> 
> AFAIK, only some NXP SoCs deal with similar kind of mapping but right now
> this is a Qcom only stuff.
> 
>>>
>>> For the sake of it, let's introduce a "config_sid" callback and do it
>>> conditionally for SM8250.
>>>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
>>> ---
>>>  drivers/pci/controller/dwc/Kconfig     |   1 +
>>>  drivers/pci/controller/dwc/pcie-qcom.c | 138 +++++++++++++++++++++++++
>>>  2 files changed, 139 insertions(+)

<snip>

>>>  
>>> +static int qcom_pcie_get_iommu_map(struct qcom_pcie *pcie)
>>> +{
>>> +	/* iommu map structure */
>>> +	struct {
>>> +		u32 bdf;
>>> +		u32 phandle;
>>> +		u32 smmu_sid;
>>> +		u32 smmu_sid_len;
>>> +	} *map;
>>> +	struct device *dev = pcie->pci->dev;
>>> +	int i, size = 0;
>>> +	u32 smmu_sid_base;
>>> +
>>> +	of_get_property(dev->of_node, "iommu-map", &size);
>>> +	if (!size)
>>> +		return 0;
>>> +
>>> +	map = kzalloc(size, GFP_KERNEL);
>>> +	if (!map)
>>> +		return -ENOMEM;
>>> +
>>> +	of_property_read_u32_array(dev->of_node,
>>> +		"iommu-map", (u32 *)map, size / sizeof(u32));
>>
>> iommu-map is a standard DT property why we have to parse it manually?
>>
> 
> So right now we don't have a way to pass this information from DT. And there
> is no IOMMU API to parse the fields also. We need to extract this information
> to program the hash tables (BDF, SID) as the mapping between BDF and SID is not
> 1:1 in SM8250.

We used iommu-map for msm8998 see this commit:

b84dfd175c09888751f501e471fdca346f582e06
("arm64: dts: qcom: msm8998: Add PCIe PHY and RC nodes")

I also Cc-ed Marc if he knows something more.

> 
> Perhaps I can add this information in commit message.


-- 
regards,
Stan