Mark Kettenis <mark.kette...@xs4all.nl> writes: > 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.
Since PT_WRITE_I and PT_WRITE_D are documented as strictly equivalent since rev. 1.1, I doubt that such an optimization is a good idea. > 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? -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
