[v5,01/18] PCI: dwc: Use the DMA-API to get the MSI address

Message ID 20171120133222.27771-2-niklas.cassel@axis.com
State Superseded
Headers show
Series
  • dwc MSI fixes, ARTPEC-6 EP mode support, ARTPEC-7 SoC support
Related show

Commit Message

Niklas Cassel Nov. 20, 2017, 1:32 p.m.
Use the DMA-API to get the MSI address. This address will be written to
our PCI config space and to the register which determines which AXI
address the DWC IP will spoof for incoming MSI irqs.

Since it is a PCIe endpoint device, rather than the CPU, that is supposed
to write to the MSI address, the proper way to get the MSI address is by
using the DMA API, not by using virt_to_phys().

Using virt_to_phys() might work on some systems, but using the DMA API
should work on all systems.

This is essentially the same thing as allocating a buffer in a driver
to which the endpoint will write to. To do this, we use the DMA API.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
 drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++---
 drivers/pci/dwc/pcie-designware.h      |  3 ++-
 2 files changed, 14 insertions(+), 4 deletions(-)

Comments

Lorenzo Pieralisi Nov. 30, 2017, 3:28 p.m. | #1
Jingoo, Joao,

I am expecting your testing on the series and ACKs on the dwc related
patches please, according to v4 review - I will mark them as needs
review/ACK waiting for you to chime in.

v4 thread:

https://patchwork.ozlabs.org/patch/833882/

Thanks,
Lorenzo

