On Thu, Aug 14, 2025 at 09:44:48AM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 14, 2025 at 03:35:06PM +0300, Leon Romanovsky wrote:
> > > Then check attrs here, not pfn_valid.
> >
> > attrs are not available in kmsan_handle_dma(). I can add it if you prefer.
>
> That makes more sense to the overall design. The comments I gave
> before were driving at a promise to never try to touch a struct page
> for ATTR_MMIO and think this should be comphrensive to never touching
> a struct page even if pfnvalid.
>
> > > > So let's keep this patch as is.
> > >
> > > Still need to fix the remarks you clipped, do not check PageHighMem
> > > just call kmap_local_pfn(). All thie PageHighMem stuff is new to this
> > > patch and should not be here, it is the wrong way to use highmem.
> >
> > Sure, thanks
>
> I am wondering if there is some reason it was written like this in the
> first place. Maybe we can't even do kmap here.. So perhaps if there is
> not a strong reason to change it just continue to check pagehighmem
> and fail.
>
> if (!(attrs & ATTR_MMIO) && PageHighMem(phys_to_page(phys)))
> return;
Does this version good enough? There is no need to call to
kmap_local_pfn() if we prevent PageHighMem pages.
diff --git a/mm/kmsan/hooks.c b/mm/kmsan/hooks.c
index eab7912a3bf0..d9cf70f4159c 100644
--- a/mm/kmsan/hooks.c
+++ b/mm/kmsan/hooks.c
@@ -337,13 +337,13 @@ static void kmsan_handle_dma_page(const void *addr,
size_t size,
/* Helper function to handle DMA data transfers. */
void kmsan_handle_dma(phys_addr_t phys, size_t size,
- enum dma_data_direction dir)
+ enum dma_data_direction dir, unsigned long attrs)
{
u64 page_offset, to_go, addr;
struct page *page;
void *kaddr;
- if (!pfn_valid(PHYS_PFN(phys)))
+ if ((attrs & ATTR_MMIO) || PageHighMem(phys_to_page(phys)))
return;
page = phys_to_page(phys);
@@ -357,19 +357,12 @@ void kmsan_handle_dma(phys_addr_t phys, size_t size,
while (size > 0) {
to_go = min(PAGE_SIZE - page_offset, (u64)size);
- if (PageHighMem(page))
- /* Handle highmem pages using kmap */
- kaddr = kmap_local_page(page);
- else
- /* Lowmem pages can be accessed directly */
- kaddr = page_address(page);
+ /* Lowmem pages can be accessed directly */
+ kaddr = page_address(page);
addr = (u64)kaddr + page_offset;
kmsan_handle_dma_page((void *)addr, to_go, dir);
- if (PageHighMem(page))
- kunmap_local(page);
-
phys += to_go;
size -= to_go;
(END)
>
> Jason
>
