On Tue, Oct 15, 2024 at 01:13:22PM +0200, Thomas Hellström wrote:
> Introduce a way for hmm_range_fault() and migrate_vma_setup() to identify
> foreign devices with fast interconnect and thereby allow
> both direct access over the interconnect and p2p migration.
>
> The need for a callback arises because without it, the p2p ability would
> need to be static and determined at dev_pagemap creation time. With
> a callback it can be determined dynamically, and in the migrate case
> the callback could separate out local device pages.
> +static bool hmm_allow_devmem(struct hmm_range *range, struct page *page)
> +{
> + if (likely(page->pgmap->owner == range->dev_private_owner))
> + return true;
> + if (likely(!range->p2p))
> + return false;
> + return range->p2p->ops->p2p_allow(range->p2p, page);
> +}
> +
> static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
> unsigned long end, pmd_t *pmdp, pte_t *ptep,
> unsigned long *hmm_pfn)
> @@ -248,8 +258,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
> * just report the PFN.
> */
> if (is_device_private_entry(entry) &&
> - pfn_swap_entry_to_page(entry)->pgmap->owner ==
> - range->dev_private_owner) {
> + hmm_allow_devmem(range, pfn_swap_entry_to_page(entry))) {
> cpu_flags = HMM_PFN_VALID;
> if (is_writable_device_private_entry(entry))
> cpu_flags |= HMM_PFN_WRITE;
This is really misnamed and took me a while to get it.
It has nothing to do with kernel P2P, you are just allowing more
selective filtering of dev_private_owner. You should focus on that in
the naming, not p2p. ie allow_dev_private()
P2P is stuff that is dealing with MEMORY_DEVICE_PCI_P2PDMA.
This is just allowing more instances of the same driver to co-ordinate
their device private memory handle, for whatever purpose.
Otherwise I don't see a particular problem, though we have talked
about widening the matching for device_private more broadly using some
kind of grouping tag or something like that instead of a callback. You
may consider that as an alternative
I would also probably try to have less indirection, you can embedd the
hmm_range struct inside a caller private data struct and use that
instead if inventing a whole new struct and pointer.
Jason