On Mon, Nov 20, 2017 at 02:32:04PM +0100, Niklas Cassel wrote:
> Use the DMA-API to get the MSI address. This address will be written to
> our PCI config space and to the register which determines which AXI
> address the DWC IP will spoof for incoming MSI irqs.
> 
> Since it is a PCIe endpoint device, rather than the CPU, that is supposed
> to write to the MSI address, the proper way to get the MSI address is by
> using the DMA API, not by using virt_to_phys().
> 
> Using virt_to_phys() might work on some systems, but using the DMA API
> should work on all systems.
> 
> This is essentially the same thing as allocating a buffer in a driver
> to which the endpoint will write to. To do this, we use the DMA API.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
>  drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++---
>  drivers/pci/dwc/pcie-designware.h      |  3 ++-
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 81e2157a7cfb..33b52fe98a01 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  
>  void dw_pcie_msi_init(struct pcie_port *pp)
>  {
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +	struct page *page;
>  	u64 msi_target;
>  
> -	pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
> -	msi_target = virt_to_phys((void *)pp->msi_data);
> +	page = alloc_page(GFP_KERNEL | GFP_DMA32);
> +	pp->msi_data = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dev, pp->msi_data)) {
> +		dev_err(dev, "failed to map MSI data\n");
> +		__free_page(page);
> +		return;
> +	}
> +	msi_target = (u64)pp->msi_data;
>  
>  	/* program the msi_data */
>  	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
> @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
>  	if (pp->ops->get_msi_addr)
>  		msi_target = pp->ops->get_msi_addr(pp);
>  	else
> -		msi_target = virt_to_phys((void *)pp->msi_data);
> +		msi_target = (u64)pp->msi_data;
>  
>  	msg.address_lo = (u32)(msi_target & 0xffffffff);
>  	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index e5d9d77b778e..ecdede68522a 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -14,6 +14,7 @@
>  #ifndef _PCIE_DESIGNWARE_H
>  #define _PCIE_DESIGNWARE_H
>  
> +#include <linux/dma-mapping.h>
>  #include <linux/irq.h>
>  #include <linux/msi.h>
>  #include <linux/pci.h>
> @@ -168,7 +169,7 @@ struct pcie_port {
>  	const struct dw_pcie_host_ops *ops;
>  	int			msi_irq;
>  	struct irq_domain	*irq_domain;
> -	unsigned long		msi_data;
> +	dma_addr_t		msi_data;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
>  
> -- 
> 2.14.2
>
Niklas Cassel Dec. 13, 2017, 1:59 p.m. | #2
On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
> Jingoo, Joao,
> 
> I am expecting your testing on the series and ACKs on the dwc related
> patches please, according to v4 review - I will mark them as needs
> review/ACK waiting for you to chime in.
> 
> v4 thread:
> 
> https://patchwork.ozlabs.org/patch/833882/
> 

Hello Lorenzo, Jingoo, Joao,

Tomorrow another 2 weeks has passed.
V1 of this patch series was posted on 2017-10-13.

I'm a bit worried that this patch series will not make it to linux-next
in time for this patch series to be included in the 4.16 pull request.

Here is the V5 patch series:
https://patchwork.ozlabs.org/project/linux-pci/list/?series=14364

Perhaps any maintainer of a designware based PCIe driver could
help out and test this patch series?

Regards,
Niklas
Lorenzo Pieralisi Dec. 13, 2017, 2:31 p.m. | #3
Jingoo, Joao,

Niklas has a point. I can't apply this series without your testing
and review, so please do test/ack if you deem that suitable.

On Wed, Dec 13, 2017 at 02:59:54PM +0100, Niklas Cassel wrote:
> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
> > Jingoo, Joao,
> > 
> > I am expecting your testing on the series and ACKs on the dwc related
> > patches please, according to v4 review - I will mark them as needs
> > review/ACK waiting for you to chime in.
> > 
> > v4 thread:
> > 
> > https://patchwork.ozlabs.org/patch/833882/
> > 
> 
> Hello Lorenzo, Jingoo, Joao,
> 
> Tomorrow another 2 weeks has passed.
> V1 of this patch series was posted on 2017-10-13.

I understand - I need their review/testing to move forward with this
series and I strongly encourage them to chime in - as soon as possible.

> I'm a bit worried that this patch series will not make it to linux-next
> in time for this patch series to be included in the 4.16 pull request.

We should be able to do it if they reply within a reasonable timeframe.

Thanks,
Lorenzo

> Here is the V5 patch series:
> https://patchwork.ozlabs.org/project/linux-pci/list/?series=14364
> 
> Perhaps any maintainer of a designware based PCIe driver could
> help out and test this patch series?
> 
> Regards,
> Niklas
Joao Pinto Dec. 13, 2017, 5:21 p.m. | #4
Hi Niklas,

Às 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
>> Jingoo, Joao,
>>
>> I am expecting your testing on the series and ACKs on the dwc related
>> patches please, according to v4 review - I will mark them as needs
>> review/ACK waiting for you to chime in.
>>
>> v4 thread:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
>>
> 
> Hello Lorenzo, Jingoo, Joao,
> 
> Tomorrow another 2 weeks has passed.
> V1 of this patch series was posted on 2017-10-13.

Sorry, I have been tight up with a debug session and not able to check this out.
Adding Gustavo in CC that is now also working in PCI software development.
I am going to check the code ASAP and we will test it as soon as the debug is
finished.

Thanks.

> 
> I'm a bit worried that this patch series will not make it to linux-next
> in time for this patch series to be included in the 4.16 pull request.
> 
> Here is the V5 patch series:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
> 
> Perhaps any maintainer of a designware based PCIe driver could
> help out and test this patch series?
> 
> Regards,
> Niklas
>
Gustavo Pimentel Dec. 14, 2017, 12:16 p.m. | #5
Hi Niklas and Lorenzo,

I'm going to work on PCI software development now as told by João and I will
test your code now.

I was retrieving the patches through the patchwork
https://patchwork.ozlabs.org/project/linux-pci/list/?submitter=65580 and I
notice that its missing the patch 13 and 17, is this right?

If not, can you give me both patches location?

Thanks.


On 13/12/2017 17:21, Joao Pinto wrote:
> Hi Niklas,
>
> Às 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
>> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
>>> Jingoo, Joao,
>>>
>>> I am expecting your testing on the series and ACKs on the dwc related
>>> patches please, according to v4 review - I will mark them as needs
>>> review/ACK waiting for you to chime in.
>>>
>>> v4 thread:
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
>>>
>> Hello Lorenzo, Jingoo, Joao,
>>
>> Tomorrow another 2 weeks has passed.
>> V1 of this patch series was posted on 2017-10-13.
> Sorry, I have been tight up with a debug session and not able to check this out.
> Adding Gustavo in CC that is now also working in PCI software development.
> I am going to check the code ASAP and we will test it as soon as the debug is
> finished.
>
> Thanks.
>
>> I'm a bit worried that this patch series will not make it to linux-next
>> in time for this patch series to be included in the 4.16 pull request.
>>
>> Here is the V5 patch series:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
>>
>> Perhaps any maintainer of a designware based PCIe driver could
>> help out and test this patch series?
>>
>> Regards,
>> Niklas
>>
Lorenzo Pieralisi Dec. 14, 2017, 12:22 p.m. | #6
On Thu, Dec 14, 2017 at 12:16:38PM +0000, Gustavo Pimentel wrote:
> Hi Niklas and Lorenzo,
> 
> I'm going to work on PCI software development now as told by Joao and I will
> test your code now.
> 
> I was retrieving the patches through the patchwork
> https://patchwork.ozlabs.org/project/linux-pci/list/?submitter=65580 and I
> notice that its missing the patch 13 and 17, is this right?

No, you just have to refine the filter, I marked the DT bindings as
ready to be applied that's why they do not show up (and by the way
you can test the series even without those two patches - that are just
DT bindings).

