* [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code @ 2018-02-07 9:04 Paul Mackerras 2018-02-07 9:06 ` [PATCH 2/2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 Paul Mackerras 2018-02-08 23:35 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code David Gibson 0 siblings, 2 replies; 6+ messages in thread From: Paul Mackerras @ 2018-02-07 9:04 UTC (permalink / raw) To: kvm, kvm-ppc; +Cc: David Gibson This fixes the computation of the HPTE index to use when the HPT resizing code encounters a bolted HPTE which is stored in its secondary HPTE group. The code inverts the HPTE group number, which is correct, but doesn't then mask it with new_hash_mask. As a result, new_pteg will be effectively negative, resulting in new_hptep pointing before the new HPT, which will corrupt memory. In addition, this removes two BUG_ON statements. The condition that the BUG_ONs were testing -- that we have computed the hash value incorrectly -- has never been observed in testing, and if it did occur, would only affect the guest, not the host. Given that BUG_ON should only be used in conditions where the kernel (i.e. the host kernel, in this case) can't possibly continue execution, it is not appropriate here. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_64_mmu_hv.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c index 9660972..d196499 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c @@ -1329,12 +1329,8 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, } new_pteg = hash & new_hash_mask; - if (vpte & HPTE_V_SECONDARY) { - BUG_ON(~pteg != (hash & old_hash_mask)); - new_pteg = ~new_pteg; - } else { - BUG_ON(pteg != (hash & old_hash_mask)); - } + if (vpte & HPTE_V_SECONDARY) + new_pteg = ~hash & new_hash_mask; new_idx = new_pteg * HPTES_PER_GROUP + (idx % HPTES_PER_GROUP); new_hptep = (__be64 *)(new->virt + (new_idx << 4)); -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 2018-02-07 9:04 [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code Paul Mackerras @ 2018-02-07 9:06 ` Paul Mackerras 2018-02-07 9:11 ` [PATCH v2] " Paul Mackerras 2018-02-08 23:35 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code David Gibson 1 sibling, 1 reply; 6+ messages in thread From: Paul Mackerras @ 2018-02-07 9:06 UTC (permalink / raw) To: kvm, kvm-ppc; +Cc: David Gibson From: David Gibson <david@gibson.dropbear.id.au> This adds code to enable the HPT resizing code to work on POWER9, which uses a slightly modified HPT entry format compared to POWER8. On POWER9, we convert HPTEs read from the HPT from the new format to the old format so that the rest of the HPT resizing code can work as before. HPTEs written to the new HPT are converted to the new format as the last step before writing them into the new HPT. This takes out the checks added by commit bcd3bb63dbc8 ("KVM: PPC: Book3S HV: Disable HPT resizing on POWER9 for now", 2017-02-18), now that HPT resizing works on POWER9. On POWER9, when we pivot to the new HPT, we now call kvmppc_setup_partition_table() to update the partition table in order to make the hardware use the new HPT. [paulus@ozlabs.org - added kvmppc_setup_partition_table() call, wrote commit message.] Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_64_mmu_hv.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c index d196499..cb34be7 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c @@ -1261,6 +1261,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Nothing to do */ goto out; + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + rpte = be64_to_cpu(hptep[1]); + vpte = hpte_new_to_old_v(vpte, rpte); + } + /* Unmap */ rev = &old->rev[idx]; guest_rpte = rev->guest_rpte; @@ -1290,7 +1295,6 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Reload PTE after unmap */ vpte = be64_to_cpu(hptep[0]); - BUG_ON(vpte & HPTE_V_VALID); BUG_ON(!(vpte & HPTE_V_ABSENT)); @@ -1299,6 +1303,12 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, goto out; rpte = be64_to_cpu(hptep[1]); + + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + vpte = hpte_new_to_old_v(vpte, rpte); + rpte = hpte_new_to_old_r(rpte); + } + pshift = kvmppc_hpte_base_page_shift(vpte, rpte); avpn = HPTE_V_AVPN_VAL(vpte) & ~(((1ul << pshift) - 1) >> 23); pteg = idx / HPTES_PER_GROUP; @@ -1336,6 +1346,10 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, new_hptep = (__be64 *)(new->virt + (new_idx << 4)); replace_vpte = be64_to_cpu(new_hptep[0]); + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + unsigned long replace_rpte = be64_to_cpu(new_hptep[1]); + replace_vpte = hpte_new_to_old_v(replace_vpte, replace_rpte); + } if (replace_vpte & (HPTE_V_VALID | HPTE_V_ABSENT)) { BUG_ON(new->order >= old->order); @@ -1351,6 +1365,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Discard the previous HPTE */ } + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + rpte = hpte_old_to_new_r(vpte, rpte); + vpte = hpte_old_to_new_v(vpte); + } + new_hptep[1] = cpu_to_be64(rpte); new->rev[new_idx].guest_rpte = guest_rpte; /* No need for a barrier, since new HPT isn't active */ @@ -1368,12 +1387,6 @@ static int resize_hpt_rehash(struct kvm_resize_hpt *resize) unsigned long i; int rc; - /* - * resize_hpt_rehash_hpte() doesn't handle the new-format HPTEs - * that POWER9 uses, and could well hit a BUG_ON on POWER9. - */ - if (cpu_has_feature(CPU_FTR_ARCH_300)) - return -EIO; for (i = 0; i < kvmppc_hpt_npte(&kvm->arch.hpt); i++) { rc = resize_hpt_rehash_hpte(resize, i); if (rc != 0) @@ -1404,6 +1417,9 @@ static void resize_hpt_pivot(struct kvm_resize_hpt *resize) synchronize_srcu_expedited(&kvm->srcu); + if (cpu_has_feature(CPU_FTR_ARCH_300)) + kvmppc_setup_partition_table(kvm); + resize_hpt_debug(resize, "resize_hpt_pivot() done\n"); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 2018-02-07 9:06 ` [PATCH 2/2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 Paul Mackerras @ 2018-02-07 9:11 ` Paul Mackerras 2018-02-08 18:11 ` Laurent Vivier 2018-02-08 23:40 ` David Gibson 0 siblings, 2 replies; 6+ messages in thread From: Paul Mackerras @ 2018-02-07 9:11 UTC (permalink / raw) To: kvm, kvm-ppc; +Cc: David Gibson From: David Gibson <david@gibson.dropbear.id.au> This adds code to enable the HPT resizing code to work on POWER9, which uses a slightly modified HPT entry format compared to POWER8. On POWER9, we convert HPTEs read from the HPT from the new format to the old format so that the rest of the HPT resizing code can work as before. HPTEs written to the new HPT are converted to the new format as the last step before writing them into the new HPT. This takes out the checks added by commit bcd3bb63dbc8 ("KVM: PPC: Book3S HV: Disable HPT resizing on POWER9 for now", 2017-02-18), now that HPT resizing works on POWER9. On POWER9, when we pivot to the new HPT, we now call kvmppc_setup_partition_table() to update the partition table in order to make the hardware use the new HPT. [paulus@ozlabs.org - added kvmppc_setup_partition_table() call, wrote commit message.] Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- v2: Include change to powerpc.c, which somehow got missed in v1. arch/powerpc/kvm/book3s_64_mmu_hv.c | 30 +++++++++++++++++++++++------- arch/powerpc/kvm/powerpc.c | 3 +-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c index d196499..cb34be7 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c @@ -1261,6 +1261,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Nothing to do */ goto out; + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + rpte = be64_to_cpu(hptep[1]); + vpte = hpte_new_to_old_v(vpte, rpte); + } + /* Unmap */ rev = &old->rev[idx]; guest_rpte = rev->guest_rpte; @@ -1290,7 +1295,6 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Reload PTE after unmap */ vpte = be64_to_cpu(hptep[0]); - BUG_ON(vpte & HPTE_V_VALID); BUG_ON(!(vpte & HPTE_V_ABSENT)); @@ -1299,6 +1303,12 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, goto out; rpte = be64_to_cpu(hptep[1]); + + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + vpte = hpte_new_to_old_v(vpte, rpte); + rpte = hpte_new_to_old_r(rpte); + } + pshift = kvmppc_hpte_base_page_shift(vpte, rpte); avpn = HPTE_V_AVPN_VAL(vpte) & ~(((1ul << pshift) - 1) >> 23); pteg = idx / HPTES_PER_GROUP; @@ -1336,6 +1346,10 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, new_hptep = (__be64 *)(new->virt + (new_idx << 4)); replace_vpte = be64_to_cpu(new_hptep[0]); + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + unsigned long replace_rpte = be64_to_cpu(new_hptep[1]); + replace_vpte = hpte_new_to_old_v(replace_vpte, replace_rpte); + } if (replace_vpte & (HPTE_V_VALID | HPTE_V_ABSENT)) { BUG_ON(new->order >= old->order); @@ -1351,6 +1365,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, /* Discard the previous HPTE */ } + if (cpu_has_feature(CPU_FTR_ARCH_300)) { + rpte = hpte_old_to_new_r(vpte, rpte); + vpte = hpte_old_to_new_v(vpte); + } + new_hptep[1] = cpu_to_be64(rpte); new->rev[new_idx].guest_rpte = guest_rpte; /* No need for a barrier, since new HPT isn't active */ @@ -1368,12 +1387,6 @@ static int resize_hpt_rehash(struct kvm_resize_hpt *resize) unsigned long i; int rc; - /* - * resize_hpt_rehash_hpte() doesn't handle the new-format HPTEs - * that POWER9 uses, and could well hit a BUG_ON on POWER9. - */ - if (cpu_has_feature(CPU_FTR_ARCH_300)) - return -EIO; for (i = 0; i < kvmppc_hpt_npte(&kvm->arch.hpt); i++) { rc = resize_hpt_rehash_hpte(resize, i); if (rc != 0) @@ -1404,6 +1417,9 @@ static void resize_hpt_pivot(struct kvm_resize_hpt *resize) synchronize_srcu_expedited(&kvm->srcu); + if (cpu_has_feature(CPU_FTR_ARCH_300)) + kvmppc_setup_partition_table(kvm); + resize_hpt_debug(resize, "resize_hpt_pivot() done\n"); } diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index b4010b8..47c7a30 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -633,8 +633,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = 1; break; case KVM_CAP_SPAPR_RESIZE_HPT: - /* Disable this on POWER9 until code handles new HPTE format */ - r = !!hv_enabled && !cpu_has_feature(CPU_FTR_ARCH_300); + r = !!hv_enabled; break; #endif #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 2018-02-07 9:11 ` [PATCH v2] " Paul Mackerras @ 2018-02-08 18:11 ` Laurent Vivier 2018-02-08 23:40 ` David Gibson 1 sibling, 0 replies; 6+ messages in thread From: Laurent Vivier @ 2018-02-08 18:11 UTC (permalink / raw) To: Paul Mackerras, kvm, kvm-ppc; +Cc: David Gibson On 07/02/2018 10:11, Paul Mackerras wrote: > From: David Gibson <david@gibson.dropbear.id.au> > > This adds code to enable the HPT resizing code to work on POWER9, > which uses a slightly modified HPT entry format compared to POWER8. > On POWER9, we convert HPTEs read from the HPT from the new format to > the old format so that the rest of the HPT resizing code can work as > before. HPTEs written to the new HPT are converted to the new format > as the last step before writing them into the new HPT. > > This takes out the checks added by commit bcd3bb63dbc8 ("KVM: PPC: > Book3S HV: Disable HPT resizing on POWER9 for now", 2017-02-18), > now that HPT resizing works on POWER9. > > On POWER9, when we pivot to the new HPT, we now call > kvmppc_setup_partition_table() to update the partition table in order > to make the hardware use the new HPT. > > [paulus@ozlabs.org - added kvmppc_setup_partition_table() call, > wrote commit message.] > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> > --- > v2: Include change to powerpc.c, which somehow got missed in v1. > > arch/powerpc/kvm/book3s_64_mmu_hv.c | 30 +++++++++++++++++++++++------- > arch/powerpc/kvm/powerpc.c | 3 +-- > 2 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index d196499..cb34be7 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -1261,6 +1261,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > /* Nothing to do */ > goto out; > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + rpte = be64_to_cpu(hptep[1]); > + vpte = hpte_new_to_old_v(vpte, rpte); > + } > + > /* Unmap */ > rev = &old->rev[idx]; > guest_rpte = rev->guest_rpte; > @@ -1290,7 +1295,6 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > > /* Reload PTE after unmap */ > vpte = be64_to_cpu(hptep[0]); > - > BUG_ON(vpte & HPTE_V_VALID); > BUG_ON(!(vpte & HPTE_V_ABSENT)); > > @@ -1299,6 +1303,12 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > goto out; > > rpte = be64_to_cpu(hptep[1]); > + > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + vpte = hpte_new_to_old_v(vpte, rpte); > + rpte = hpte_new_to_old_r(rpte); > + } > + > pshift = kvmppc_hpte_base_page_shift(vpte, rpte); > avpn = HPTE_V_AVPN_VAL(vpte) & ~(((1ul << pshift) - 1) >> 23); > pteg = idx / HPTES_PER_GROUP; > @@ -1336,6 +1346,10 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > new_hptep = (__be64 *)(new->virt + (new_idx << 4)); > > replace_vpte = be64_to_cpu(new_hptep[0]); > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + unsigned long replace_rpte = be64_to_cpu(new_hptep[1]); > + replace_vpte = hpte_new_to_old_v(replace_vpte, replace_rpte); > + } > > if (replace_vpte & (HPTE_V_VALID | HPTE_V_ABSENT)) { > BUG_ON(new->order >= old->order); > @@ -1351,6 +1365,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > /* Discard the previous HPTE */ > } > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + rpte = hpte_old_to_new_r(vpte, rpte); > + vpte = hpte_old_to_new_v(vpte); > + } > + > new_hptep[1] = cpu_to_be64(rpte); > new->rev[new_idx].guest_rpte = guest_rpte; > /* No need for a barrier, since new HPT isn't active */ > @@ -1368,12 +1387,6 @@ static int resize_hpt_rehash(struct kvm_resize_hpt *resize) > unsigned long i; > int rc; > > - /* > - * resize_hpt_rehash_hpte() doesn't handle the new-format HPTEs > - * that POWER9 uses, and could well hit a BUG_ON on POWER9. > - */ > - if (cpu_has_feature(CPU_FTR_ARCH_300)) > - return -EIO; > for (i = 0; i < kvmppc_hpt_npte(&kvm->arch.hpt); i++) { > rc = resize_hpt_rehash_hpte(resize, i); > if (rc != 0) > @@ -1404,6 +1417,9 @@ static void resize_hpt_pivot(struct kvm_resize_hpt *resize) > > synchronize_srcu_expedited(&kvm->srcu); > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + kvmppc_setup_partition_table(kvm); > + > resize_hpt_debug(resize, "resize_hpt_pivot() done\n"); > } > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index b4010b8..47c7a30 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -633,8 +633,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = 1; > break; > case KVM_CAP_SPAPR_RESIZE_HPT: > - /* Disable this on POWER9 until code handles new HPTE format */ > - r = !!hv_enabled && !cpu_has_feature(CPU_FTR_ARCH_300); > + r = !!hv_enabled; > break; > #endif > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > I've tested this series with a RHEL 4.14 kernel on a POWER9 host and a RHEL 3.10 kernel in the guest (max-cpu-compat=power8 and "-m 80G,slots=256,maxmem=257G"), all works well: [root@localhost ~]# cat /sys/kernel/debug/powerpc/hpt_order 30 [root@localhost ~]# echo 31 > /sys/kernel/debug/powerpc/hpt_order [ 65.888226] lpar: Attempting to resize HPT to shift 31 [ 66.634834] lpar: HPT resize to shift 31 complete (210 ms / 535 ms) [root@localhost ~]# cat /sys/kernel/debug/powerpc/hpt_order 31 [root@localhost ~]# echo 29 > /sys/kernel/debug/powerpc/hpt_order [ 77.023857] lpar: Attempting to resize HPT to shift 29 [ 77.739819] lpar: HPT resize to shift 29 complete (105 ms / 610 ms) [root@localhost ~]# cat /sys/kernel/debug/powerpc/hpt_order 29 Previously, it was failing with: -bash: echo: write error: No such device Tested-by: Laurent Vivier <lvivier@redhat.com> Thanks, Laurent ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 2018-02-07 9:11 ` [PATCH v2] " Paul Mackerras 2018-02-08 18:11 ` Laurent Vivier @ 2018-02-08 23:40 ` David Gibson 1 sibling, 0 replies; 6+ messages in thread From: David Gibson @ 2018-02-08 23:40 UTC (permalink / raw) To: Paul Mackerras; +Cc: kvm, kvm-ppc [-- Attachment #1: Type: text/plain, Size: 5108 bytes --] On Wed, Feb 07, 2018 at 08:11:35PM +1100, Paul Mackerras wrote: > From: David Gibson <david@gibson.dropbear.id.au> > > This adds code to enable the HPT resizing code to work on POWER9, > which uses a slightly modified HPT entry format compared to POWER8. > On POWER9, we convert HPTEs read from the HPT from the new format to > the old format so that the rest of the HPT resizing code can work as > before. HPTEs written to the new HPT are converted to the new format > as the last step before writing them into the new HPT. > > This takes out the checks added by commit bcd3bb63dbc8 ("KVM: PPC: > Book3S HV: Disable HPT resizing on POWER9 for now", 2017-02-18), > now that HPT resizing works on POWER9. > > On POWER9, when we pivot to the new HPT, we now call > kvmppc_setup_partition_table() to update the partition table in order > to make the hardware use the new HPT. > > [paulus@ozlabs.org - added kvmppc_setup_partition_table() call, > wrote commit message.] > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > v2: Include change to powerpc.c, which somehow got missed in v1. > > arch/powerpc/kvm/book3s_64_mmu_hv.c | 30 +++++++++++++++++++++++------- > arch/powerpc/kvm/powerpc.c | 3 +-- > 2 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index d196499..cb34be7 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -1261,6 +1261,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > /* Nothing to do */ > goto out; > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + rpte = be64_to_cpu(hptep[1]); > + vpte = hpte_new_to_old_v(vpte, rpte); > + } > + > /* Unmap */ > rev = &old->rev[idx]; > guest_rpte = rev->guest_rpte; > @@ -1290,7 +1295,6 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > > /* Reload PTE after unmap */ > vpte = be64_to_cpu(hptep[0]); > - > BUG_ON(vpte & HPTE_V_VALID); > BUG_ON(!(vpte & HPTE_V_ABSENT)); > > @@ -1299,6 +1303,12 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > goto out; > > rpte = be64_to_cpu(hptep[1]); > + > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + vpte = hpte_new_to_old_v(vpte, rpte); > + rpte = hpte_new_to_old_r(rpte); > + } > + > pshift = kvmppc_hpte_base_page_shift(vpte, rpte); > avpn = HPTE_V_AVPN_VAL(vpte) & ~(((1ul << pshift) - 1) >> 23); > pteg = idx / HPTES_PER_GROUP; > @@ -1336,6 +1346,10 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > new_hptep = (__be64 *)(new->virt + (new_idx << 4)); > > replace_vpte = be64_to_cpu(new_hptep[0]); > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + unsigned long replace_rpte = be64_to_cpu(new_hptep[1]); > + replace_vpte = hpte_new_to_old_v(replace_vpte, replace_rpte); > + } > > if (replace_vpte & (HPTE_V_VALID | HPTE_V_ABSENT)) { > BUG_ON(new->order >= old->order); > @@ -1351,6 +1365,11 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > /* Discard the previous HPTE */ > } > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) { > + rpte = hpte_old_to_new_r(vpte, rpte); > + vpte = hpte_old_to_new_v(vpte); > + } > + > new_hptep[1] = cpu_to_be64(rpte); > new->rev[new_idx].guest_rpte = guest_rpte; > /* No need for a barrier, since new HPT isn't active */ > @@ -1368,12 +1387,6 @@ static int resize_hpt_rehash(struct kvm_resize_hpt *resize) > unsigned long i; > int rc; > > - /* > - * resize_hpt_rehash_hpte() doesn't handle the new-format HPTEs > - * that POWER9 uses, and could well hit a BUG_ON on POWER9. > - */ > - if (cpu_has_feature(CPU_FTR_ARCH_300)) > - return -EIO; > for (i = 0; i < kvmppc_hpt_npte(&kvm->arch.hpt); i++) { > rc = resize_hpt_rehash_hpte(resize, i); > if (rc != 0) > @@ -1404,6 +1417,9 @@ static void resize_hpt_pivot(struct kvm_resize_hpt *resize) > > synchronize_srcu_expedited(&kvm->srcu); > > + if (cpu_has_feature(CPU_FTR_ARCH_300)) > + kvmppc_setup_partition_table(kvm); > + > resize_hpt_debug(resize, "resize_hpt_pivot() done\n"); > } > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index b4010b8..47c7a30 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -633,8 +633,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = 1; > break; > case KVM_CAP_SPAPR_RESIZE_HPT: > - /* Disable this on POWER9 until code handles new HPTE format */ > - r = !!hv_enabled && !cpu_has_feature(CPU_FTR_ARCH_300); > + r = !!hv_enabled; > break; > #endif > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code 2018-02-07 9:04 [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code Paul Mackerras 2018-02-07 9:06 ` [PATCH 2/2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 Paul Mackerras @ 2018-02-08 23:35 ` David Gibson 1 sibling, 0 replies; 6+ messages in thread From: David Gibson @ 2018-02-08 23:35 UTC (permalink / raw) To: Paul Mackerras; +Cc: kvm, kvm-ppc [-- Attachment #1: Type: text/plain, Size: 2226 bytes --] On Wed, Feb 07, 2018 at 08:04:45PM +1100, Paul Mackerras wrote: > This fixes the computation of the HPTE index to use when the HPT > resizing code encounters a bolted HPTE which is stored in its > secondary HPTE group. The code inverts the HPTE group number, which > is correct, but doesn't then mask it with new_hash_mask. As a result, > new_pteg will be effectively negative, resulting in new_hptep > pointing before the new HPT, which will corrupt memory. > > In addition, this removes two BUG_ON statements. The condition that > the BUG_ONs were testing -- that we have computed the hash value > incorrectly -- has never been observed in testing, and if it did > occur, would only affect the guest, not the host. Given that > BUG_ON should only be used in conditions where the kernel (i.e. > the host kernel, in this case) can't possibly continue execution, > it is not appropriate here. Fair enough. I think those were there because they were useful during early development, but should have been removed after that. > > Signed-off-by: Paul Mackerras <paulus@ozlabs.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > arch/powerpc/kvm/book3s_64_mmu_hv.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c > index 9660972..d196499 100644 > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c > @@ -1329,12 +1329,8 @@ static unsigned long resize_hpt_rehash_hpte(struct kvm_resize_hpt *resize, > } > > new_pteg = hash & new_hash_mask; > - if (vpte & HPTE_V_SECONDARY) { > - BUG_ON(~pteg != (hash & old_hash_mask)); > - new_pteg = ~new_pteg; > - } else { > - BUG_ON(pteg != (hash & old_hash_mask)); > - } > + if (vpte & HPTE_V_SECONDARY) > + new_pteg = ~hash & new_hash_mask; > > new_idx = new_pteg * HPTES_PER_GROUP + (idx % HPTES_PER_GROUP); > new_hptep = (__be64 *)(new->virt + (new_idx << 4)); -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-08 23:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-07 9:04 [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code Paul Mackerras 2018-02-07 9:06 ` [PATCH 2/2] KVM: PPC: Book3S HV: Make HPT resizing work on POWER9 Paul Mackerras 2018-02-07 9:11 ` [PATCH v2] " Paul Mackerras 2018-02-08 18:11 ` Laurent Vivier 2018-02-08 23:40 ` David Gibson 2018-02-08 23:35 ` [PATCH 1/2] KVM: PPC: Book3S HV: Fix handling of secondary HPTEG in HPT resizing code David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).