heh update email address

do we need bus_* barriers anywhere? this may be mostly fine on x86 but eg arm/riscv it may need explicit barriers?

so are all of these devices fine with 64 bit DMA?

This means the consumer needs to do two mmaps, right? One for data and one for control/registers, so the correct memory attributes are used?

I think this function in particular deserves a doxygen style comment on what it does and entry/exit values.

I think all of the methods here deserve documenting what they do, even if it is brief.

It tested in amd64 only . But the framework should work with any architecture .

Why aren't they charged against those limits? The backing pages are effectively mlocked.

Why do you do the validation separately from handling the actual ioctls? It is harder to read that way.

This loop runs exactly once, I don't think that's the intent.

As I said below I think this code shouldn't be messing with the vm_map at all.

This should be a list of allowed object types (or better, some characterization of the object properties that we want), not a list that needs to be updated each time a new object type is added.

Suppose a process maps some of its pages into device page tables. Then it forks, and the host mapping becomes read-only in order to implement COW semantics. Now the device has a writable mapping to read-only memory. What should happen in this case?

You already faulted the pages above, why look them up again? The last parameter to vm_fault() can be used to return the faulted page. Or use vm_fault_quick_hold_pages(), which is probably better here. That way, gasket_classify_range_locked() doesn't need to deal with vm_map entries at all. You can just look at the returned pages to see which type of VM object you're dealing with.

What does this mean? Why is it sufficient to check for only those two flags?

Isn't this handled by vm_fault()?

This is weird on its own.

But what is true is that you cannot really track opens vs. close with cdevsw methods. D_TRACKCLOSE would not help.

Yes why vm_fault_quick_hold() cannot be applied there?

This looks incomplete?

Suppose a writer opens this device. Then it can pass the resulting fd to another process, e.g., via fork() or a SCM_RIGHTS message, and you end up with multiple processes that have write access, so gs_owner isn't well-defined. And the owner can exit, in which case you're left with a dangling pointer here.

Is there some desire to support sharing the device among multiple processes? If not, then you should probably set D_NONPASSABLE in the cdevsw flags. If so, then we should maintain per-fd state using devfs_set_cdevpriv(). That is probably cleaner than trying to count opens, keep track of an owner, etc..

Is there some specific reason this needs to be 0660 instead of 0600? If not I'd prefer to use the latter.

Why do you need to busy the page? vm_page_dirty() requires the VM object lock.

This comment looks stale, the cdev implements d_mmap_single, not d_mmap.

Does gd_coherent_base get set anywhere?

It took me a long time to understand what this is actually saying. I think it goes like this: vm_fault_quick_hold_pages() doesn't work as you want when applied to the coherent region: if the pages are mapped into user PTs, then pmap_extract_and_hold() will return the real vm_page structure, which belongs to kernel_object, since PHYS_TO_VM_PAGE() checks the vm_page_array first. But if we end up going into the fault handler, then we'll get back fake pages.[1] So you are working around this inconsistency by looking at the vm_map. But that's racy.

Can't you instead look at the physical address of the pages returned by vm_fault_quick_hold_pages() to know whether they belong to the coherent region?

[1] This is surprising and I bet it causes problems elsewhere. Possibly pmap_extract_and_hold() should be returning fake pages here...

I traced how this prot is used elsewhere. It looks like the device mappings of these pages just ignore these protection bit, so how do you ensure that the device can't write to memory that's supposed to be read-only?