Thanks,
Lorenzo

> If not, can you give me both patches location?
> 
> Thanks.
> 
> 
> On 13/12/2017 17:21, Joao Pinto wrote:
> > Hi Niklas,
> >
> > ??s 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
> >> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
> >>> Jingoo, Joao,
> >>>
> >>> I am expecting your testing on the series and ACKs on the dwc related
> >>> patches please, according to v4 review - I will mark them as needs
> >>> review/ACK waiting for you to chime in.
> >>>
> >>> v4 thread:
> >>>
> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
> >>>
> >> Hello Lorenzo, Jingoo, Joao,
> >>
> >> Tomorrow another 2 weeks has passed.
> >> V1 of this patch series was posted on 2017-10-13.
> > Sorry, I have been tight up with a debug session and not able to check this out.
> > Adding Gustavo in CC that is now also working in PCI software development.
> > I am going to check the code ASAP and we will test it as soon as the debug is
> > finished.
> >
> > Thanks.
> >
> >> I'm a bit worried that this patch series will not make it to linux-next
> >> in time for this patch series to be included in the 4.16 pull request.
> >>
> >> Here is the V5 patch series:
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
> >>
> >> Perhaps any maintainer of a designware based PCIe driver could
> >> help out and test this patch series?
> >>
> >> Regards,
> >> Niklas
> >>
> 
>
Gustavo Pimentel Dec. 14, 2017, 12:38 p.m. | #7
Ok. I will compile and test it now.

Thanks.


On 14/12/2017 12:22, Lorenzo Pieralisi wrote:
> On Thu, Dec 14, 2017 at 12:16:38PM +0000, Gustavo Pimentel wrote:
>> Hi Niklas and Lorenzo,
>>
>> I'm going to work on PCI software development now as told by Joao and I will
>> test your code now.
>>
>> I was retrieving the patches through the patchwork
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fsubmitter-3D65580&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=paO6WYeoTil-HWIJVBhwaWcIZP7r95MZNRzKMN6kprU&s=yjtNhJKPJdsHD_A52xWSD2g_YNHuLhO1YADKsPaPsio&e= and I
>> notice that its missing the patch 13 and 17, is this right?
> No, you just have to refine the filter, I marked the DT bindings as
> ready to be applied that's why they do not show up (and by the way
> you can test the series even without those two patches - that are just
> DT bindings).
>
> Thanks,
> Lorenzo
>
>> If not, can you give me both patches location?
>>
>> Thanks.
>>
>>
>> On 13/12/2017 17:21, Joao Pinto wrote:
>>> Hi Niklas,
>>>
>>> ??s 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
>>>> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
>>>>> Jingoo, Joao,
>>>>>
>>>>> I am expecting your testing on the series and ACKs on the dwc related
>>>>> patches please, according to v4 review - I will mark them as needs
>>>>> review/ACK waiting for you to chime in.
>>>>>
>>>>> v4 thread:
>>>>>
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
>>>>>
>>>> Hello Lorenzo, Jingoo, Joao,
>>>>
>>>> Tomorrow another 2 weeks has passed.
>>>> V1 of this patch series was posted on 2017-10-13.
>>> Sorry, I have been tight up with a debug session and not able to check this out.
>>> Adding Gustavo in CC that is now also working in PCI software development.
>>> I am going to check the code ASAP and we will test it as soon as the debug is
>>> finished.
>>>
>>> Thanks.
>>>
>>>> I'm a bit worried that this patch series will not make it to linux-next
>>>> in time for this patch series to be included in the 4.16 pull request.
>>>>
>>>> Here is the V5 patch series:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
>>>>
>>>> Perhaps any maintainer of a designware based PCIe driver could
>>>> help out and test this patch series?
>>>>
>>>> Regards,
>>>> Niklas
>>>>
>>
Lorenzo Pieralisi Dec. 18, 2017, 3:57 p.m. | #8
Hi Gustavo,

