Nouveau - Oct 2024 - [PATCH v1 1/4] mm/hmm: HMM API for P2P DMA to device zone pages

On Wed, Oct 16, 2024 at 02:44:45PM -0300, Jason Gunthorpe
wrote:
> > > FWIW, I've been expecting this series to be rebased on top of
Leon's
> > > new DMA API series so it doesn't have this issue..
> > 
> > That's not going to make a difference at this level.
> 
> I'm not sure what you are asking then.
> 
> Patch 2 does pci_p2pdma_add_resource() and so a valid struct page with
> a P2P ZONE_DEVICE type exists, and that gets returned back to the
> hmm/odp code.
> 
> Today odp calls dma_map_page() which only works by chance in limited
> cases. With Leon's revision it will call hmm_dma_map_pfn() ->
> dma_iova_link() which does call pci_p2pdma_map_type() and should do
> the right thing.
Again none of this affects the code posted here.  It reshuffles the
callers but has no direct affect on the patches posted here.

(and the current DMA series lacks P2P support, I'm trying to figure
out how to properly handle it at the moment).
> > IOMMU or not doens't matter much for P2P.  The important
difference is
> > through the host bridge or through a switch.  dma_map_page will work
> > for P2P through the host brige (assuming the host bridge even support
> > it as it also lacks the error handling for when not), but it lacks the
> > handling for P2P through a switch.
> 
> On most x86 systems the BAR/bus address of the P2P memory is the same
> as the CPU address, so without an IOMMU translation dma_map_page()
> will return the CPU/host physical address which is the same as the
> BAR/bus address and that will take the P2P switch path for testing.
Maybe.  Either way the use of dma_map_page is incorrect.

> 
> Jason
---end quoted text---

On Thu, Oct 17, 2024 at 04:58:12AM -0700, Christoph Hellwig
wrote:
> On Wed, Oct 16, 2024 at 02:44:45PM -0300, Jason Gunthorpe wrote:
> > > > FWIW, I've been expecting this series to be rebased on
top of Leon's
> > > > new DMA API series so it doesn't have this issue..
> > > 
> > > That's not going to make a difference at this level.
> > 
> > I'm not sure what you are asking then.
> > 
> > Patch 2 does pci_p2pdma_add_resource() and so a valid struct page with
> > a P2P ZONE_DEVICE type exists, and that gets returned back to the
> > hmm/odp code.
> > 
> > Today odp calls dma_map_page() which only works by chance in limited
> > cases. With Leon's revision it will call hmm_dma_map_pfn() ->
> > dma_iova_link() which does call pci_p2pdma_map_type() and should do
> > the right thing.
> 
> Again none of this affects the code posted here.  It reshuffles the
> callers but has no direct affect on the patches posted here.
I didn't realize till last night that Leon's series did not have P2P
support.

What I'm trying to say is that this is a multi-series project.

A followup based on Leon's initial work will get the ODP DMA mapping
path able to support ZONE_DEVICE P2P pages.

Once that is done, this series sits on top. This series is only about
hmm and effectively allows hmm_range_fault() to return a ZONE_DEVICE
P2P page.

Yonatan should explain this better in the cover letter and mark it as
a RFC series.

So, I know we are still figuring out the P2P support on the DMA API
side, but my expectation for hmm is that hmm_range_fault() returing a
ZONE_DEVICE P2P page is going to be what we want.
> (and the current DMA series lacks P2P support, I'm trying to figure
> out how to properly handle it at the moment).
Yes, I see, I looked through those patches last night and there is a
gap there.

Broadly I think whatever flow NVMe uses for P2P will apply to ODP as
well.

Thanks,
Jason