On 2/9/21 4:43 PM, Claudio Imbrenda wrote:
> Extend kvm_s390_shadow_fault to return the pointer to the valid leaf
> DAT table entry, or to the invalid entry.
> 
> Also return some flags in the lower bits of the address:
> DAT_PROT: indicates that DAT protection applies because of the
>           protection bit in the segment (or, if EDAT, region) tables
> NOT_PTE: indicates that the address of the DAT table entry returned
>          does not refer to a PTE, but to a segment or region table.
> 
> Signed-off-by: Claudio Imbrenda <imbre...@linux.ibm.com>
> Cc: sta...@vger.kernel.org

Reviewed-by: Janosch Frank <fran...@de.ibm.com>

Small nit below.

> ---
>  arch/s390/kvm/gaccess.c | 30 +++++++++++++++++++++++++-----
>  arch/s390/kvm/gaccess.h |  5 ++++-
>  arch/s390/kvm/vsie.c    |  8 ++++----
>  3 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 6d6b57059493..e0ab83f051d2 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -976,7 +976,9 @@ int kvm_s390_check_low_addr_prot_real(struct kvm_vcpu 
> *vcpu, unsigned long gra)
>   * kvm_s390_shadow_tables - walk the guest page table and create shadow 
> tables
>   * @sg: pointer to the shadow guest address space structure
>   * @saddr: faulting address in the shadow gmap
> - * @pgt: pointer to the page table address result
> + * @pgt: pointer to the beginning of the page table for the given address if
> + *       successful (return value 0), or to the first invalid DAT entry in
> + *       case of exceptions (return value > 0)
>   * @fake: pgt references contiguous guest memory block, not a pgtable
>   */
>  static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
> @@ -1034,6 +1036,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>                       rfte.val = ptr;
>                       goto shadow_r2t;
>               }
> +             *pgt = ptr + vaddr.rfx * 8;
>               rc = gmap_read_table(parent, ptr + vaddr.rfx * 8, &rfte.val);
>               if (rc)
>                       return rc;
> @@ -1060,6 +1063,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>                       rste.val = ptr;
>                       goto shadow_r3t;
>               }
> +             *pgt = ptr + vaddr.rsx * 8;
>               rc = gmap_read_table(parent, ptr + vaddr.rsx * 8, &rste.val);
>               if (rc)
>                       return rc;
> @@ -1087,6 +1091,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>                       rtte.val = ptr;
>                       goto shadow_sgt;
>               }
> +             *pgt = ptr + vaddr.rtx * 8;
>               rc = gmap_read_table(parent, ptr + vaddr.rtx * 8, &rtte.val);
>               if (rc)
>                       return rc;
> @@ -1123,6 +1128,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>                       ste.val = ptr;
>                       goto shadow_pgt;
>               }
> +             *pgt = ptr + vaddr.sx * 8;
>               rc = gmap_read_table(parent, ptr + vaddr.sx * 8, &ste.val);
>               if (rc)
>                       return rc;
> @@ -1157,6 +1163,8 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>   * @vcpu: virtual cpu
>   * @sg: pointer to the shadow guest address space structure
>   * @saddr: faulting address in the shadow gmap
> + * @datptr: will contain the address of the faulting DAT table entry, or of
> + *          the valid leaf, plus some flags
>   *
>   * Returns: - 0 if the shadow fault was successfully resolved
>   *       - > 0 (pgm exception code) on exceptions while faulting
> @@ -1165,11 +1173,11 @@ static int kvm_s390_shadow_tables(struct gmap *sg, 
> unsigned long saddr,
>   *       - -ENOMEM if out of memory
>   */
>  int kvm_s390_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg,
> -                       unsigned long saddr)
> +                       unsigned long saddr, unsigned long *datptr)
>  {
>       union vaddress vaddr;
>       union page_table_entry pte;
> -     unsigned long pgt;
> +     unsigned long pgt = 0;
>       int dat_protection, fake;
>       int rc;
>  
> @@ -1191,8 +1199,20 @@ int kvm_s390_shadow_fault(struct kvm_vcpu *vcpu, 
> struct gmap *sg,
>               pte.val = pgt + vaddr.px * PAGE_SIZE;
>               goto shadow_page;
>       }
> -     if (!rc)
> -             rc = gmap_read_table(sg->parent, pgt + vaddr.px * 8, &pte.val);
> +
> +     switch (rc) {
> +     case PGM_SEGMENT_TRANSLATION:
> +     case PGM_REGION_THIRD_TRANS:
> +     case PGM_REGION_SECOND_TRANS:
> +     case PGM_REGION_FIRST_TRANS:
> +             pgt |= NOT_PTE;
> +             break;
> +     case 0:
> +             pgt += vaddr.px * 8;
> +             rc = gmap_read_table(sg->parent, pgt, &pte.val);
> +     }
> +     if (*datptr)
> +             *datptr = pgt | dat_protection * DAT_PROT;
>       if (!rc && pte.i)
>               rc = PGM_PAGE_TRANSLATION;
>       if (!rc && pte.z)
> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
> index f4c51756c462..fec26bbb17ba 100644
> --- a/arch/s390/kvm/gaccess.h
> +++ b/arch/s390/kvm/gaccess.h
> @@ -359,7 +359,10 @@ void ipte_unlock(struct kvm_vcpu *vcpu);
>  int ipte_lock_held(struct kvm_vcpu *vcpu);
>  int kvm_s390_check_low_addr_prot_real(struct kvm_vcpu *vcpu, unsigned long 
> gra);
>  
> +#define DAT_PROT 2
> +#define NOT_PTE 4

I'd like to have a PEI prefix and a short comment where this comes from,
something like:
"MVPG PEI indication bits"

> +
>  int kvm_s390_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *shadow,
> -                       unsigned long saddr);
> +                       unsigned long saddr, unsigned long *datptr);
>  
>  #endif /* __KVM_S390_GACCESS_H */
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index c5d0a58b2c29..7db022141db3 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -619,10 +619,10 @@ static int map_prefix(struct kvm_vcpu *vcpu, struct 
> vsie_page *vsie_page)
>       /* with mso/msl, the prefix lies at offset *mso* */
>       prefix += scb_s->mso;
>  
> -     rc = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, prefix);
> +     rc = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, prefix, NULL);
>       if (!rc && (scb_s->ecb & ECB_TE))
>               rc = kvm_s390_shadow_fault(vcpu, vsie_page->gmap,
> -                                        prefix + PAGE_SIZE);
> +                                        prefix + PAGE_SIZE, NULL);
>       /*
>        * We don't have to mprotect, we will be called for all unshadows.
>        * SIE will detect if protection applies and trigger a validity.
> @@ -913,7 +913,7 @@ static int handle_fault(struct kvm_vcpu *vcpu, struct 
> vsie_page *vsie_page)
>                                   current->thread.gmap_addr, 1);
>  
>       rc = kvm_s390_shadow_fault(vcpu, vsie_page->gmap,
> -                                current->thread.gmap_addr);
> +                                current->thread.gmap_addr, NULL);
>       if (rc > 0) {
>               rc = inject_fault(vcpu, rc,
>                                 current->thread.gmap_addr,
> @@ -935,7 +935,7 @@ static void handle_last_fault(struct kvm_vcpu *vcpu,
>  {
>       if (vsie_page->fault_addr)
>               kvm_s390_shadow_fault(vcpu, vsie_page->gmap,
> -                                   vsie_page->fault_addr);
> +                                   vsie_page->fault_addr, NULL);
>       vsie_page->fault_addr = 0;
>  }
>  
> 

Reply via email to