On 3/20/26 17:15, Claudio Imbrenda wrote:
> When shadowing, the guest page tables are write-protected, in order to
> trap changes and properly unshadow the shadow mapping for the nested
> guest. Already shadowed levels are skipped, so that only the needed
> levels are write protected.
>
> Currently the levels that get write protected are exactly one level too
> deep: the last level (nested guest memory) gets protected in the wrong
> way, and will be protected again correctly a few lines afterwards; most
> importantly, the highest non-shadowed level does *not* get write
> protected.
>
> Moreover, if the nested guest is running in a real address space, there
> are no DAT tables to shadow.
>
> Write protect the correct levels, so that all the levels that need to
> be protected are protected, and avoid double protecting the last level;
> skip attempting to shadow the DAT tables when the nested guest is
> running in a real address space.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
> ---
> arch/s390/kvm/gaccess.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 1054f9bd107f..17563f889c6b 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -1517,13 +1517,15 @@ static int _gaccess_do_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *sg,
> * Skip levels that are already protected. For each level, protect
> * only the page containing the entry, not the whole table.
> */
> - for (i = gl ; i >= w->level; i--) {
> - rc = gmap_protect_rmap(mc, sg, entries[i - 1].gfn, gpa_to_gfn(saddr),
> - entries[i - 1].pfn, i, entries[i - 1].writable);
Yeah, tt for page tables is -1 and adding -1 ends up being LEVEL_MEM...
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> - if (rc)
> - return rc;
> - if (!sg->parent)
> - return -EAGAIN;
> + if (w->level > LEVEL_MEM) {
> + for (i = gl ; i >= w->level; i--) {
You could have fixed the rogue white space after "gl"
> + rc = gmap_protect_rmap(mc, sg, entries[i].gfn, gpa_to_gfn(saddr),
> + entries[i].pfn, i + 1, entries[i].writable);
> + if (rc)
> + return rc;
> + if (!sg->parent)
> + return -EAGAIN;
> + }
> }
>
> rc = dat_entry_walk(NULL, entries[LEVEL_MEM].gfn, sg->parent->asce, DAT_WALK_LEAF,