On Thu, Dec 14, 2017 at 12:38:04PM +0000, Gustavo Pimentel wrote:
> Ok. I will compile and test it now.

Have you managed to retrieve the patches and test them ?

Thank you,
Lorenzo

> Thanks.
> 
> 
> On 14/12/2017 12:22, Lorenzo Pieralisi wrote:
> > On Thu, Dec 14, 2017 at 12:16:38PM +0000, Gustavo Pimentel wrote:
> >> Hi Niklas and Lorenzo,
> >>
> >> I'm going to work on PCI software development now as told by Joao and I will
> >> test your code now.
> >>
> >> I was retrieving the patches through the patchwork
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fsubmitter-3D65580&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=paO6WYeoTil-HWIJVBhwaWcIZP7r95MZNRzKMN6kprU&s=yjtNhJKPJdsHD_A52xWSD2g_YNHuLhO1YADKsPaPsio&e= and I
> >> notice that its missing the patch 13 and 17, is this right?
> > No, you just have to refine the filter, I marked the DT bindings as
> > ready to be applied that's why they do not show up (and by the way
> > you can test the series even without those two patches - that are just
> > DT bindings).
> >
> > Thanks,
> > Lorenzo
> >
> >> If not, can you give me both patches location?
> >>
> >> Thanks.
> >>
> >>
> >> On 13/12/2017 17:21, Joao Pinto wrote:
> >>> Hi Niklas,
> >>>
> >>> ??s 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
> >>>> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
> >>>>> Jingoo, Joao,
> >>>>>
> >>>>> I am expecting your testing on the series and ACKs on the dwc related
> >>>>> patches please, according to v4 review - I will mark them as needs
> >>>>> review/ACK waiting for you to chime in.
> >>>>>
> >>>>> v4 thread:
> >>>>>
> >>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
> >>>>>
> >>>> Hello Lorenzo, Jingoo, Joao,
> >>>>
> >>>> Tomorrow another 2 weeks has passed.
> >>>> V1 of this patch series was posted on 2017-10-13.
> >>> Sorry, I have been tight up with a debug session and not able to check this out.
> >>> Adding Gustavo in CC that is now also working in PCI software development.
> >>> I am going to check the code ASAP and we will test it as soon as the debug is
> >>> finished.
> >>>
> >>> Thanks.
> >>>
> >>>> I'm a bit worried that this patch series will not make it to linux-next
> >>>> in time for this patch series to be included in the 4.16 pull request.
> >>>>
> >>>> Here is the V5 patch series:
> >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
> >>>>
> >>>> Perhaps any maintainer of a designware based PCIe driver could
> >>>> help out and test this patch series?
> >>>>
> >>>> Regards,
> >>>> Niklas
> >>>>
> >>
> 
>
Gustavo Pimentel Dec. 18, 2017, 4:11 p.m. | #9
Hi Lorenzo,

Yes, I have retrieve patches and performed some basic tests without having any
problems till now.

However, I'm still trying to test the changes with our automated system. It's
being very difficult to find a time slot available for running this tests.

