Mathieu - schreef op 2016-05-28 13:05:
Martin Natano wrote:
The diff reads fine to me, however it is incomplete. There are some
callers of process_domem() in arch/. They will need to be changed too.
req seems to be in sync with uio_rw in all the cases, so just removing
the last argument should do it.
Thanks for the feedback. The missing callers where an overlook on my
part, sorry for that.
Here is a regenerated diff including all the call site. As a side note,
obviously every one of them was using PT_WRITE_I, that's why it went
unnoticed.
The thing you guys are missing is that on some architectures making
changes to instructions (PT_WRITE_I) requires some additional operations
to guarantee that the CPU actually sees those updated instructions.
Typically this is the case on architectures with separate data and
instruction caches, where the instruction cache doesn't snoop the data
cache. On such architectures (powerpc and arm are examples) you need to
flush the data cache and invalidate the instruction cache. That may be
a somewhat expensive operation.
As you probably guessed, pmap_proc_iflush() is the function that takes
care of this. Since you still call pmap_proc_iflush(), the diff isn't
wrong from a correctness point of view, but I think we should keep the
optimization of not calling pmap_proc_iflush() for PT_WRITE_D.
As for the original issue. Adding UVM_IO_FIXPROT for PT_WRITE_D as
well, means that it will now be able to make changes to read-only data.
That is probably corrrect.
So I think the only thing that should be changed is the following bit:
@@ -734,11 +724,11 @@ process_domem(struct proc *curp, struct proc *p,
struct uio *uio, int req)
vm->vm_refcnt++;
error = uvm_io(&vm->vm_map, uio,
- (req == PT_WRITE_I) ? UVM_IO_FIXPROT : 0);
+ (uio->uio_rw == UIO_WRITE) ? UVM_IO_FIXPROT : 0);
Is that indeed enough to fix the original problem?
