On Tue, Nov 26, 2024 at 10:30 PM Jann Horn <jannh@xxxxxxxxxx> wrote: > > On Fri, Nov 22, 2024 at 4:41 PM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote: > > This type will be used when setting up a new vma in an f_ops->mmap() > > hook. Using a separate type from VmAreaRef allows us to have a separate > > set of operations that you are only able to use during the mmap() hook. > > For example, the VM_MIXEDMAP flag must not be changed after the initial > > setup that happens during the f_ops->mmap() hook. > > > > To avoid setting invalid flag values, the methods for clearing > > VM_MAYWRITE and similar involve a check of VM_WRITE, and return an error > > if VM_WRITE is set. Trying to use `try_clear_maywrite` without checking > > the return value results in a compilation error because the `Result` > > type is marked #[must_use]. > > > > For now, there's only a method for VM_MIXEDMAP and not VM_PFNMAP. When > > we add a VM_PFNMAP method, we will need some way to prevent you from > > setting both VM_MIXEDMAP and VM_PFNMAP on the same vma. > > > > Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> > > Thanks, this looks really neat! > > Reviewed-by: Jann Horn <jannh@xxxxxxxxxx> > > > + /// Set the `VM_IO` flag on this vma. > > + /// > > + /// This marks the vma as being a memory-mapped I/O region. > > nit: VM_IO isn't really exclusively used for MMIO; the header comment > says "Memory mapped I/O or similar", while the comment in > remap_pfn_range_internal() says "VM_IO tells people not to look at > these pages (accesses can have side effects)". But I don't really have > a good definition of what VM_IO actually means; so I don't really have > a concrete suggestion for what do do here. So my comment isn't very > actionable, I guess it's fine to leave this as-is unless someone > actually has a good definition... I can use this comment? This is used for memory mapped IO and similar. The flag tells other parts of the kernel to not look at the pages. For memory mapped IO this is useful as accesses to the pages could have side effects. Alice
