When pinning guest memory using pin_blocks(), gfn_to_page() was used, which under the hood uses FOLL_GET, which is the wrong type of flag for memory that is accessed directly by hardware. Fix by using kvm_vcpu_{,un}map(), which uses FOLL_PIN and is the correct thing to do. While technically a bugfix, the previous behaviour can only lead to guest corruption in extremely convoluted test scenarios that cannot happen in real/production systems. Signed-off-by: Claudio Imbrenda --- arch/s390/kvm/s390/vsie.c | 132 ++++++++++++++------------------------ 1 file changed, 48 insertions(+), 84 deletions(-) diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c index 3b5c322bf8b4..d2f788bfc956 100644 --- a/arch/s390/kvm/s390/vsie.c +++ b/arch/s390/kvm/s390/vsie.c @@ -49,20 +49,21 @@ struct vsie_page { /* address of the last reported fault to guest2 */ unsigned long fault_addr; /* 0x0228 */ /* calculated guest addresses of satellite control blocks */ - gpa_t sca_gpa; /* 0x0230 */ - gpa_t itdba_gpa; /* 0x0238 */ - gpa_t gvrd_gpa; /* 0x0240 */ - gpa_t riccbd_gpa; /* 0x0248 */ - gpa_t sdnx_gpa; /* 0x0250 */ + struct kvm_host_map sca_map; /* 0x0230 */ + struct kvm_host_map itdba_map; /* 0x0260 */ + struct kvm_host_map gvrd_map; /* 0x0290 */ + struct kvm_host_map riccbd_map; /* 0x02c0 */ + struct kvm_host_map sdnx_map; /* 0x02f0 */ + struct kvm_host_map scb_map; /* 0x0320 */ /* * guest address of the original SCB. Remains set for free vsie * pages, so we can properly look them up in our addr_to_page * radix tree. */ - gpa_t scb_gpa; /* 0x0258 */ + gpa_t scb_gpa; /* 0x0350 */ /* the shadow gmap in use by the vsie_page */ - struct gmap_cache gmap_cache; /* 0x0260 */ - __u8 reserved[0x06f8 - 0x0278]; /* 0x0278 */ + struct gmap_cache gmap_cache; /* 0x0358 */ + __u8 reserved[0x06f8 - 0x0370]; /* 0x0370 */ struct kvm_s390_crypto_cb crycb; /* 0x06f8 */ __u8 fac[8 + S390_ARCH_FAC_LIST_SIZE_BYTE];/* 0x07f8 */ }; @@ -681,72 +682,45 @@ static int map_prefix(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, struct } /* - * Pin the guest page given by gpa and set hpa to the pinned host address. + * pin_guest_page() - Pin the given guest page and return its hpa. + * @vcpu: the vCPU + * @gpa: the gpa to pin + * @map: the map representing this pinning. + * * Will always be pinned writable. * - * Returns: - 0 on success - * - -EINVAL if the gpa is not valid guest storage + * Returns: - 0 if the gpa is not valid guest storage, + * - The host hpa of the pinned page in case of success */ -static int pin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t *hpa) +static hpa_t pin_guest_page(struct kvm_vcpu *vcpu, gpa_t gpa, struct kvm_host_map *map) { - struct page *page; - - page = gfn_to_page(kvm, gpa_to_gfn(gpa)); - if (!page) - return -EINVAL; - *hpa = (hpa_t)page_to_phys(page) + (gpa & ~PAGE_MASK); - return 0; -} - -/* Unpins a page previously pinned via pin_guest_page, marking it as dirty. */ -static void unpin_guest_page(struct kvm *kvm, gpa_t gpa, hpa_t hpa) -{ - kvm_release_page_dirty(pfn_to_page(phys_to_pfn(hpa))); - /* mark the page always as dirty for migration */ - mark_page_dirty(kvm, gpa_to_gfn(gpa)); + if (kvm_vcpu_map(vcpu, gpa_to_gfn(gpa), map)) + return 0; + if (!kvm_vcpu_mapped(map)) + return 0; + return (hpa_t)PFN_PHYS(map->pfn) + (gpa & ~PAGE_MASK); } /* unpin all blocks previously pinned by pin_blocks(), marking them dirty */ static void unpin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) { struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; - hpa_t hpa; - hpa = (u64) scb_s->scaoh << 32 | scb_s->scaol; - if (hpa) { - unpin_guest_page(vcpu->kvm, vsie_page->sca_gpa, hpa); - vsie_page->sca_gpa = 0; - scb_s->scaol = 0; - scb_s->scaoh = 0; - } + kvm_vcpu_unmap(vcpu, &vsie_page->sca_map); + scb_s->scaol = 0; + scb_s->scaoh = 0; - hpa = scb_s->itdba; - if (hpa) { - unpin_guest_page(vcpu->kvm, vsie_page->itdba_gpa, hpa); - vsie_page->itdba_gpa = 0; - scb_s->itdba = 0; - } + kvm_vcpu_unmap(vcpu, &vsie_page->itdba_map); + scb_s->itdba = 0; - hpa = scb_s->gvrd; - if (hpa) { - unpin_guest_page(vcpu->kvm, vsie_page->gvrd_gpa, hpa); - vsie_page->gvrd_gpa = 0; - scb_s->gvrd = 0; - } + kvm_vcpu_unmap(vcpu, &vsie_page->gvrd_map); + scb_s->gvrd = 0; - hpa = scb_s->riccbd; - if (hpa) { - unpin_guest_page(vcpu->kvm, vsie_page->riccbd_gpa, hpa); - vsie_page->riccbd_gpa = 0; - scb_s->riccbd = 0; - } + kvm_vcpu_unmap(vcpu, &vsie_page->riccbd_map); + scb_s->riccbd = 0; - hpa = scb_s->sdnxo; - if (hpa) { - unpin_guest_page(vcpu->kvm, vsie_page->sdnx_gpa, hpa); - vsie_page->sdnx_gpa = 0; - scb_s->sdnxo = 0; - } + kvm_vcpu_unmap(vcpu, &vsie_page->sdnx_map); + scb_s->sdnxo = 0; } /* @@ -783,13 +757,12 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) ((gpa + offsetof(struct bsca_block, cpu[0]) - 1) & PAGE_MASK)) rc = set_validity_icpt(scb_s, 0x003bU); if (!rc) { - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) + hpa = pin_guest_page(vcpu, gpa, &vsie_page->sca_map); + if (!hpa) rc = set_validity_icpt(scb_s, 0x0034U); } - if (rc) + if (rc || !hpa) goto unpin; - vsie_page->sca_gpa = gpa; scb_s->scaoh = (u32)((u64)hpa >> 32); scb_s->scaol = (u32)(u64)hpa; } @@ -801,12 +774,11 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) goto unpin; } /* 256 bytes cannot cross page boundaries */ - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) { + hpa = pin_guest_page(vcpu, gpa, &vsie_page->itdba_map); + if (!hpa) { rc = set_validity_icpt(scb_s, 0x0080U); goto unpin; } - vsie_page->itdba_gpa = gpa; scb_s->itdba = hpa; } @@ -820,12 +792,11 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) * 512 bytes vector registers cannot cross page boundaries * if this block gets bigger, we have to shadow it. */ - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) { + hpa = pin_guest_page(vcpu, gpa, &vsie_page->gvrd_map); + if (!hpa) { rc = set_validity_icpt(scb_s, 0x1310U); goto unpin; } - vsie_page->gvrd_gpa = gpa; scb_s->gvrd = hpa; } @@ -836,13 +807,12 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) goto unpin; } /* 64 bytes cannot cross page boundaries */ - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) { + hpa = pin_guest_page(vcpu, gpa, &vsie_page->riccbd_map); + if (!hpa) { rc = set_validity_icpt(scb_s, 0x0043U); goto unpin; } /* Validity 0x0044 will be checked by SIE */ - vsie_page->riccbd_gpa = gpa; scb_s->riccbd = hpa; } if (((scb_s->ecb & ECB_GS) && !(scb_s->ecd & ECD_HOSTREGMGMT)) || @@ -866,12 +836,11 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) /* Due to alignment rules (checked above) this cannot * cross page boundaries */ - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) { + hpa = pin_guest_page(vcpu, gpa, &vsie_page->sdnx_map); + if (!hpa) { rc = set_validity_icpt(scb_s, 0x10b0U); goto unpin; } - vsie_page->sdnx_gpa = gpa; scb_s->sdnxo = hpa | sdnxc; } return 0; @@ -884,10 +853,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, gpa_t gpa) { - hpa_t hpa = virt_to_phys(vsie_page->scb_o); - - if (hpa) - unpin_guest_page(vcpu->kvm, gpa, hpa); + kvm_vcpu_unmap(vcpu, &vsie_page->scb_map); vsie_page->scb_o = NULL; } @@ -901,12 +867,10 @@ static int pin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, gpa_t gpa) { hpa_t hpa; - int rc; - rc = pin_guest_page(vcpu->kvm, gpa, &hpa); - if (rc) { - rc = kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING); - WARN_ON_ONCE(rc); + hpa = pin_guest_page(vcpu, gpa, &vsie_page->scb_map); + if (!hpa) { + WARN_ON_ONCE(kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING)); return 1; } vsie_page->scb_o = phys_to_virt(hpa); -- 2.56.0