arm64: Fix pmap_copy()'s handling of 2MB mappings

[markj]

Summary

When copying mappings from parent to child, we clear the accessed and
dirty bits. This is done for both 4KB and 2MB PTEs. However,
pmap_demote_l2() asserts that writable superpages must be dirty. This
is to avoid races with the MMU setting the dirty bit during promotion
and demotion. pmap_copy() can create clean, writable superpage
mappings, so it violates this assertion.

Modify pmap_copy() to make new 2MB mappings read-only, like we do on
amd64. I am not sure though why we shouldn't simply copy the dirty bit
over to the child.

Fixes: ca2cae0b4dd
Reported by: Jenkins via mhorne

[kib]

Don't we mark the copied mapping as clean to avoid unnecessary writes? Suppose that the source mapping is destroyed, and backing pages are marked dirty and written to the storage. Now, if the copied mapping is destroyed without ever being written to, we would re-dirty and write them again.

[markj]

In D30643#688425, @kib wrote:

Don't we mark the copied mapping as clean to avoid unnecessary writes? Suppose that the source mapping is destroyed, and backing pages are marked dirty and written to the storage. Now, if the copied mapping is destroyed without ever being written to, we would re-dirty and write them again.

When writing the dirty pages, won't we downgrade all writable mappings to RO and clear dirty bits from the copied mapping as well?

[kib]

In D30643#688428, @markj wrote:

In D30643#688425, @kib wrote:

Don't we mark the copied mapping as clean to avoid unnecessary writes? Suppose that the source mapping is destroyed, and backing pages are marked dirty and written to the storage. Now, if the copied mapping is destroyed without ever being written to, we would re-dirty and write them again.

When writing the dirty pages, won't we downgrade all writable mappings to RO and clear dirty bits from the copied mapping as well?

But then system would need to demote and invalidate TLB. I still think that not marking mapping as dirty is the right thing to do, when we know that there were no write.

[alc]

You wrote, "Modify pmap_copy() to make new 2MB mappings read-only, like we do on amd64. I am not sure though why we shouldn't simply copy the dirty bit over to the child."

I'm confused. :-) I'm looking at the amd64 source code right now, and for 2MB page mappings it copies PG_A and PG_M. Moreover, it copies PG_RW. However, for 4KB page mappings, it does clear PG_A and PG_M. In other words, we don't handle different page sizes uniformly.

[markj]

In D30643#688707, @alc wrote:

You wrote, "Modify pmap_copy() to make new 2MB mappings read-only, like we do on amd64. I am not sure though why we shouldn't simply copy the dirty bit over to the child."

I'm confused. :-) I'm looking at the amd64 source code right now, and for 2MB page mappings it copies PG_A and PG_M. Moreover, it copies PG_RW. However, for 4KB page mappings, it does clear PG_A and PG_M. In other words, we don't handle different page sizes uniformly.

I see, I switched them around in my head when reading the code. The amd64 behaviour makes more sense to me now.

[markj]

Preserve AF, DBM and RW flags when copying 2MB pages.

[kib]

I still think it is better to clear PG_A|PG_M|PG_RW on copy of 2M pages for amd64. Ideally we would only clear PG_A|PG_M, but this breaks known invariant.

[alc]

In D30643#688425, @kib wrote:

Don't we mark the copied mapping as clean to avoid unnecessary writes? Suppose that the source mapping is destroyed, and backing pages are marked dirty and written to the storage. Now, if the copied mapping is destroyed without ever being written to, we would re-dirty and write them again.

No. To write the page(s) out, we would first perform a pmap_remove_write(), which clears PG_M from all of the mappings, including the copied ones.