linux-kernel - Re: [PATCH v3 0/3] KVM: s390: Some gaccess cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c6896c31-b9db-e241-5f47-fc96fd53a2cb@linux.ibm.com>
Date:   Tue, 7 Dec 2021 10:27:08 +0100
From:   Janosch Frank <frankja@...ux.ibm.com>
To:     Janis Schoetterl-Glausch <scgl@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>
Cc:     David Hildenbrand <david@...hat.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        kvm@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 0/3] KVM: s390: Some gaccess cleanup

On 11/26/21 17:45, Janis Schoetterl-Glausch wrote:
> Cleanup s390 guest access code a bit, getting rid of some code
> duplication and improving readability.
> 
> v2 -> v3
> 	minor changes only
> 		typo fixes
> 		whitespace
> 		line reordering
> 		picked up Reviewed-by's
> 
> v1 -> v2
> 	separate patch for renamed variable
> 		fragment_len instead of seg
> 	expand comment of guest_range_to_gpas
> 	fix nits

Thanks, picked

> 
> Janis Schoetterl-Glausch (3):
>    KVM: s390: gaccess: Refactor gpa and length calculation
>    KVM: s390: gaccess: Refactor access address range check
>    KVM: s390: gaccess: Cleanup access to guest pages
> 
>   arch/s390/kvm/gaccess.c | 158 +++++++++++++++++++++++-----------------
>   1 file changed, 92 insertions(+), 66 deletions(-)
> 
> Range-diff against v2:
> 1:  60d050210198 ! 1:  e5d7d2d7a4da KVM: s390: gaccess: Refactor gpa and length calculation
>      @@ Metadata
>        ## Commit message ##
>           KVM: s390: gaccess: Refactor gpa and length calculation
>       
>      -    Improve readability be renaming the length variable and
>      +    Improve readability by renaming the length variable and
>           not calculating the offset manually.
>       
>           Signed-off-by: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
>      +    Reviewed-by: Janosch Frank <frankja@...ux.ibm.com>
>       
>        ## arch/s390/kvm/gaccess.c ##
>       @@ arch/s390/kvm/gaccess.c: int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
>      @@ arch/s390/kvm/gaccess.c: int access_guest(struct kvm_vcpu *vcpu, unsigned long g
>        	psw_t *psw = &vcpu->arch.sie_block->gpsw;
>       -	unsigned long _len, nr_pages, gpa, idx;
>       +	unsigned long nr_pages, gpa, idx;
>      -+	unsigned int fragment_len;
>        	unsigned long pages_array[2];
>      ++	unsigned int fragment_len;
>        	unsigned long *pages;
>        	int need_ipte_lock;
>      + 	union asce asce;
>       @@ arch/s390/kvm/gaccess.c: int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, void *data,
>        		ipte_lock(vcpu);
>        	rc = guest_page_range(vcpu, ga, ar, pages, nr_pages, asce, mode);
> 2:  7080846c8c07 ! 2:  91cadb42cbbc KVM: s390: gaccess: Refactor access address range check
>      @@ Commit message
>           range.
>       
>           Signed-off-by: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
>      +    Reviewed-by: Janosch Frank <frankja@...ux.ibm.com>
>       
>        ## arch/s390/kvm/gaccess.c ##
>       @@ arch/s390/kvm/gaccess.c: static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
>      @@ arch/s390/kvm/gaccess.c: static int low_address_protection_enabled(struct kvm_vc
>       + * a correct exception into the guest.
>       + * The resulting gpas are stored into @gpas, unless it is NULL.
>       + *
>      -+ * Note: All gpas except the first one start at the beginning of a page.
>      ++ * Note: All fragments except the first one start at the beginning of a page.
>       + *       When deriving the boundaries of a fragment from a gpa, all but the last
>       + *       fragment end at the end of the page.
>       + *
>      @@ arch/s390/kvm/gaccess.c: int access_guest(struct kvm_vcpu *vcpu, unsigned long g
>        {
>        	psw_t *psw = &vcpu->arch.sie_block->gpsw;
>       -	unsigned long nr_pages, gpa, idx;
>      +-	unsigned long pages_array[2];
>       +	unsigned long nr_pages, idx;
>      ++	unsigned long gpa_array[2];
>        	unsigned int fragment_len;
>      --	unsigned long pages_array[2];
>       -	unsigned long *pages;
>      -+	unsigned long gpa_array[2];
>       +	unsigned long *gpas;
>        	int need_ipte_lock;
>        	union asce asce;
> 3:  c991cbdbfbd5 ! 3:  f5000a22efcd KVM: s390: gaccess: Cleanup access to guest frames
>      @@ Metadata
>       Author: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
>       
>        ## Commit message ##
>      -    KVM: s390: gaccess: Cleanup access to guest frames
>      +    KVM: s390: gaccess: Cleanup access to guest pages
>       
>           Introduce a helper function for guest frame access.
>       
>           Signed-off-by: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
>      +    Reviewed-by: Janosch Frank <frankja@...ux.ibm.com>
>       
>        ## arch/s390/kvm/gaccess.c ##
>       @@ arch/s390/kvm/gaccess.c: static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar,
>      @@ arch/s390/kvm/gaccess.c: static int guest_range_to_gpas(struct kvm_vcpu *vcpu, u
>        }
>        
>       +static int access_guest_page(struct kvm *kvm, enum gacc_mode mode, gpa_t gpa,
>      -+			      void *data, unsigned int len)
>      ++			     void *data, unsigned int len)
>       +{
>       +	const unsigned int offset = offset_in_page(gpa);
>       +	const gfn_t gfn = gpa_to_gfn(gpa);
> 
> base-commit: d25f27432f80a800a3592db128254c8140bd71bf
>