I hope in this week I could give you go/not go flag. Sorry the delay...

Regards,

Gustavo


On 18/12/2017 15:57, Lorenzo Pieralisi wrote:
> Hi Gustavo,
>
> On Thu, Dec 14, 2017 at 12:38:04PM +0000, Gustavo Pimentel wrote:
>> Ok. I will compile and test it now.
> Have you managed to retrieve the patches and test them ?
>
> Thank you,
> Lorenzo
>
>> Thanks.
>>
>>
>> On 14/12/2017 12:22, Lorenzo Pieralisi wrote:
>>> On Thu, Dec 14, 2017 at 12:16:38PM +0000, Gustavo Pimentel wrote:
>>>> Hi Niklas and Lorenzo,
>>>>
>>>> I'm going to work on PCI software development now as told by Joao and I will
>>>> test your code now.
>>>>
>>>> I was retrieving the patches through the patchwork
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fsubmitter-3D65580&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=paO6WYeoTil-HWIJVBhwaWcIZP7r95MZNRzKMN6kprU&s=yjtNhJKPJdsHD_A52xWSD2g_YNHuLhO1YADKsPaPsio&e= and I
>>>> notice that its missing the patch 13 and 17, is this right?
>>> No, you just have to refine the filter, I marked the DT bindings as
>>> ready to be applied that's why they do not show up (and by the way
>>> you can test the series even without those two patches - that are just
>>> DT bindings).
>>>
>>> Thanks,
>>> Lorenzo
>>>
>>>> If not, can you give me both patches location?
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On 13/12/2017 17:21, Joao Pinto wrote:
>>>>> Hi Niklas,
>>>>>
>>>>> ??s 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
>>>>>> On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
>>>>>>> Jingoo, Joao,
>>>>>>>
>>>>>>> I am expecting your testing on the series and ACKs on the dwc related
>>>>>>> patches please, according to v4 review - I will mark them as needs
>>>>>>> review/ACK waiting for you to chime in.
>>>>>>>
>>>>>>> v4 thread:
>>>>>>>
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
>>>>>>>
>>>>>> Hello Lorenzo, Jingoo, Joao,
>>>>>>
>>>>>> Tomorrow another 2 weeks has passed.
>>>>>> V1 of this patch series was posted on 2017-10-13.
>>>>> Sorry, I have been tight up with a debug session and not able to check this out.
>>>>> Adding Gustavo in CC that is now also working in PCI software development.
>>>>> I am going to check the code ASAP and we will test it as soon as the debug is
>>>>> finished.
>>>>>
>>>>> Thanks.
>>>>>
>>>>>> I'm a bit worried that this patch series will not make it to linux-next
>>>>>> in time for this patch series to be included in the 4.16 pull request.
>>>>>>
>>>>>> Here is the V5 patch series:
>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
>>>>>>
>>>>>> Perhaps any maintainer of a designware based PCIe driver could
>>>>>> help out and test this patch series?
>>>>>>
>>>>>> Regards,
>>>>>> Niklas
>>>>>>
>>
Lorenzo Pieralisi Dec. 19, 2017, 10:19 a.m. | #10
On Mon, Nov 20, 2017 at 02:32:04PM +0100, Niklas Cassel wrote:
> Use the DMA-API to get the MSI address. This address will be written to
> our PCI config space and to the register which determines which AXI
> address the DWC IP will spoof for incoming MSI irqs.
> 
> Since it is a PCIe endpoint device, rather than the CPU, that is supposed
> to write to the MSI address, the proper way to get the MSI address is by
> using the DMA API, not by using virt_to_phys().
> 
> Using virt_to_phys() might work on some systems, but using the DMA API
> should work on all systems.
> 
> This is essentially the same thing as allocating a buffer in a driver
> to which the endpoint will write to. To do this, we use the DMA API.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> ---
>  drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++---
>  drivers/pci/dwc/pcie-designware.h      |  3 ++-
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> index 81e2157a7cfb..33b52fe98a01 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  
>  void dw_pcie_msi_init(struct pcie_port *pp)
>  {
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +	struct page *page;
>  	u64 msi_target;
>  
> -	pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
> -	msi_target = virt_to_phys((void *)pp->msi_data);
> +	page = alloc_page(GFP_KERNEL | GFP_DMA32);

See this thread about GFP_DMA32:

https://patchwork.ozlabs.org/patch/834864/

I need to look back at this set earlier versions, I do not know why
you change the allocation flags but GFP_DMA32 may not provide what
you need - I think you should either remove it or provide a
justification for it given the discussion above.

Lorenzo

> +	pp->msi_data = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dev, pp->msi_data)) {
> +		dev_err(dev, "failed to map MSI data\n");
> +		__free_page(page);
> +		return;
> +	}
> +	msi_target = (u64)pp->msi_data;
>  
>  	/* program the msi_data */
>  	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
> @@ -187,7 +196,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
>  	if (pp->ops->get_msi_addr)
>  		msi_target = pp->ops->get_msi_addr(pp);
>  	else
> -		msi_target = virt_to_phys((void *)pp->msi_data);
> +		msi_target = (u64)pp->msi_data;
>  
>  	msg.address_lo = (u32)(msi_target & 0xffffffff);
>  	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index e5d9d77b778e..ecdede68522a 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -14,6 +14,7 @@
>  #ifndef _PCIE_DESIGNWARE_H
>  #define _PCIE_DESIGNWARE_H
>  
> +#include <linux/dma-mapping.h>
>  #include <linux/irq.h>
>  #include <linux/msi.h>
>  #include <linux/pci.h>
> @@ -168,7 +169,7 @@ struct pcie_port {
>  	const struct dw_pcie_host_ops *ops;
>  	int			msi_irq;
>  	struct irq_domain	*irq_domain;
> -	unsigned long		msi_data;
> +	dma_addr_t		msi_data;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
>  
> -- 
> 2.14.2
>
Lorenzo Pieralisi Dec. 19, 2017, 2:10 p.m. | #11
On Tue, Dec 19, 2017 at 12:45:30PM +0000, Gustavo Pimentel wrote:
>    Tested-By: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>    Using an arc board with USB and SATA end points with a root complex PCIe
>    IP core version 5.00, I ran the the following tests:
>     - I checked that all end points were listed correctly;
>     - I checked that no PCI related errors was present in dmesg;
>     - I copied a file from arc to the USB flash drive and checked the
>    checksum;
>     - I copied a file from USB flash drive to the ARC and checked the
>    checksum;
>     - I copied a file from arc to the SATA disk and checked the checksum;
>     - I copied a file from SATA disk to the arc and checked the checksum;
>     - I copied a file from USB flash drive to the SATA disk and checked the
>    checksum;
>     - I copied a file from SATA disk to the USB flash drive and checked the
>    checksum;
>     - I checked that no PCI related errors was present in dmesg;
> 
>    Everything seemed normal to me.

Thank you very much. I would still need ACKs from the respective
maintainers (Joao/Jingoo/Kishon) in order to push this upstream please.

Thanks,
Lorenzo

>    On 18/12/2017 16:11, Gustavo Pimentel wrote:
> 
>  Hi Lorenzo,
> 
>  Yes, I have retrieve patches and performed some basic tests without having any
>  problems till now.
> 
>  However, I'm still trying to test the changes with our automated system. It's
>  being very difficult to find a time slot available for running this tests.
> 
>  I hope in this week I could give you go/not go flag. Sorry the delay...
> 
>  Regards,
> 
>  Gustavo
> 
> 
>  On 18/12/2017 15:57, Lorenzo Pieralisi wrote:
> 
>  Hi Gustavo,
> 
>  On Thu, Dec 14, 2017 at 12:38:04PM +0000, Gustavo Pimentel wrote:
> 
>  Ok. I will compile and test it now.
> 
>  Have you managed to retrieve the patches and test them ?
> 
>  Thank you,
>  Lorenzo
> 
> 
>  Thanks.
> 
> 
>  On 14/12/2017 12:22, Lorenzo Pieralisi wrote:
> 
>  On Thu, Dec 14, 2017 at 12:16:38PM +0000, Gustavo Pimentel wrote:
> 
>  Hi Niklas and Lorenzo,
> 
>  I'm going to work on PCI software development now as told by Joao and I will
>  test your code now.
> 
>  I was retrieving the patches through the patchwork
>  https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fsubmitter-3D65580&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=paO6WYeoTil-HWIJVBhwaWcIZP7r95MZNRzKMN6kprU&s=yjtNhJKPJdsHD_A52xWSD2g_YNHuLhO1YADKsPaPsio&e= and I
>  notice that its missing the patch 13 and 17, is this right?
> 
>  No, you just have to refine the filter, I marked the DT bindings as
>  ready to be applied that's why they do not show up (and by the way
>  you can test the series even without those two patches - that are just
>  DT bindings).
> 
>  Thanks,
>  Lorenzo
> 
> 
>  If not, can you give me both patches location?
> 
>  Thanks.
> 
> 
>  On 13/12/2017 17:21, Joao Pinto wrote:
> 
>  Hi Niklas,
> 
>  ??s 1:59 PM de 12/13/2017, Niklas Cassel escreveu:
> 
>  On Thu, Nov 30, 2017 at 03:28:43PM +0000, Lorenzo Pieralisi wrote:
> 
>  Jingoo, Joao,
> 
>  I am expecting your testing on the series and ACKs on the dwc related
>  patches please, according to v4 review - I will mark them as needs
>  review/ACK waiting for you to chime in.
> 
>  v4 thread:
> 
>  https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_833882_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=mqyDi45qm4qE0jcm0KviYAoLHmMXMMIWEW-gAR0RKOw&e=
> 
> 
>  Hello Lorenzo, Jingoo, Joao,
> 
>  Tomorrow another 2 weeks has passed.
>  V1 of this patch series was posted on 2017-10-13.
> 
>  Sorry, I have been tight up with a debug session and not able to check this out.
>  Adding Gustavo in CC that is now also working in PCI software development.
>  I am going to check the code ASAP and we will test it as soon as the debug is
>  finished.
> 
>  Thanks.
> 
> 
>  I'm a bit worried that this patch series will not make it to linux-next
>  in time for this patch series to be included in the 4.16 pull request.
> 
>  Here is the V5 patch series:
>  https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list_-3Fseries-3D14364&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=CQAV8rCgm2jd7m3iSWL5vBbGTqSc7yN3N7zeQDz-ZUY&s=qXtUHFXK7_nCndQob0I0qDByBdJsL7rRjZ9cx7c847U&e=
> 
>  Perhaps any maintainer of a designware based PCIe driver could
>  help out and test this patch series?
> 
>  Regards,
>  Niklas
Niklas Cassel Dec. 19, 2017, 10:13 p.m. | #12
On Tue, Dec 19, 2017 at 10:19:18AM +0000, Lorenzo Pieralisi wrote:
> On Mon, Nov 20, 2017 at 02:32:04PM +0100, Niklas Cassel wrote:
> > Use the DMA-API to get the MSI address. This address will be written to
> > our PCI config space and to the register which determines which AXI
> > address the DWC IP will spoof for incoming MSI irqs.
> > 
> > Since it is a PCIe endpoint device, rather than the CPU, that is supposed
> > to write to the MSI address, the proper way to get the MSI address is by
> > using the DMA API, not by using virt_to_phys().
> > 
> > Using virt_to_phys() might work on some systems, but using the DMA API
> > should work on all systems.
> > 
> > This is essentially the same thing as allocating a buffer in a driver
> > to which the endpoint will write to. To do this, we use the DMA API.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> > ---
> >  drivers/pci/dwc/pcie-designware-host.c | 15 ++++++++++++---
> >  drivers/pci/dwc/pcie-designware.h      |  3 ++-
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
> > index 81e2157a7cfb..33b52fe98a01 100644
> > --- a/drivers/pci/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/dwc/pcie-designware-host.c
> > @@ -83,10 +83,19 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
> >  
> >  void dw_pcie_msi_init(struct pcie_port *pp)
> >  {
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct device *dev = pci->dev;
> > +	struct page *page;
> >  	u64 msi_target;
> >  
> > -	pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
> > -	msi_target = virt_to_phys((void *)pp->msi_data);
> > +	page = alloc_page(GFP_KERNEL | GFP_DMA32);
> 
> See this thread about GFP_DMA32:
> 
> https://patchwork.ozlabs.org/patch/834864/
> 
> I need to look back at this set earlier versions, I do not know why
> you change the allocation flags but GFP_DMA32 may not provide what
> you need - I think you should either remove it or provide a
> justification for it given the discussion above.
> 
> Lorenzo

Hello Lorenzo,

I agree, if we want to change this so that the MSI address is guaranteed
to be in the first 4 GB (since some PCIe endpoints only support 32-bit
MSI), we should do so in a separate patch.

Further more, according to the discussion you linked, any such patch
should consider using GFP_DMA instead of GFP_DMA32, since the
ZONE_DMA32 to ZONE_DMA fallback (in case CONFIG_ZONE_DMA32 is not
set) seems to be broken (and has been for several years).

I will submit a new patch set version where I drop GFP_DMA32.


Regards,
Niklas
Niklas Cassel Dec. 19, 2017, 11:55 p.m. | #13
On Tue, Dec 19, 2017 at 12:45:30PM +0000, Gustavo Pimentel wrote:
> Tested-By: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> 
> Using an arc board with USB and SATA end points with a root complex PCIe IP core
> version 5.00, I ran the the following tests:
>  - I checked that all end points were listed correctly;
>  - I checked that no PCI related errors was present in dmesg;
>  - I copied a file from arc to the USB flash drive and checked the checksum;
>  - I copied a file from USB flash drive to the ARC and checked the checksum;
>  - I copied a file from arc to the SATA disk and checked the checksum;
>  - I copied a file from SATA disk to the arc and checked the checksum;
>  - I copied a file from USB flash drive to the SATA disk and checked the checksum;
>  - I copied a file from SATA disk to the USB flash drive and checked the checksum;
>  - I checked that no PCI related errors was present in dmesg;
> 
> Everything seemed normal to me.

Hello Gustavo,

Thanks a lot for testing.

FYI: I submitted a new patch set to address Lorenzo's review comments,
however, I do not think that we need to redo all the testing, since
now we are using the same GFP flags for MSI as we did before, which
obviously worked.

https://marc.info/?l=devicetree&m=151372652827788&w=2

Regards,
Niklas

Patch

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 81e2157a7cfb..33b52fe98a01 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -83,10 +83,19 @@  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 
 void dw_pcie_msi_init(struct pcie_port *pp)
 {
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+	struct page *page;
 	u64 msi_target;
 
-	pp->msi_data = __get_free_pages(GFP_KERNEL, 0);
-	msi_target = virt_to_phys((void *)pp->msi_data);
+	page = alloc_page(GFP_KERNEL | GFP_DMA32);
+	pp->msi_data = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE);
+	if (dma_mapping_error(dev, pp->msi_data)) {
+		dev_err(dev, "failed to map MSI data\n");
+		__free_page(page);
+		return;
+	}
+	msi_target = (u64)pp->msi_data;
 
 	/* program the msi_data */
 	dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
@@ -187,7 +196,7 @@  static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
 	if (pp->ops->get_msi_addr)
 		msi_target = pp->ops->get_msi_addr(pp);
 	else
-		msi_target = virt_to_phys((void *)pp->msi_data);
+		msi_target = (u64)pp->msi_data;
 
 	msg.address_lo = (u32)(msi_target & 0xffffffff);
 	msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff);
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index e5d9d77b778e..ecdede68522a 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -14,6 +14,7 @@ 
 #ifndef _PCIE_DESIGNWARE_H
 #define _PCIE_DESIGNWARE_H
 
+#include <linux/dma-mapping.h>
 #include <linux/irq.h>
 #include <linux/msi.h>
 #include <linux/pci.h>
@@ -168,7 +169,7 @@  struct pcie_port {
 	const struct dw_pcie_host_ops *ops;
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
-	unsigned long		msi_data;
+	dma_addr_t		msi_data;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };