Bharata B Rao <bharata@xxxxxxxxxxxxx> writes:
> On Tue, Jun 22, 2021 at 10:05:45AM +0530, Bharata B Rao wrote:
>> On Mon, Jun 21, 2021 at 10:12:42AM -0700, Nathan Chancellor wrote:
>> > I have not seen this reported yet so apologies if it has and there is a
>> > fix I am missing:
>> >
>> > arch/powerpc/kvm/book3s_hv_nested.c:1334:11: error: variable 'ap' is uninitialized when used here [-Werror,-Wuninitialized]
>> > ap, start, end);
>> > ^~
>> > arch/powerpc/kvm/book3s_hv_nested.c:1276:25: note: initialize the variable 'ap' to silence this warning
>> > unsigned long psize, ap;
>> > ^
>> > = 0
>>
>> Thanks for catching this, this wasn't caught in my environment.
>>
>> I will repost the series with proper initialization to ap.
>
> Michael,
>
> Here is the fix for this on top of powerpc/next. If it is easier
> and cleaner to fold this into the original series and re-post
> the whole series against any updated tree, let me know.
Thanks. I squashed this in.
cheers
> From 2e7198e28c0d1137f3230d4645e9cfddaccf4987 Mon Sep 17 00:00:00 2001
> From: Bharata B Rao <bharata@xxxxxxxxxxxxx>
> Date: Tue, 22 Jun 2021 12:07:01 +0530
> Subject: [PATCH 1/1] KVM: PPC: Book3S HV: Use proper ap value in
> H_RPT_INVALIDATE
>
> The ap value that is used when performing range based partition
> scoped invalidations for the nested guests wasn't initialized
> correctly.
>
> Fix this and while we are here, reorganize the routine that does
> this invalidation for better readability.
>
> Fixes: 0e67d866cb32 ("KVM: PPC: Book3S HV: Nested support in H_RPT_INVALIDATE")
> Signed-off-by: Bharata B Rao <bharata@xxxxxxxxxxxxx>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 90 +++++++++++++----------------
> 1 file changed, 40 insertions(+), 50 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index d78efb5f5bb3..3a06ac0b53e2 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -1222,27 +1222,6 @@ long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu)
> return H_SUCCESS;
> }
>
> -static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
> - unsigned long lpid,
> - unsigned long page_size,
> - unsigned long ap,
> - unsigned long start,
> - unsigned long end)
> -{
> - unsigned long addr = start;
> - int ret;
> -
> - do {
> - ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
> - get_epn(addr));
> - if (ret)
> - return ret;
> - addr += page_size;
> - } while (addr < end);
> -
> - return ret;
> -}
> -
> static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
> unsigned long lpid, unsigned long ric)
> {
> @@ -1263,6 +1242,42 @@ static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
> */
> static unsigned long tlb_range_flush_page_ceiling __read_mostly = 33;
>
> +static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
> + unsigned long lpid,
> + unsigned long pg_sizes,
> + unsigned long start,
> + unsigned long end)
> +{
> + int ret = H_P4;
> + unsigned long addr, nr_pages;
> + struct mmu_psize_def *def;
> + unsigned long psize, ap, page_size;
> + bool flush_lpid;
> +
> + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> + def = &mmu_psize_defs[psize];
> + if (!(pg_sizes & def->h_rpt_pgsize))
> + continue;
> +
> + nr_pages = (end - start) >> def->shift;
> + flush_lpid = nr_pages > tlb_range_flush_page_ceiling;
> + if (flush_lpid)
> + return do_tlb_invalidate_nested_all(vcpu, lpid,
> + RIC_FLUSH_TLB);
> + addr = start;
> + ap = mmu_get_ap(psize);
> + page_size = 1UL << def->shift;
> + do {
> + ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
> + get_epn(addr));
> + if (ret)
> + return H_P4;
> + addr += page_size;
> + } while (addr < end);
> + }
> + return ret;
> +}
> +
> /*
> * Performs partition-scoped invalidations for nested guests
> * as part of H_RPT_INVALIDATE hcall.
> @@ -1271,10 +1286,6 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
> unsigned long type, unsigned long pg_sizes,
> unsigned long start, unsigned long end)
> {
> - struct kvm_nested_guest *gp;
> - long ret;
> - unsigned long psize, ap;
> -
> /*
> * If L2 lpid isn't valid, we need to return H_PARAMETER.
> *
> @@ -1284,8 +1295,7 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
> * H_ENTER_NESTED call. Since we can't differentiate this case from
> * the invalid case, we ignore such flush requests and return success.
> */
> - gp = kvmhv_find_nested(vcpu->kvm, lpid);
> - if (!gp)
> + if (!kvmhv_find_nested(vcpu->kvm, lpid))
> return H_SUCCESS;
>
> /*
> @@ -1313,29 +1323,9 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
> if (start == 0 && end == -1)
> return do_tlb_invalidate_nested_all(vcpu, lpid, RIC_FLUSH_TLB);
>
> - if (type & H_RPTI_TYPE_TLB) {
> - struct mmu_psize_def *def;
> - bool flush_lpid;
> - unsigned long nr_pages;
> -
> - for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> - def = &mmu_psize_defs[psize];
> - if (!(pg_sizes & def->h_rpt_pgsize))
> - continue;
> -
> - nr_pages = (end - start) >> def->shift;
> - flush_lpid = nr_pages > tlb_range_flush_page_ceiling;
> - if (flush_lpid)
> - return do_tlb_invalidate_nested_all(vcpu, lpid,
> - RIC_FLUSH_TLB);
> -
> - ret = do_tlb_invalidate_nested_tlb(vcpu, lpid,
> - (1UL << def->shift),
> - ap, start, end);
> - if (ret)
> - return H_P4;
> - }
> - }
> + if (type & H_RPTI_TYPE_TLB)
> + return do_tlb_invalidate_nested_tlb(vcpu, lpid, pg_sizes,
> + start, end);
> return H_SUCCESS;
> }
>
> --
> 2.31.1
