Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
Am 29.01.19 um 21:24 schrieb Logan Gunthorpe: On 2019-01-29 12:56 p.m., Alex Deucher wrote: On Tue, Jan 29, 2019 at 12:47 PM wrote: From: Jérôme Glisse device_test_p2p() return true if two devices can peer to peer to each other. We add a generic function as different inter-connect can support peer to peer and we want to genericaly test this no matter what the inter-connect might be. However this version only support PCIE for now. What about something like these patches: https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=4fab9ff69cb968183f717551441b475fabce6c1c https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=f90b12d41c277335d08c9dab62433f27c0fadbe5 They are a bit more thorough. Those new functions seem to have a lot of overlap with the code that is already upstream in p2pdma Perhaps you should be improving the p2pdma functions if they aren't suitable for what you want already instead of creating new ones. Yeah, well that's what I was suggesting for the very beginning :) But completely agree the existing functions should be improved instead of adding new ones, Christian. Logan ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 3:25 PM Logan Gunthorpe wrote: > > > > On 2019-01-29 12:56 p.m., Alex Deucher wrote: > > On Tue, Jan 29, 2019 at 12:47 PM wrote: > >> > >> From: Jérôme Glisse > >> > >> device_test_p2p() return true if two devices can peer to peer to > >> each other. We add a generic function as different inter-connect > >> can support peer to peer and we want to genericaly test this no > >> matter what the inter-connect might be. However this version only > >> support PCIE for now. > >> > > > > What about something like these patches: > > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=4fab9ff69cb968183f717551441b475fabce6c1c > > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=f90b12d41c277335d08c9dab62433f27c0fadbe5 > > They are a bit more thorough. > > Those new functions seem to have a lot of overlap with the code that is > already upstream in p2pdma Perhaps you should be improving the > p2pdma functions if they aren't suitable for what you want already > instead of creating new ones. Could be. Those patches are pretty old. They probably need to be rebased on the latest upstream p2p stuff. Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 01:44:09PM -0700, Logan Gunthorpe wrote:
>
>
> On 2019-01-29 12:44 p.m., Greg Kroah-Hartman wrote:
> > On Tue, Jan 29, 2019 at 11:24:09AM -0700, Logan Gunthorpe wrote:
> >>
> >>
> >> On 2019-01-29 10:47 a.m., jgli...@redhat.com wrote:
> >>> +bool pci_test_p2p(struct device *devA, struct device *devB)
> >>> +{
> >>> + struct pci_dev *pciA, *pciB;
> >>> + bool ret;
> >>> + int tmp;
> >>> +
> >>> + /*
> >>> + * For now we only support PCIE peer to peer but other inter-connect
> >>> + * can be added.
> >>> + */
> >>> + pciA = find_parent_pci_dev(devA);
> >>> + pciB = find_parent_pci_dev(devB);
> >>> + if (pciA == NULL || pciB == NULL) {
> >>> + ret = false;
> >>> + goto out;
> >>> + }
> >>> +
> >>> + tmp = upstream_bridge_distance(pciA, pciB, NULL);
> >>> + ret = tmp < 0 ? false : true;
> >>> +
> >>> +out:
> >>> + pci_dev_put(pciB);
> >>> + pci_dev_put(pciA);
> >>> + return false;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(pci_test_p2p);
> >>
> >> This function only ever returns false
> >
> > I guess it was nevr actually tested :(
> >
> > I feel really worried about passing random 'struct device' pointers into
> > the PCI layer. Are we _sure_ it can handle this properly?
>
> Yes, there are a couple of pci_p2pdma functions that take struct devices
> directly simply because it's way more convenient for the caller. That's
> what find_parent_pci_dev() takes care of (it returns false if the device
> is not a PCI device). Whether that's appropriate here is hard to say
> seeing we haven't seen any caller code.
Caller code as a reference (i already given that link in other part of
thread but just so that people don't have to follow all branches).
https://cgit.freedesktop.org/~glisse/linux/commit/?h=hmm-p2p=401a567696eafb1d4faf7054ab0d7c3a16a5ef06
Cheers,
Jérôme
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On 2019-01-29 12:44 p.m., Greg Kroah-Hartman wrote:
> On Tue, Jan 29, 2019 at 11:24:09AM -0700, Logan Gunthorpe wrote:
>>
>>
>> On 2019-01-29 10:47 a.m., jgli...@redhat.com wrote:
>>> +bool pci_test_p2p(struct device *devA, struct device *devB)
>>> +{
>>> + struct pci_dev *pciA, *pciB;
>>> + bool ret;
>>> + int tmp;
>>> +
>>> + /*
>>> +* For now we only support PCIE peer to peer but other inter-connect
>>> +* can be added.
>>> +*/
>>> + pciA = find_parent_pci_dev(devA);
>>> + pciB = find_parent_pci_dev(devB);
>>> + if (pciA == NULL || pciB == NULL) {
>>> + ret = false;
>>> + goto out;
>>> + }
>>> +
>>> + tmp = upstream_bridge_distance(pciA, pciB, NULL);
>>> + ret = tmp < 0 ? false : true;
>>> +
>>> +out:
>>> + pci_dev_put(pciB);
>>> + pci_dev_put(pciA);
>>> + return false;
>>> +}
>>> +EXPORT_SYMBOL_GPL(pci_test_p2p);
>>
>> This function only ever returns false
>
> I guess it was nevr actually tested :(
>
> I feel really worried about passing random 'struct device' pointers into
> the PCI layer. Are we _sure_ it can handle this properly?
Yes, there are a couple of pci_p2pdma functions that take struct devices
directly simply because it's way more convenient for the caller. That's
what find_parent_pci_dev() takes care of (it returns false if the device
is not a PCI device). Whether that's appropriate here is hard to say
seeing we haven't seen any caller code.
Logan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On 2019-01-29 12:56 p.m., Alex Deucher wrote: > On Tue, Jan 29, 2019 at 12:47 PM wrote: >> >> From: Jérôme Glisse >> >> device_test_p2p() return true if two devices can peer to peer to >> each other. We add a generic function as different inter-connect >> can support peer to peer and we want to genericaly test this no >> matter what the inter-connect might be. However this version only >> support PCIE for now. >> > > What about something like these patches: > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=4fab9ff69cb968183f717551441b475fabce6c1c > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=f90b12d41c277335d08c9dab62433f27c0fadbe5 > They are a bit more thorough. Those new functions seem to have a lot of overlap with the code that is already upstream in p2pdma Perhaps you should be improving the p2pdma functions if they aren't suitable for what you want already instead of creating new ones. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 02:56:38PM -0500, Alex Deucher wrote: > On Tue, Jan 29, 2019 at 12:47 PM wrote: > > > > From: Jérôme Glisse > > > > device_test_p2p() return true if two devices can peer to peer to > > each other. We add a generic function as different inter-connect > > can support peer to peer and we want to genericaly test this no > > matter what the inter-connect might be. However this version only > > support PCIE for now. > > > > What about something like these patches: > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=4fab9ff69cb968183f717551441b475fabce6c1c > https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=f90b12d41c277335d08c9dab62433f27c0fadbe5 > They are a bit more thorough. Yes it would be better, i forgot about those. I can include them next time i post. Thank you for reminding me about those :) Cheers, Jérôme ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 12:47 PM wrote:
>
> From: Jérôme Glisse
>
> device_test_p2p() return true if two devices can peer to peer to
> each other. We add a generic function as different inter-connect
> can support peer to peer and we want to genericaly test this no
> matter what the inter-connect might be. However this version only
> support PCIE for now.
>
What about something like these patches:
https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=4fab9ff69cb968183f717551441b475fabce6c1c
https://cgit.freedesktop.org/~deathsimple/linux/commit/?h=p2p=f90b12d41c277335d08c9dab62433f27c0fadbe5
They are a bit more thorough.
Alex
> Signed-off-by: Jérôme Glisse
> Cc: Logan Gunthorpe
> Cc: Greg Kroah-Hartman
> Cc: Rafael J. Wysocki
> Cc: Bjorn Helgaas
> Cc: Christian Koenig
> Cc: Felix Kuehling
> Cc: Jason Gunthorpe
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: Christoph Hellwig
> Cc: Marek Szyprowski
> Cc: Robin Murphy
> Cc: Joerg Roedel
> Cc: iommu@lists.linux-foundation.org
> ---
> drivers/pci/p2pdma.c | 27 +++
> include/linux/pci-p2pdma.h | 6 ++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index c52298d76e64..620ac60babb5 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -797,3 +797,30 @@ ssize_t pci_p2pdma_enable_show(char *page, struct
> pci_dev *p2p_dev,
> return sprintf(page, "%s\n", pci_name(p2p_dev));
> }
> EXPORT_SYMBOL_GPL(pci_p2pdma_enable_show);
> +
> +bool pci_test_p2p(struct device *devA, struct device *devB)
> +{
> + struct pci_dev *pciA, *pciB;
> + bool ret;
> + int tmp;
> +
> + /*
> +* For now we only support PCIE peer to peer but other inter-connect
> +* can be added.
> +*/
> + pciA = find_parent_pci_dev(devA);
> + pciB = find_parent_pci_dev(devB);
> + if (pciA == NULL || pciB == NULL) {
> + ret = false;
> + goto out;
> + }
> +
> + tmp = upstream_bridge_distance(pciA, pciB, NULL);
> + ret = tmp < 0 ? false : true;
> +
> +out:
> + pci_dev_put(pciB);
> + pci_dev_put(pciA);
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(pci_test_p2p);
> diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
> index bca9bc3e5be7..7671cc499a08 100644
> --- a/include/linux/pci-p2pdma.h
> +++ b/include/linux/pci-p2pdma.h
> @@ -36,6 +36,7 @@ int pci_p2pdma_enable_store(const char *page, struct
> pci_dev **p2p_dev,
> bool *use_p2pdma);
> ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev,
>bool use_p2pdma);
> +bool pci_test_p2p(struct device *devA, struct device *devB);
> #else /* CONFIG_PCI_P2PDMA */
> static inline int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar,
> size_t size, u64 offset)
> @@ -97,6 +98,11 @@ static inline ssize_t pci_p2pdma_enable_show(char *page,
> {
> return sprintf(page, "none\n");
> }
> +
> +static inline bool pci_test_p2p(struct device *devA, struct device *devB)
> +{
> + return false;
> +}
> #endif /* CONFIG_PCI_P2PDMA */
>
>
> --
> 2.17.2
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 08:44:26PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 29, 2019 at 11:24:09AM -0700, Logan Gunthorpe wrote:
> >
> >
> > On 2019-01-29 10:47 a.m., jgli...@redhat.com wrote:
> > > +bool pci_test_p2p(struct device *devA, struct device *devB)
> > > +{
> > > + struct pci_dev *pciA, *pciB;
> > > + bool ret;
> > > + int tmp;
> > > +
> > > + /*
> > > + * For now we only support PCIE peer to peer but other inter-connect
> > > + * can be added.
> > > + */
> > > + pciA = find_parent_pci_dev(devA);
> > > + pciB = find_parent_pci_dev(devB);
> > > + if (pciA == NULL || pciB == NULL) {
> > > + ret = false;
> > > + goto out;
> > > + }
> > > +
> > > + tmp = upstream_bridge_distance(pciA, pciB, NULL);
> > > + ret = tmp < 0 ? false : true;
> > > +
> > > +out:
> > > + pci_dev_put(pciB);
> > > + pci_dev_put(pciA);
> > > + return false;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_test_p2p);
> >
> > This function only ever returns false
>
> I guess it was nevr actually tested :(
>
> I feel really worried about passing random 'struct device' pointers into
> the PCI layer. Are we _sure_ it can handle this properly?
>
Oh yes i fixed it on the test rig and forgot to patch
my local git tree. My bad.
Cheers,
Jérôme
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On Tue, Jan 29, 2019 at 11:24:09AM -0700, Logan Gunthorpe wrote:
>
>
> On 2019-01-29 10:47 a.m., jgli...@redhat.com wrote:
> > +bool pci_test_p2p(struct device *devA, struct device *devB)
> > +{
> > + struct pci_dev *pciA, *pciB;
> > + bool ret;
> > + int tmp;
> > +
> > + /*
> > +* For now we only support PCIE peer to peer but other inter-connect
> > +* can be added.
> > +*/
> > + pciA = find_parent_pci_dev(devA);
> > + pciB = find_parent_pci_dev(devB);
> > + if (pciA == NULL || pciB == NULL) {
> > + ret = false;
> > + goto out;
> > + }
> > +
> > + tmp = upstream_bridge_distance(pciA, pciB, NULL);
> > + ret = tmp < 0 ? false : true;
> > +
> > +out:
> > + pci_dev_put(pciB);
> > + pci_dev_put(pciA);
> > + return false;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_test_p2p);
>
> This function only ever returns false
I guess it was nevr actually tested :(
I feel really worried about passing random 'struct device' pointers into
the PCI layer. Are we _sure_ it can handle this properly?
thanks,
greg k-h
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/5] pci/p2p: add a function to test peer to peer capability
On 2019-01-29 10:47 a.m., jgli...@redhat.com wrote:
> +bool pci_test_p2p(struct device *devA, struct device *devB)
> +{
> + struct pci_dev *pciA, *pciB;
> + bool ret;
> + int tmp;
> +
> + /*
> + * For now we only support PCIE peer to peer but other inter-connect
> + * can be added.
> + */
> + pciA = find_parent_pci_dev(devA);
> + pciB = find_parent_pci_dev(devB);
> + if (pciA == NULL || pciB == NULL) {
> + ret = false;
> + goto out;
> + }
> +
> + tmp = upstream_bridge_distance(pciA, pciB, NULL);
> + ret = tmp < 0 ? false : true;
> +
> +out:
> + pci_dev_put(pciB);
> + pci_dev_put(pciA);
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(pci_test_p2p);
This function only ever returns false
Logan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
