linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Stephen Bates <sbates@raithlin.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Logan Gunthorpe <logang@deltatee.com>,
	Alex Williamson <alex.williamson@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"Christoph Hellwig" <hch@lst.de>, "Jens Axboe" <axboe@kernel.dk>,
	"Keith Busch" <keith.busch@intel.com>,
	"Sagi Grimberg" <sagi@grimberg.me>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Jason Gunthorpe" <jgg@mellanox.com>,
	"Max Gurtovoy" <maxg@mellanox.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Benjamin Herrenschmidt" <benh@kernel.crashing.org>
Subject: Re: [PATCH v4 04/14] PCI/P2PDMA: Clear ACS P2P flags for all devices behind switches
Date: Tue, 8 May 2018 18:37:51 +0200	[thread overview]
Message-ID: <44488fb8-8105-b6f8-3438-720ee39d8520@amd.com> (raw)
In-Reply-To: <479F2F93-903B-4A8A-8989-A4BD0E5E1471@raithlin.com>

Am 08.05.2018 um 16:25 schrieb Stephen Bates:
>      
> Hi Christian
>
>> AMD APUs mandatory need the ACS flag set for the GPU integrated in the
>> CPU when IOMMU is enabled or otherwise you will break SVM.
> OK but in this case aren't you losing (many of) the benefits of P2P since all DMAs will now get routed up to the IOMMU before being passed down to the destination PCIe EP?

Well I'm not an expert on this, but I think that is an incorrect 
assumption you guys use here.

At least in the default configuration even with IOMMU enabled P2P 
transactions does NOT necessary travel up to the root complex for 
translation.

It's already late here, but if nobody beats me I'm going to dig up the 
necessary documentation tomorrow.

Regards,
Christian.

>
>> Similar problems arise when you do this for dedicated GPU, but we
>> haven't upstreamed the support for this yet.
> Hmm, as above. With ACS enabled on all downstream ports any P2P enabled DMA will be routed to the IOMMU which removes a lot of the benefit.
>      
>> So that is a clear NAK from my side for the approach.
> Do you have an alternative? This is the approach we arrived it after a reasonably lengthy discussion on the mailing lists. Alex, are you still comfortable with this approach?
>      
>> And what exactly is the problem here?
>   
> We had a pretty lengthy discussion on this topic on one of the previous revisions. The issue is that currently there is no mechanism in the IOMMU code to inform VMs if IOMMU groupings change. Since p2pdma can dynamically change its topology (due to PCI hotplug) we had to be cognizant of the fact that ACS settings could change. Since there is no way to currently handle changing ACS settings and hence IOMMU groupings the consensus was to simply disable ACS on all ports in a p2pdma domain. This effectively makes all the devices in the p2pdma domain part of the same IOMMU grouping. The plan will be to address this in time and add a mechanism for IOMMU grouping changes and notification to VMs but that's not part of this series. Note you are still allowed to have ACS functioning on other PCI domains so if you do not a plurality of IOMMU groupings you can still achieve it (but you can't do p2pdma across IOMMU groupings, which is safe).
>
>> I'm currently testing P2P with  GPUs in different IOMMU domains and at least with AMD IOMMUs that works perfectly fine.
> Yup that should work though again I have to ask are you disabling ACS on the ports between the two peer devices to get the p2p benefit? If not you are not getting all the performance benefit (due to IOMMU routing), if you are then there are obviously security implications between those IOMMU domains if they are assigned to different VMs. And now the issue is if new devices are added and the p2p topology needed to change there would be no way to inform the VMs of any IOMMU group change.
>
> Cheers
>
> Stephen
>      
>

  reply	other threads:[~2018-05-08 16:38 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-23 23:30 [PATCH v4 00/14] Copy Offload in NVMe Fabrics with P2P PCI Memory Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 01/14] PCI/P2PDMA: Support peer-to-peer memory Logan Gunthorpe
2018-05-07 23:00   ` Bjorn Helgaas
2018-05-07 23:09     ` Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 02/14] PCI/P2PDMA: Add sysfs group to display p2pmem stats Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 03/14] PCI/P2PDMA: Add PCI p2pmem dma mappings to adjust the bus offset Logan Gunthorpe
2018-05-07 23:02   ` Bjorn Helgaas
2018-04-23 23:30 ` [PATCH v4 04/14] PCI/P2PDMA: Clear ACS P2P flags for all devices behind switches Logan Gunthorpe
2018-04-24  3:33   ` Randy Dunlap
2018-05-07 23:13   ` Bjorn Helgaas
2018-05-08  7:17     ` Christian König
2018-05-08 14:25       ` Stephen  Bates
2018-05-08 16:37         ` Christian König [this message]
2018-05-08 16:27       ` Logan Gunthorpe
2018-05-08 16:50         ` Christian König
2018-05-08 19:13           ` Logan Gunthorpe
2018-05-08 19:34             ` Alex Williamson
2018-05-08 19:45               ` Logan Gunthorpe
2018-05-08 20:13                 ` Alex Williamson
2018-05-08 20:19                   ` Logan Gunthorpe
2018-05-08 20:43                     ` Alex Williamson
2018-05-08 20:49                       ` Logan Gunthorpe
2018-05-08 21:26                         ` Alex Williamson
2018-05-08 21:42                           ` Stephen  Bates
2018-05-08 22:03                             ` Alex Williamson
2018-05-08 22:10                               ` Logan Gunthorpe
2018-05-08 22:25                                 ` Stephen  Bates
2018-05-08 23:11                                   ` Alex Williamson
2018-05-08 23:31                                     ` Logan Gunthorpe
2018-05-09  0:17                                       ` Alex Williamson
2018-05-08 22:32                                 ` Alex Williamson
2018-05-08 23:00                                   ` Dan Williams
2018-05-08 23:15                                     ` Logan Gunthorpe
2018-05-09 12:38                                       ` Stephen  Bates
2018-05-08 22:21                               ` Don Dutile
2018-05-09 12:44                                 ` Stephen  Bates
2018-05-09 15:58                                   ` Don Dutile
2018-05-08 20:50                     ` Jerome Glisse
2018-05-08 21:35                       ` Stephen  Bates
2018-05-09 13:12                       ` Stephen  Bates
2018-05-09 13:40                         ` Christian König
2018-05-09 15:41                           ` Stephen  Bates
2018-05-09 16:07                             ` Jerome Glisse
2018-05-09 16:30                               ` Stephen  Bates
2018-05-09 17:49                                 ` Jerome Glisse
2018-05-10 14:20                                   ` Stephen  Bates
2018-05-10 14:29                                     ` Christian König
2018-05-10 14:59                                       ` Jerome Glisse
2018-05-10 18:44                                         ` Stephen  Bates
2018-05-09 16:45                           ` Logan Gunthorpe
2018-05-10 12:52                             ` Christian König
2018-05-10 14:16                               ` Stephen  Bates
2018-05-10 14:41                                 ` Jerome Glisse
2018-05-10 18:41                                   ` Stephen  Bates
2018-05-10 18:59                                     ` Logan Gunthorpe
2018-05-10 19:10                                     ` Alex Williamson
2018-05-10 19:24                                       ` Jerome Glisse
2018-05-10 16:32                                 ` Logan Gunthorpe
2018-05-10 17:11                                   ` Stephen  Bates
2018-05-10 17:15                                     ` Logan Gunthorpe
2018-05-11  8:52                                       ` Christian König
2018-05-11 15:48                                         ` Logan Gunthorpe
2018-05-11 21:50                                           ` Stephen  Bates
2018-05-11 22:24                                             ` Stephen  Bates
2018-05-11 22:55                                               ` Logan Gunthorpe
2018-05-08 14:31   ` Dan Williams
2018-05-08 14:44     ` Stephen  Bates
2018-05-08 21:04       ` Don Dutile
2018-05-08 21:27         ` Stephen  Bates
2018-05-08 23:06           ` Don Dutile
2018-05-09  0:01             ` Alex Williamson
2018-05-09 12:35               ` Stephen  Bates
2018-05-09 14:44                 ` Alex Williamson
2018-05-09 15:52                   ` Don Dutile
2018-05-09 15:47               ` Don Dutile
2018-05-09 15:53           ` Don Dutile
2018-04-23 23:30 ` [PATCH v4 05/14] docs-rst: Add a new directory for PCI documentation Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 06/14] PCI/P2PDMA: Add P2P DMA driver writer's documentation Logan Gunthorpe
2018-05-07 23:20   ` Bjorn Helgaas
2018-05-22 21:24   ` Randy Dunlap
2018-05-22 21:28     ` Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 07/14] block: Introduce PCI P2P flags for request and request queue Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 08/14] IB/core: Ensure we map P2P memory correctly in rdma_rw_ctx_[init|destroy]() Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 09/14] nvme-pci: Use PCI p2pmem subsystem to manage the CMB Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 10/14] nvme-pci: Add support for P2P memory in requests Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 11/14] nvme-pci: Add a quirk for a pseudo CMB Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 12/14] nvmet: Introduce helper functions to allocate and free request SGLs Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 13/14] nvmet-rdma: Use new SGL alloc/free helper for requests Logan Gunthorpe
2018-04-23 23:30 ` [PATCH v4 14/14] nvmet: Optionally use PCI P2P memory Logan Gunthorpe
2018-05-02 11:51 ` [PATCH v4 00/14] Copy Offload in NVMe Fabrics with P2P PCI Memory Christian König
2018-05-02 15:56   ` Logan Gunthorpe
2018-05-03  9:05     ` Christian König
2018-05-03 15:59       ` Logan Gunthorpe
2018-05-03 17:29         ` Christian König
2018-05-03 18:43           ` Logan Gunthorpe
2018-05-04 14:27             ` Christian König
2018-05-04 15:52               ` Logan Gunthorpe
2018-05-07 23:23 ` Bjorn Helgaas
2018-05-07 23:34   ` Logan Gunthorpe
2018-05-08 16:57   ` Alex Williamson
2018-05-08 19:14     ` Logan Gunthorpe
2018-05-08 21:25     ` Don Dutile
2018-05-08 21:40       ` Alex Williamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44488fb8-8105-b6f8-3438-720ee39d8520@amd.com \
    --to=christian.koenig@amd.com \
    --cc=alex.williamson@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=benh@kernel.crashing.org \
    --cc=bhelgaas@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=helgaas@kernel.org \
    --cc=jgg@mellanox.com \
    --cc=jglisse@redhat.com \
    --cc=keith.busch@intel.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=logang@deltatee.com \
    --cc=maxg@mellanox.com \
    --cc=sagi@grimberg.me \
    --cc=sbates@raithlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).