On Wed, Apr 3, 2019 at 9:42 PM Anshuman Khandual <anshuman.khandual@xxxxxxx> wrote: > > > > On 04/03/2019 07:28 PM, Robin Murphy wrote: > > [ +Dan, Jerome ] > > > > On 03/04/2019 05:30, Anshuman Khandual wrote: > >> Arch implementation for functions which create or destroy vmemmap mapping > >> (vmemmap_populate, vmemmap_free) can comprehend and allocate from inside > >> device memory range through driver provided vmem_altmap structure which > >> fulfils all requirements to enable ZONE_DEVICE on the platform. Hence just > > > > ZONE_DEVICE is about more than just altmap support, no? > > Hot plugging the memory into a dev->numa_node's ZONE_DEVICE and initializing the > struct pages for it has stand alone and self contained use case. The driver could > just want to manage the memory itself but with struct pages either in the RAM or > in the device memory range through struct vmem_altmap. The driver may not choose > to opt for HMM, FS DAX, P2PDMA (use cases of ZONE_DEVICE) where it may have to > map these pages into any user pagetable which would necessitate support for > pte|pmd|pud_devmap. What's left for ZONE_DEVICE if none of the above cases are used? > Though I am still working towards getting HMM, FS DAX, P2PDMA enabled on arm64, > IMHO ZONE_DEVICE is self contained and can be evaluated in itself. I'm not convinced. What's the specific use case. > > > > >> enable ZONE_DEVICE by subscribing to ARCH_HAS_ZONE_DEVICE. But this is only > >> applicable for ARM64_4K_PAGES (ARM64_SWAPPER_USES_SECTION_MAPS) only which > >> creates vmemmap section mappings and utilize vmem_altmap structure. > > > > What prevents it from working with other page sizes? One of the foremost use-cases for our 52-bit VA/PA support is to enable mapping large quantities of persistent memory, so we really do need this for 64K pages too. FWIW, it appears not to be an issue for PowerPC. > > > On !AR64_4K_PAGES vmemmap_populate() calls vmemmap_populate_basepages() which > does not support struct vmem_altmap right now. Originally was planning to send > the vmemmap_populate_basepages() enablement patches separately but will post it > here for review. > > > > >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> > >> --- > >> arch/arm64/Kconfig | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index db3e625..b5d8cf5 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -31,6 +31,7 @@ config ARM64 > >> select ARCH_HAS_SYSCALL_WRAPPER > >> select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT > >> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > >> + select ARCH_HAS_ZONE_DEVICE if ARM64_4K_PAGES > > > > IIRC certain configurations (HMM?) don't even build if you just turn this on alone (although of course things may have changed elsewhere in the meantime) - crucially, though, from previous discussions[1] it seems fundamentally unsafe, since I don't think we can guarantee that nobody will touch the corners of ZONE_DEVICE that also require pte_devmap in order not to go subtly wrong. I did get as far as cooking up some patches to sort that out [2][3] which I never got round to posting for their own sake, so please consider picking those up as part of this series. > > In the previous discussion mentioned here [1] it sort of indicates that we > cannot have a viable (ARCH_HAS_ZONE_DEVICE=y but !__HAVE_ARCH_PTE_DEVMAP). I > dont understand why ! Because ZONE_DEVICE was specifically invented to solve get_user_pages() for DAX. > The driver can just hotplug the range into ZONE_DEVICE, > manage the memory itself without mapping them to user page table ever. Then why do you even need 'struct page' objects? > IIUC > ZONE_DEVICE must not need user mapped device PFN support. No, you don't understand correctly, or I don't understand how you plan to use ZONE_DEVICE outside it's intended use case. > All the corner case > problems discussed previously come in once these new 'device PFN' memory which > is now in ZONE_DEVICE get mapped into user page table. > > > > > Robin. > > > >> select ARCH_HAVE_NMI_SAFE_CMPXCHG > >> select ARCH_INLINE_READ_LOCK if !PREEMPT > >> select ARCH_INLINE_READ_LOCK_BH if !PREEMPT > >> > > > > > > [1] https://lore.kernel.org/linux-mm/CAA9_cmfA9GS+1M1aSyv1ty5jKY3iho3CERhnRAruWJW3PfmpgA@xxxxxxxxxxxxxx/#t > > [2] http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=61816b833afdb56b49c2e58f5289ae18809e5d67 > > [3] http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=a5a16560eb1becf9a1d4cc0d03d6b5e76da4f4e1 > > (apologies to anyone if the linux-arm.org server is being flaky as usual and requires a few tries to respond properly) > > I have not evaluated pte_devmap(). Will consider [3] when enabling it. But > I still dont understand why ZONE_DEVICE can not be enabled and used from a > driver which never requires user mapping or pte|pmd|pud_devmap() support. Because there are mm paths that make assumptions about ZONE_DEVICE that your use case might violate.
