On Mon, 12 Aug 2019 13:27:36 +0200
David Hildenbrand <da...@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 <da...@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 <coh...@redhat.com>
