| Message ID | 20190812112737.6652-6-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/mmu: Storage key reference and change bit handling | expand |
On Mon, 12 Aug 2019 13:27:36 +0200 David Hildenbrand <david@redhat.com> wrote: > Any access sets the reference bit. In case we have a read-fault, we > should not allow writes to the TLB entry if the change bit was not > already set. > > This is a preparation for proper storage-key reference/change bit handling > in TCG and a fix for KVM whereby read accesses would set the change > bit (old KVM versions without the ioctl to carry out the translation). That would be really old kvm versions, right? So no real need to e.g. cc:stable? > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > target/s390x/mmu_helper.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c > index 227a822e42..ba4b460ac6 100644 > --- a/target/s390x/mmu_helper.c > +++ b/target/s390x/mmu_helper.c > @@ -421,14 +421,28 @@ nodat: > return 0; > } > > - if (*flags & PAGE_READ) { > - key |= SK_R; > - } > - > - if (*flags & PAGE_WRITE) { > + switch (rw) { > + case MMU_DATA_LOAD: > + case MMU_INST_FETCH: > + /* > + * The TLB entry has to remain write-protected on read-faults if > + * the storage key does not indicate a change already. Otherwise > + * we might miss setting the change bit on write accesses. > + */ > + if (!(key & SK_C)) { > + *flags &= ~PAGE_WRITE; > + } > + break; > + case MMU_DATA_STORE: > key |= SK_C; > + break; > + default: > + g_assert_not_reached(); > } > > + /* Any store/fetch sets the reference bit */ > + key |= SK_R; > + > r = skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key); > if (r) { > trace_set_skeys_nonzero(r); I've stared at this for quite some time now and have convinced myself that it looks sane. Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 13.08.19 16:54, Cornelia Huck wrote: > On Mon, 12 Aug 2019 13:27:36 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> Any access sets the reference bit. In case we have a read-fault, we >> should not allow writes to the TLB entry if the change bit was not >> already set. >> >> This is a preparation for proper storage-key reference/change bit handling >> in TCG and a fix for KVM whereby read accesses would set the change >> bit (old KVM versions without the ioctl to carry out the translation). > > That would be really old kvm versions, right? So no real need to e.g. > cc:stable? Yes - nothing a distribution ever supported AFAIK.
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index 227a822e42..ba4b460ac6 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -421,14 +421,28 @@ nodat: return 0; } - if (*flags & PAGE_READ) { - key |= SK_R; - } - - if (*flags & PAGE_WRITE) { + switch (rw) { + case MMU_DATA_LOAD: + case MMU_INST_FETCH: + /* + * The TLB entry has to remain write-protected on read-faults if + * the storage key does not indicate a change already. Otherwise + * we might miss setting the change bit on write accesses. + */ + if (!(key & SK_C)) { + *flags &= ~PAGE_WRITE; + } + break; + case MMU_DATA_STORE: key |= SK_C; + break; + default: + g_assert_not_reached(); } + /* Any store/fetch sets the reference bit */ + key |= SK_R; + r = skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key); if (r) { trace_set_skeys_nonzero(r);
Any access sets the reference bit. In case we have a read-fault, we should not allow writes to the TLB entry if the change bit was not already set. This is a preparation for proper storage-key reference/change bit handling in TCG and a fix for KVM whereby read accesses would set the change bit (old KVM versions without the ioctl to carry out the translation). Signed-off-by: David Hildenbrand <david@redhat.com> --- target/s390x/mmu_helper.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)