diff mbox series

Whitelist AMD host bridge device(s) to enable P2P DMA

Message ID 20210811191104.21919-1-Ramesh.Errabolu@amd.com (mailing list archive)
State New, archived
Headers show
Series Whitelist AMD host bridge device(s) to enable P2P DMA | expand

Commit Message

Ramesh Errabolu Aug. 11, 2021, 7:11 p.m. UTC
Current implementation will disallow P2P DMA if the participating
devices belong to different root complexes. Implementation allows
this default behavior to be overridden for whitelisted devices. The
patch adds an AMD host bridge to be whitelisted

Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
---
 drivers/pci/p2pdma.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Alex Deucher Aug. 11, 2021, 7:29 p.m. UTC | #1
On Wed, Aug 11, 2021 at 3:11 PM Ramesh Errabolu <Ramesh.Errabolu@amd.com> wrote:
>
> Current implementation will disallow P2P DMA if the participating
> devices belong to different root complexes. Implementation allows
> this default behavior to be overridden for whitelisted devices. The
> patch adds an AMD host bridge to be whitelisted

Why do we need this?  cpu_supports_p2pdma() should return true for all
AMD Zen CPUs.

Alex

>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> ---
>  drivers/pci/p2pdma.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 196382630363..7003bb9faf23 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -305,6 +305,8 @@ static const struct pci_p2pdma_whitelist_entry {
>         {PCI_VENDOR_ID_INTEL,   0x2032, 0},
>         {PCI_VENDOR_ID_INTEL,   0x2033, 0},
>         {PCI_VENDOR_ID_INTEL,   0x2020, 0},
> +       /* AMD Host Bridge Devices */
> +       {PCI_VENDOR_ID_AMD,     0x1480, 0},
>         {}
>  };
>
> --
> 2.31.1
>
Felix Kuehling Aug. 11, 2021, 8:50 p.m. UTC | #2
Am 2021-08-11 um 3:29 p.m. schrieb Alex Deucher:
> On Wed, Aug 11, 2021 at 3:11 PM Ramesh Errabolu <Ramesh.Errabolu@amd.com> wrote:
>> Current implementation will disallow P2P DMA if the participating
>> devices belong to different root complexes. Implementation allows
>> this default behavior to be overridden for whitelisted devices. The
>> patch adds an AMD host bridge to be whitelisted
> Why do we need this?  cpu_supports_p2pdma() should return true for all
> AMD Zen CPUs.

This is part of our on-going work to get P2P support upstream. We want
to use pci_p2pdma_distance_many to determine whether P2P is possible
between a pair of devices. This whitelist is used in this function. This
will affect the P2P links reported in the topology and it will be
double-checked in the BO mapping function to ensure a peer mapping is legal.

I think this change is a bit free of context at the moment, as we're
still working on a few other loose ends for P2P support in our internal
branch. I'm hoping we'll have a bigger patch series for upstreamable KFD
P2P support ready in a few weeks. I also think we'll probably want to
add a few more PCI IDs for other supported AMD root complexes.

Regards,
  Felix


>
> Alex
>
>> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
>> ---
>>  drivers/pci/p2pdma.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
>> index 196382630363..7003bb9faf23 100644
>> --- a/drivers/pci/p2pdma.c
>> +++ b/drivers/pci/p2pdma.c
>> @@ -305,6 +305,8 @@ static const struct pci_p2pdma_whitelist_entry {
>>         {PCI_VENDOR_ID_INTEL,   0x2032, 0},
>>         {PCI_VENDOR_ID_INTEL,   0x2033, 0},
>>         {PCI_VENDOR_ID_INTEL,   0x2020, 0},
>> +       /* AMD Host Bridge Devices */
>> +       {PCI_VENDOR_ID_AMD,     0x1480, 0},
>>         {}
>>  };
>>
>> --
>> 2.31.1
>>
Alex Deucher Aug. 11, 2021, 9:06 p.m. UTC | #3
On Wed, Aug 11, 2021 at 4:50 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
>
> Am 2021-08-11 um 3:29 p.m. schrieb Alex Deucher:
> > On Wed, Aug 11, 2021 at 3:11 PM Ramesh Errabolu <Ramesh.Errabolu@amd.com> wrote:
> >> Current implementation will disallow P2P DMA if the participating
> >> devices belong to different root complexes. Implementation allows
> >> this default behavior to be overridden for whitelisted devices. The
> >> patch adds an AMD host bridge to be whitelisted
> > Why do we need this?  cpu_supports_p2pdma() should return true for all
> > AMD Zen CPUs.
>
> This is part of our on-going work to get P2P support upstream. We want
> to use pci_p2pdma_distance_many to determine whether P2P is possible
> between a pair of devices. This whitelist is used in this function. This
> will affect the P2P links reported in the topology and it will be
> double-checked in the BO mapping function to ensure a peer mapping is legal.
>
> I think this change is a bit free of context at the moment, as we're
> still working on a few other loose ends for P2P support in our internal
> branch. I'm hoping we'll have a bigger patch series for upstreamable KFD
> P2P support ready in a few weeks. I also think we'll probably want to
> add a few more PCI IDs for other supported AMD root complexes.

We don't need to keep adding AMD root complexes.  You must be using an
older kernel or something.  All root complexes on all Zen platforms
support P2P DMA.  See:

commit dea286bb71baded7d2fb4f090e3b9fd2c1ccac58
Author: Logan Gunthorpe <logang@deltatee.com>
Date:   Wed Jul 29 17:18:44 2020 -0600

    PCI/P2PDMA: Allow P2PDMA on AMD Zen and newer CPUs

    Allow P2PDMA if the CPU vendor is AMD and family is 0x17 (Zen) or greater.

    [bhelgaas: commit log, simplify #if/#else/#endif]
    Link: https://lore.kernel.org/r/20200729231844.4653-1-logang@deltatee.com
    Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
    Cc: Christian König <christian.koenig@amd.com>
    Cc: Huang Rui <ray.huang@amd.com>

Alex


>
> Regards,
>   Felix
>
>
> >
> > Alex
> >
> >> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> >> ---
> >>  drivers/pci/p2pdma.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> >> index 196382630363..7003bb9faf23 100644
> >> --- a/drivers/pci/p2pdma.c
> >> +++ b/drivers/pci/p2pdma.c
> >> @@ -305,6 +305,8 @@ static const struct pci_p2pdma_whitelist_entry {
> >>         {PCI_VENDOR_ID_INTEL,   0x2032, 0},
> >>         {PCI_VENDOR_ID_INTEL,   0x2033, 0},
> >>         {PCI_VENDOR_ID_INTEL,   0x2020, 0},
> >> +       /* AMD Host Bridge Devices */
> >> +       {PCI_VENDOR_ID_AMD,     0x1480, 0},
> >>         {}
> >>  };
> >>
> >> --
> >> 2.31.1
> >>
Ramesh Errabolu Aug. 12, 2021, 1:09 a.m. UTC | #4
I will investigate it further. I am using DKMS-5.11 branch. The codebase I am using to build has the right definition i.e. allow P2PDMA for Zen CPU's.

Regards,
Ramesh

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com> 
Sent: Wednesday, August 11, 2021 4:07 PM
To: Kuehling, Felix <Felix.Kuehling@amd.com>
Cc: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>; Maling list - DRI developers <dri-devel@lists.freedesktop.org>; Bjorn Helgaas <bhelgaas@google.com>; Linux PCI <linux-pci@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Whitelist AMD host bridge device(s) to enable P2P DMA

[CAUTION: External Email]

On Wed, Aug 11, 2021 at 4:50 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
>
> Am 2021-08-11 um 3:29 p.m. schrieb Alex Deucher:
> > On Wed, Aug 11, 2021 at 3:11 PM Ramesh Errabolu <Ramesh.Errabolu@amd.com> wrote:
> >> Current implementation will disallow P2P DMA if the participating 
> >> devices belong to different root complexes. Implementation allows 
> >> this default behavior to be overridden for whitelisted devices. The 
> >> patch adds an AMD host bridge to be whitelisted
> > Why do we need this?  cpu_supports_p2pdma() should return true for 
> > all AMD Zen CPUs.
>
> This is part of our on-going work to get P2P support upstream. We want 
> to use pci_p2pdma_distance_many to determine whether P2P is possible 
> between a pair of devices. This whitelist is used in this function. 
> This will affect the P2P links reported in the topology and it will be 
> double-checked in the BO mapping function to ensure a peer mapping is legal.
>
> I think this change is a bit free of context at the moment, as we're 
> still working on a few other loose ends for P2P support in our 
> internal branch. I'm hoping we'll have a bigger patch series for 
> upstreamable KFD P2P support ready in a few weeks. I also think we'll 
> probably want to add a few more PCI IDs for other supported AMD root complexes.

We don't need to keep adding AMD root complexes.  You must be using an older kernel or something.  All root complexes on all Zen platforms support P2P DMA.  See:

commit dea286bb71baded7d2fb4f090e3b9fd2c1ccac58
Author: Logan Gunthorpe <logang@deltatee.com>
Date:   Wed Jul 29 17:18:44 2020 -0600

    PCI/P2PDMA: Allow P2PDMA on AMD Zen and newer CPUs

    Allow P2PDMA if the CPU vendor is AMD and family is 0x17 (Zen) or greater.

    [bhelgaas: commit log, simplify #if/#else/#endif]
    Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20200729231844.4653-1-logang%40deltatee.com&amp;data=04%7C01%7CRamesh.Errabolu%40amd.com%7C7ffc96d473d4471889f808d95d0bf57b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637643128220735445%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=vmwJP2VwKPrXwl3ngUfwN%2BJu3JWeMP0ZZMDh29evW%2Bg%3D&amp;reserved=0
    Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
    Cc: Christian König <christian.koenig@amd.com>
    Cc: Huang Rui <ray.huang@amd.com>

Alex


>
> Regards,
>   Felix
>
>
> >
> > Alex
> >
> >> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> >> ---
> >>  drivers/pci/p2pdma.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 
> >> 196382630363..7003bb9faf23 100644
> >> --- a/drivers/pci/p2pdma.c
> >> +++ b/drivers/pci/p2pdma.c
> >> @@ -305,6 +305,8 @@ static const struct pci_p2pdma_whitelist_entry {
> >>         {PCI_VENDOR_ID_INTEL,   0x2032, 0},
> >>         {PCI_VENDOR_ID_INTEL,   0x2033, 0},
> >>         {PCI_VENDOR_ID_INTEL,   0x2020, 0},
> >> +       /* AMD Host Bridge Devices */
> >> +       {PCI_VENDOR_ID_AMD,     0x1480, 0},
> >>         {}
> >>  };
> >>
> >> --
> >> 2.31.1
> >>
diff mbox series

Patch

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 196382630363..7003bb9faf23 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -305,6 +305,8 @@  static const struct pci_p2pdma_whitelist_entry {
 	{PCI_VENDOR_ID_INTEL,	0x2032, 0},
 	{PCI_VENDOR_ID_INTEL,	0x2033, 0},
 	{PCI_VENDOR_ID_INTEL,	0x2020, 0},
+	/* AMD Host Bridge Devices */
+	{PCI_VENDOR_ID_AMD,	0x1480, 0},
 	{}
 };