* [PATCH v2 24/30] KVM: PPC: Book3S PR: Support TAR handling for PR KVM HTM.
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
Currently guest kernel doesn't handle TAR fac unavailable and it always
runs with TAR bit on. PR KVM will lazily enable TAR. TAR is not a
frequent-use reg and it is not included in SVCPU struct.
Due to the above, the checkpointed TAR val might be a bogus TAR val.
To solve this issue, we will make vcpu->arch.fscr tar bit consistent
with shadow_fscr when TM enabled.
At the end of emulating treclaim., the correct TAR val need to be loaded
into reg if FSCR_TAR bit is on.
At the beginning of emulating trechkpt., TAR needs to be flushed so that
the right tar val can be copy into tar_tm.
Tested with:
tools/testing/selftests/powerpc/tm/tm-tar
tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar (remove DSCR/PPR
related testing).
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/include/asm/kvm_book3s.h | 2 ++
arch/powerpc/kvm/book3s_emulate.c | 4 ++++
arch/powerpc/kvm/book3s_pr.c | 21 ++++++++++++++++-----
arch/powerpc/kvm/tm.S | 16 ++++++++++++++--
4 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 5873906..4148a3d 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -267,6 +267,8 @@ static inline void kvmppc_save_tm_sprs(struct kvm_vcpu *vcpu) {}
static inline void kvmppc_restore_tm_sprs(struct kvm_vcpu *vcpu) {}
#endif
+void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac);
+
extern int kvm_irq_bypass;
static inline struct kvmppc_vcpu_book3s *to_book3s(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 6c5ef8c..8fd3af6a 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -173,6 +173,9 @@ static void kvmppc_emulate_treclaim(struct kvm_vcpu *vcpu, int ra_val)
guest_msr &= ~(MSR_TS_MASK);
kvmppc_set_msr(vcpu, guest_msr);
preempt_enable();
+
+ if (vcpu->arch.shadow_fscr & FSCR_TAR)
+ mtspr(SPRN_TAR, vcpu->arch.tar);
}
static void kvmppc_emulate_trchkpt(struct kvm_vcpu *vcpu)
@@ -185,6 +188,7 @@ static void kvmppc_emulate_trchkpt(struct kvm_vcpu *vcpu)
* copy.
*/
kvmppc_giveup_ext(vcpu, MSR_VSX);
+ kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
kvmppc_copyto_vcpu_tm(vcpu);
kvmppc_save_tm_sprs(vcpu);
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 29ec92e..19f9934 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -55,7 +55,7 @@
static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr,
ulong msr);
-static void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac);
+static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac);
/* Some compatibility defines */
#ifdef CONFIG_PPC_BOOK3S_32
@@ -348,6 +348,7 @@ void kvmppc_save_tm_pr(struct kvm_vcpu *vcpu)
return;
}
+ kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
kvmppc_giveup_ext(vcpu, MSR_VSX);
preempt_disable();
@@ -359,8 +360,11 @@ void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu)
{
if (!MSR_TM_ACTIVE(kvmppc_get_msr(vcpu))) {
kvmppc_restore_tm_sprs(vcpu);
- if (kvmppc_get_msr(vcpu) & MSR_TM)
+ if (kvmppc_get_msr(vcpu) & MSR_TM) {
kvmppc_handle_lost_math_exts(vcpu);
+ if (vcpu->arch.fscr & FSCR_TAR)
+ kvmppc_handle_fac(vcpu, FSCR_TAR_LG);
+ }
return;
}
@@ -368,9 +372,11 @@ void kvmppc_restore_tm_pr(struct kvm_vcpu *vcpu)
_kvmppc_restore_tm_pr(vcpu, kvmppc_get_msr(vcpu));
preempt_enable();
- if (kvmppc_get_msr(vcpu) & MSR_TM)
+ if (kvmppc_get_msr(vcpu) & MSR_TM) {
kvmppc_handle_lost_math_exts(vcpu);
-
+ if (vcpu->arch.fscr & FSCR_TAR)
+ kvmppc_handle_fac(vcpu, FSCR_TAR_LG);
+ }
}
#endif
@@ -828,7 +834,7 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu, ulong msr)
}
/* Give up facility (TAR / EBB / DSCR) */
-static void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac)
+void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac)
{
#ifdef CONFIG_PPC_BOOK3S_64
if (!(vcpu->arch.shadow_fscr & (1ULL << fac))) {
@@ -1029,7 +1035,12 @@ void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
/* TAR got dropped, drop it in shadow too */
kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
+ } else if (!(vcpu->arch.fscr & FSCR_TAR) && (fscr & FSCR_TAR)) {
+ vcpu->arch.fscr = fscr;
+ kvmppc_handle_fac(vcpu, FSCR_TAR_LG);
+ return;
}
+
vcpu->arch.fscr = fscr;
}
#endif
diff --git a/arch/powerpc/kvm/tm.S b/arch/powerpc/kvm/tm.S
index 269dd11..aa84e4a 100644
--- a/arch/powerpc/kvm/tm.S
+++ b/arch/powerpc/kvm/tm.S
@@ -164,15 +164,21 @@ _GLOBAL(_kvmppc_save_tm_pr)
mfmsr r5
SAVE_GPR(5, r1)
- /* also save DSCR/CR so that it can be recovered later */
+ /* also save DSCR/CR/TAR so that it can be recovered later */
mfspr r6, SPRN_DSCR
SAVE_GPR(6, r1)
mfcr r7
stw r7, _CCR(r1)
+ mfspr r8, SPRN_TAR
+ SAVE_GPR(8, r1)
+
bl __kvmppc_save_tm
+ REST_GPR(8, r1)
+ mtspr SPRN_TAR, r8
+
ld r7, _CCR(r1)
mtcr r7
@@ -335,15 +341,21 @@ _GLOBAL(_kvmppc_restore_tm_pr)
mfmsr r5
SAVE_GPR(5, r1)
- /* also save DSCR/CR so that it can be recovered later */
+ /* also save DSCR/CR/TAR so that it can be recovered later */
mfspr r6, SPRN_DSCR
SAVE_GPR(6, r1)
mfcr r7
stw r7, _CCR(r1)
+ mfspr r8, SPRN_TAR
+ SAVE_GPR(8, r1)
+
bl __kvmppc_restore_tm
+ REST_GPR(8, r1)
+ mtspr SPRN_TAR, r8
+
ld r7, _CCR(r1)
mtcr r7
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 25/30] KVM: PPC: Book3S PR: enable HTM for PR KVM for KVM_CHECK_EXTENSION ioctl
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
With current patch set, PR KVM now supports HTM. So this patch turns it
on for PR KVM.
Tested with:
https://github.com/justdoitqd/publicFiles/blob/master/test_kvm_htm_cap.c
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/powerpc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 748562e..2d10314 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -648,8 +648,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
break;
#endif
case KVM_CAP_PPC_HTM:
- r = hv_enabled &&
- (cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_HTM_COMP);
+ r = (cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_HTM_COMP);
break;
default:
r = 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 26/30] KVM: PPC: move vcpu_load/vcpu_put down to each ioctl case in kvm_arch_vcpu_ioctl
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
Although we already have kvm_arch_vcpu_async_ioctl() which doesn't require
ioctl to load vcpu, the sync ioctl code need to be cleaned up when
CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL is not configured.
This patch moves vcpu_load/vcpu_put down to each ioctl switch case so that
each ioctl can decide to do vcpu_load/vcpu_put or not independently.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/powerpc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2d10314..cf3effd 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1629,16 +1629,16 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
void __user *argp = (void __user *)arg;
long r;
- vcpu_load(vcpu);
-
switch (ioctl) {
case KVM_ENABLE_CAP:
{
struct kvm_enable_cap cap;
r = -EFAULT;
+ vcpu_load(vcpu);
if (copy_from_user(&cap, argp, sizeof(cap)))
goto out;
r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
+ vcpu_put(vcpu);
break;
}
@@ -1647,12 +1647,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
{
struct kvm_one_reg reg;
r = -EFAULT;
+ vcpu_load(vcpu);
if (copy_from_user(®, argp, sizeof(reg)))
goto out;
if (ioctl == KVM_SET_ONE_REG)
r = kvm_vcpu_ioctl_set_one_reg(vcpu, ®);
else
r = kvm_vcpu_ioctl_get_one_reg(vcpu, ®);
+ vcpu_put(vcpu);
break;
}
@@ -1660,9 +1662,11 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
case KVM_DIRTY_TLB: {
struct kvm_dirty_tlb dirty;
r = -EFAULT;
+ vcpu_load(vcpu);
if (copy_from_user(&dirty, argp, sizeof(dirty)))
goto out;
r = kvm_vcpu_ioctl_dirty_tlb(vcpu, &dirty);
+ vcpu_put(vcpu);
break;
}
#endif
@@ -1671,7 +1675,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
}
out:
- vcpu_put(vcpu);
return r;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 27/30] KVM: PPC: remove load/put vcpu for KVM_GET/SET_ONE_REG ioctl
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
Due to the vcpu mutex locking/unlock has been moved out of vcpu_load()
/vcpu_put(), KVM_GET_ONE_REG and KVM_SET_ONE_REG doesn't need to do
ioctl with loading vcpu anymore. This patch removes vcpu_load()/vcpu_put()
from KVM_GET_ONE_REG and KVM_SET_ONE_REG ioctl.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/powerpc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index cf3effd..7987fa3 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1647,14 +1647,12 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
{
struct kvm_one_reg reg;
r = -EFAULT;
- vcpu_load(vcpu);
if (copy_from_user(®, argp, sizeof(reg)))
goto out;
if (ioctl == KVM_SET_ONE_REG)
r = kvm_vcpu_ioctl_set_one_reg(vcpu, ®);
else
r = kvm_vcpu_ioctl_get_one_reg(vcpu, ®);
- vcpu_put(vcpu);
break;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 28/30] KVM: PPC: remove load/put vcpu for KVM_GET_REGS/KVM_SET_REGS
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
In both HV/PR KVM, the KVM_SET_REGS/KVM_GET_REGS ioctl should
be able to perform without load vcpu. This patch adds
KVM_SET_ONE_REG/KVM_GET_ONE_REG implementation to async ioctl
function.
Due to the vcpu mutex locking/unlock has been moved out of vcpu_load()
/vcpu_put(), KVM_SET_REGS/KVM_GET_REGS don't need to do
ioctl with loading vcpu anymore. This patch removes vcpu_load()/vcpu_put()
from KVM_SET_REGS/KVM_GET_REGS ioctl.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/book3s.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 234531d..10b2a21 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -509,8 +509,6 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
int i;
- vcpu_load(vcpu);
-
regs->pc = kvmppc_get_pc(vcpu);
regs->cr = kvmppc_get_cr(vcpu);
regs->ctr = kvmppc_get_ctr(vcpu);
@@ -532,7 +530,6 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
regs->gpr[i] = kvmppc_get_gpr(vcpu, i);
- vcpu_put(vcpu);
return 0;
}
@@ -540,8 +537,6 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
{
int i;
- vcpu_load(vcpu);
-
kvmppc_set_pc(vcpu, regs->pc);
kvmppc_set_cr(vcpu, regs->cr);
kvmppc_set_ctr(vcpu, regs->ctr);
@@ -562,7 +557,6 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
for (i = 0; i < ARRAY_SIZE(regs->gpr); i++)
kvmppc_set_gpr(vcpu, i, regs->gpr[i]);
- vcpu_put(vcpu);
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 29/30] KVM: PPC: add KVM_SET_ONE_REG/KVM_GET_ONE_REG to async ioctl
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
In both HV/PR KVM, the KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctl should
be able to perform without load vcpu. This patch adds
KVM_SET_ONE_REG/KVM_GET_ONE_REG implementation to async ioctl
function.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/powerpc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 7987fa3..6afd004 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1619,6 +1619,19 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
return -EFAULT;
return kvm_vcpu_ioctl_interrupt(vcpu, &irq);
}
+
+ if ((ioctl == KVM_SET_ONE_REG) || (ioctl == KVM_GET_ONE_REG)) {
+ struct kvm_one_reg reg;
+
+ if (copy_from_user(®, argp, sizeof(reg)))
+ return -EFAULT;
+
+ if (ioctl == KVM_SET_ONE_REG)
+ return kvm_vcpu_ioctl_set_one_reg(vcpu, ®);
+ else
+ return kvm_vcpu_ioctl_get_one_reg(vcpu, ®);
+ }
+
return -ENOIOCTLCMD;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 30/30] KVM: PPC: Book3S PR: enable kvmppc_get/set_one_reg_pr() for HTM registers
From: wei.guo.simon @ 2018-02-27 17:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras, kvm, kvm-ppc, Simon Guo
In-Reply-To: <1519753958-11756-1-git-send-email-wei.guo.simon@gmail.com>
From: Simon Guo <wei.guo.simon@gmail.com>
We need to migrate PR KVM during transaction and qemu will use
kvmppc_get_one_reg_pr()/kvmppc_set_one_reg_pr() APIs to get/set
transaction checkpoint state. This patch adds support for that.
So far PPC PR qemu doesn't fully function for migration but the
savevm/loadvm can be done against a RHEL72 guest. During savevm/
loadvm procedure, the kvm ioctls will be invoked as well.
Test has been performed to savevm/loadvm for a guest running
a HTM test program:
https://github.com/justdoitqd/publicFiles/blob/master/test-tm-mig.c
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/powerpc/kvm/book3s_pr.c | 133 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 133 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 19f9934..3439229 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1546,6 +1546,73 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id,
else
*val = get_reg_val(id, 0);
break;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ case KVM_REG_PPC_TFHAR:
+ *val = get_reg_val(id, vcpu->arch.tfhar);
+ break;
+ case KVM_REG_PPC_TFIAR:
+ *val = get_reg_val(id, vcpu->arch.tfiar);
+ break;
+ case KVM_REG_PPC_TEXASR:
+ *val = get_reg_val(id, vcpu->arch.texasr);
+ break;
+ case KVM_REG_PPC_TM_GPR0 ... KVM_REG_PPC_TM_GPR31:
+ *val = get_reg_val(id,
+ vcpu->arch.gpr_tm[id-KVM_REG_PPC_TM_GPR0]);
+ break;
+ case KVM_REG_PPC_TM_VSR0 ... KVM_REG_PPC_TM_VSR63:
+ {
+ int i, j;
+
+ i = id - KVM_REG_PPC_TM_VSR0;
+ if (i < 32)
+ for (j = 0; j < TS_FPRWIDTH; j++)
+ val->vsxval[j] = vcpu->arch.fp_tm.fpr[i][j];
+ else {
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ val->vval = vcpu->arch.vr_tm.vr[i-32];
+ else
+ r = -ENXIO;
+ }
+ break;
+ }
+ case KVM_REG_PPC_TM_CR:
+ *val = get_reg_val(id, vcpu->arch.cr_tm);
+ break;
+ case KVM_REG_PPC_TM_XER:
+ *val = get_reg_val(id, vcpu->arch.xer_tm);
+ break;
+ case KVM_REG_PPC_TM_LR:
+ *val = get_reg_val(id, vcpu->arch.lr_tm);
+ break;
+ case KVM_REG_PPC_TM_CTR:
+ *val = get_reg_val(id, vcpu->arch.ctr_tm);
+ break;
+ case KVM_REG_PPC_TM_FPSCR:
+ *val = get_reg_val(id, vcpu->arch.fp_tm.fpscr);
+ break;
+ case KVM_REG_PPC_TM_AMR:
+ *val = get_reg_val(id, vcpu->arch.amr_tm);
+ break;
+ case KVM_REG_PPC_TM_PPR:
+ *val = get_reg_val(id, vcpu->arch.ppr_tm);
+ break;
+ case KVM_REG_PPC_TM_VRSAVE:
+ *val = get_reg_val(id, vcpu->arch.vrsave_tm);
+ break;
+ case KVM_REG_PPC_TM_VSCR:
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ *val = get_reg_val(id, vcpu->arch.vr_tm.vscr.u[3]);
+ else
+ r = -ENXIO;
+ break;
+ case KVM_REG_PPC_TM_DSCR:
+ *val = get_reg_val(id, vcpu->arch.dscr_tm);
+ break;
+ case KVM_REG_PPC_TM_TAR:
+ *val = get_reg_val(id, vcpu->arch.tar_tm);
+ break;
+#endif
default:
r = -EINVAL;
break;
@@ -1579,6 +1646,72 @@ static int kvmppc_set_one_reg_pr(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_LPCR_64:
kvmppc_set_lpcr_pr(vcpu, set_reg_val(id, *val));
break;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ case KVM_REG_PPC_TFHAR:
+ vcpu->arch.tfhar = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TFIAR:
+ vcpu->arch.tfiar = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TEXASR:
+ vcpu->arch.texasr = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_GPR0 ... KVM_REG_PPC_TM_GPR31:
+ vcpu->arch.gpr_tm[id - KVM_REG_PPC_TM_GPR0] =
+ set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_VSR0 ... KVM_REG_PPC_TM_VSR63:
+ {
+ int i, j;
+
+ i = id - KVM_REG_PPC_TM_VSR0;
+ if (i < 32)
+ for (j = 0; j < TS_FPRWIDTH; j++)
+ vcpu->arch.fp_tm.fpr[i][j] = val->vsxval[j];
+ else
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ vcpu->arch.vr_tm.vr[i-32] = val->vval;
+ else
+ r = -ENXIO;
+ break;
+ }
+ case KVM_REG_PPC_TM_CR:
+ vcpu->arch.cr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_XER:
+ vcpu->arch.xer_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_LR:
+ vcpu->arch.lr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_CTR:
+ vcpu->arch.ctr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_FPSCR:
+ vcpu->arch.fp_tm.fpscr = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_AMR:
+ vcpu->arch.amr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_PPR:
+ vcpu->arch.ppr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_VRSAVE:
+ vcpu->arch.vrsave_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_VSCR:
+ if (cpu_has_feature(CPU_FTR_ALTIVEC))
+ vcpu->arch.vr.vscr.u[3] = set_reg_val(id, *val);
+ else
+ r = -ENXIO;
+ break;
+ case KVM_REG_PPC_TM_DSCR:
+ vcpu->arch.dscr_tm = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_TM_TAR:
+ vcpu->arch.tar_tm = set_reg_val(id, *val);
+ break;
+#endif
default:
r = -EINVAL;
break;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 01/21] powerpc: Remove warning on array size when empty
From: Segher Boessenkool @ 2018-02-27 20:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mathieu Malaterre, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Paul Mackerras, Linux Kernel Mailing List, Jiri Slaby
In-Reply-To: <CAHp75VePco3hNo_vM916CiLzBAd18i=O1TaecD5YEDewvMpYYQ@mail.gmail.com>
On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> > <christophe.leroy@c-s.fr> wrote:
>
> >>>>> Much simpler is just add
> >>>>>
> >>>>> if (ARRAY_SIZE() == 0)
> >>>>> return;
>
> >> Or add in front:
> >> if (!ARRAY_SIZE(feature_properties))
> >> return;
> >
> > (not tested) I believe the compiler still go over the for() loop and
> > will complain about the original unsigned comparison.
>
> Did you run tests? Did you look into object file?
>
> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.
It does, but it also still warns (this warning is done very early in the
compiler pipeline).
Segher
^ permalink raw reply
* [PATCH v2] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Alistair Popple @ 2018-02-28 0:38 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Balbir Singh, Mark Hairgrove, mpe, Alistair Popple
When sending TLB invalidates to the NPU we need to send extra flushes due
to a hardware issue. The original implementation would lock the all the
ATSD MMIO registers sequentially before unlocking and relocking each of
them sequentially to do the extra flush.
This introduced a deadlock as it is possible for one thread to hold one
ATSD register whilst waiting for another register to be freed while the
other thread is holding that register waiting for the one in the first
thread to be freed.
For example if there are two threads and two ATSD registers:
Thread A Thread B
Acquire 1
Acquire 2
Release 1 Acquire 1
Wait 1 Wait 2
Both threads will be stuck waiting to acquire a register resulting in an
RCU stall warning or soft lockup.
This patch solves the deadlock by refactoring the code to ensure registers
are not released between flushes and to ensure all registers are either
acquired or released together and in order.
Fixes: bbd5ff50afff ("powerpc/powernv/npu-dma: Add explicit flush when sending an ATSD")
Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
v2: Added memory barriers around ->npdev[] and ATSD register aquisition/release
arch/powerpc/platforms/powernv/npu-dma.c | 203 +++++++++++++++++--------------
1 file changed, 113 insertions(+), 90 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index 0a253b64ac5f..2fed4b116e19 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -410,6 +410,11 @@ struct npu_context {
void *priv;
};
+struct mmio_atsd_reg {
+ struct npu *npu;
+ int reg;
+};
+
/*
* Find a free MMIO ATSD register and mark it in use. Return -ENOSPC
* if none are available.
@@ -419,7 +424,7 @@ static int get_mmio_atsd_reg(struct npu *npu)
int i;
for (i = 0; i < npu->mmio_atsd_count; i++) {
- if (!test_and_set_bit(i, &npu->mmio_atsd_usage))
+ if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage))
return i;
}
@@ -428,86 +433,90 @@ static int get_mmio_atsd_reg(struct npu *npu)
static void put_mmio_atsd_reg(struct npu *npu, int reg)
{
- clear_bit(reg, &npu->mmio_atsd_usage);
+ clear_bit_unlock(reg, &npu->mmio_atsd_usage);
}
/* MMIO ATSD register offsets */
#define XTS_ATSD_AVA 1
#define XTS_ATSD_STAT 2
-static int mmio_launch_invalidate(struct npu *npu, unsigned long launch,
- unsigned long va)
+static void mmio_launch_invalidate(struct mmio_atsd_reg *mmio_atsd_reg,
+ unsigned long launch, unsigned long va)
{
- int mmio_atsd_reg;
-
- do {
- mmio_atsd_reg = get_mmio_atsd_reg(npu);
- cpu_relax();
- } while (mmio_atsd_reg < 0);
+ struct npu *npu = mmio_atsd_reg->npu;
+ int reg = mmio_atsd_reg->reg;
__raw_writeq(cpu_to_be64(va),
- npu->mmio_atsd_regs[mmio_atsd_reg] + XTS_ATSD_AVA);
+ npu->mmio_atsd_regs[reg] + XTS_ATSD_AVA);
eieio();
- __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[mmio_atsd_reg]);
-
- return mmio_atsd_reg;
+ __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[reg]);
}
-static int mmio_invalidate_pid(struct npu *npu, unsigned long pid, bool flush)
+static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
+ unsigned long pid, bool flush)
{
+ int i;
unsigned long launch;
- /* IS set to invalidate matching PID */
- launch = PPC_BIT(12);
+ for (i = 0; i <= max_npu2_index; i++) {
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
+
+ /* IS set to invalidate matching PID */
+ launch = PPC_BIT(12);
- /* PRS set to process-scoped */
- launch |= PPC_BIT(13);
+ /* PRS set to process-scoped */
+ launch |= PPC_BIT(13);
- /* AP */
- launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
+ /* AP */
+ launch |= (u64)
+ mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
+ /* PID */
+ launch |= pid << PPC_BITLSHIFT(38);
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+ /* No flush */
+ launch |= !flush << PPC_BITLSHIFT(39);
- /* Invalidating the entire process doesn't use a va */
- return mmio_launch_invalidate(npu, launch, 0);
+ /* Invalidating the entire process doesn't use a va */
+ mmio_launch_invalidate(&mmio_atsd_reg[i], launch, 0);
+ }
}
-static int mmio_invalidate_va(struct npu *npu, unsigned long va,
- unsigned long pid, bool flush)
+static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
+ unsigned long va, unsigned long pid, bool flush)
{
+ int i;
unsigned long launch;
- /* IS set to invalidate target VA */
- launch = 0;
+ for (i = 0; i <= max_npu2_index; i++) {
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
+
+ /* IS set to invalidate target VA */
+ launch = 0;
- /* PRS set to process scoped */
- launch |= PPC_BIT(13);
+ /* PRS set to process scoped */
+ launch |= PPC_BIT(13);
- /* AP */
- launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
+ /* AP */
+ launch |= (u64)
+ mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
- /* PID */
- launch |= pid << PPC_BITLSHIFT(38);
+ /* PID */
+ launch |= pid << PPC_BITLSHIFT(38);
- /* No flush */
- launch |= !flush << PPC_BITLSHIFT(39);
+ /* No flush */
+ launch |= !flush << PPC_BITLSHIFT(39);
- return mmio_launch_invalidate(npu, launch, va);
+ mmio_launch_invalidate(&mmio_atsd_reg[i], launch, va);
+ }
}
#define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
-struct mmio_atsd_reg {
- struct npu *npu;
- int reg;
-};
-
static void mmio_invalidate_wait(
- struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS], bool flush)
+ struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
{
struct npu *npu;
int i, reg;
@@ -522,16 +531,46 @@ static void mmio_invalidate_wait(
reg = mmio_atsd_reg[i].reg;
while (__raw_readq(npu->mmio_atsd_regs[reg] + XTS_ATSD_STAT))
cpu_relax();
+ }
+}
- put_mmio_atsd_reg(npu, reg);
+static void acquire_atsd_reg(struct npu_context *npu_context,
+ struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
+{
+ int i, j;
+ struct npu *npu;
+ struct pci_dev *npdev;
+ struct pnv_phb *nphb;
- /*
- * The GPU requires two flush ATSDs to ensure all entries have
- * been flushed. We use PID 0 as it will never be used for a
- * process on the GPU.
- */
- if (flush)
- mmio_invalidate_pid(npu, 0, true);
+ for (i = 0; i <= max_npu2_index; i++) {
+ mmio_atsd_reg[i].reg = -1;
+ for (j = 0; j < NV_MAX_LINKS; j++) {
+ npdev = READ_ONCE(npu_context->npdev[i][j]);
+ if (!npdev)
+ continue;
+
+ nphb = pci_bus_to_host(npdev->bus)->private_data;
+ npu = &nphb->npu;
+ mmio_atsd_reg[i].npu = npu;
+ mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
+ while (mmio_atsd_reg[i].reg < 0) {
+ mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
+ cpu_relax();
+ }
+ break;
+ }
+ }
+}
+
+static void release_atsd_reg(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
+{
+ int i;
+
+ for (i = 0; i <= max_npu2_index; i++) {
+ if (mmio_atsd_reg[i].reg < 0)
+ continue;
+
+ put_mmio_atsd_reg(mmio_atsd_reg[i].npu, mmio_atsd_reg[i].reg);
}
}
@@ -542,10 +581,6 @@ static void mmio_invalidate_wait(
static void mmio_invalidate(struct npu_context *npu_context, int va,
unsigned long address, bool flush)
{
- int i, j;
- struct npu *npu;
- struct pnv_phb *nphb;
- struct pci_dev *npdev;
struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
unsigned long pid = npu_context->mm->context.id;
@@ -561,37 +596,25 @@ static void mmio_invalidate(struct npu_context *npu_context, int va,
* Loop over all the NPUs this process is active on and launch
* an invalidate.
*/
- for (i = 0; i <= max_npu2_index; i++) {
- mmio_atsd_reg[i].reg = -1;
- for (j = 0; j < NV_MAX_LINKS; j++) {
- npdev = npu_context->npdev[i][j];
- if (!npdev)
- continue;
-
- nphb = pci_bus_to_host(npdev->bus)->private_data;
- npu = &nphb->npu;
- mmio_atsd_reg[i].npu = npu;
-
- if (va)
- mmio_atsd_reg[i].reg =
- mmio_invalidate_va(npu, address, pid,
- flush);
- else
- mmio_atsd_reg[i].reg =
- mmio_invalidate_pid(npu, pid, flush);
-
- /*
- * The NPU hardware forwards the shootdown to all GPUs
- * so we only have to launch one shootdown per NPU.
- */
- break;
- }
+ acquire_atsd_reg(npu_context, mmio_atsd_reg);
+ if (va)
+ mmio_invalidate_va(mmio_atsd_reg, address, pid, flush);
+ else
+ mmio_invalidate_pid(mmio_atsd_reg, pid, flush);
+
+ mmio_invalidate_wait(mmio_atsd_reg);
+ if (flush) {
+ /*
+ * The GPU requires two flush ATSDs to ensure all entries have
+ * been flushed. We use PID 0 as it will never be used for a
+ * process on the GPU.
+ */
+ mmio_invalidate_pid(mmio_atsd_reg, 0, true);
+ mmio_invalidate_wait(mmio_atsd_reg);
+ mmio_invalidate_pid(mmio_atsd_reg, 0, true);
+ mmio_invalidate_wait(mmio_atsd_reg);
}
-
- mmio_invalidate_wait(mmio_atsd_reg, flush);
- if (flush)
- /* Wait for the flush to complete */
- mmio_invalidate_wait(mmio_atsd_reg, false);
+ release_atsd_reg(mmio_atsd_reg);
}
static void pnv_npu2_mn_release(struct mmu_notifier *mn,
@@ -726,7 +749,7 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
&nvlink_index)))
return ERR_PTR(-ENODEV);
- npu_context->npdev[npu->index][nvlink_index] = npdev;
+ WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
if (!nphb->npu.nmmu_flush) {
/*
@@ -778,7 +801,7 @@ void pnv_npu2_destroy_context(struct npu_context *npu_context,
if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
&nvlink_index)))
return;
- npu_context->npdev[npu->index][nvlink_index] = NULL;
+ WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], NULL);
opal_npu_destroy_context(nphb->opal_id, npu_context->mm->context.id,
PCI_DEVID(gpdev->bus->number, gpdev->devfn));
kref_put(&npu_context->kref, pnv_npu2_release_context);
--
2.11.0
^ permalink raw reply related
* powerpc/powernv/mce: Don't silently restart the machine
From: Balbir Singh @ 2018-02-28 1:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, npiggin, Balbir Singh
On MCE the current code will restart the machine with
ppc_md.restart(). This case was extremely unlikely since
prior to that a skiboot call is made and that resulted in
a checkstop for analysis.
With newer skiboots, on P9 we don't checkstop the box by
default, instead we return back to the kernel to extract
useful information at the time of the MCE. While we still
get this information, this patch converts the restart to
a panic(), so that if configured a dump can be taken and
we can track and probably debug the potential issue causing
the MCE.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 69b5263fc9e3..b510a6f41b00 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -500,9 +500,12 @@ void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
* opal to trigger checkstop explicitly for error analysis.
* The FSP PRD component would have already got notified
* about this error through other channels.
+ * 4. We are running on a newer skiboot that by default does
+ * not cause a checkstop, drops us back to the kernel to
+ * extract context and state at the time of the error.
*/
- ppc_md.restart(NULL);
+ panic("PowerNV Unrecovered Machine Check");
}
int opal_machine_check(struct pt_regs *regs)
--
2.13.6
^ permalink raw reply related
* [PATCH] KVM: surround kvm_arch_vcpu_async_ioctl() with #ifdef
From: wei.guo.simon @ 2018-02-28 2:31 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, linux-mips, linuxppc-dev, Paolo Bonzini, Simon Guo
From: Simon Guo <wei.guo.simon@gmail.com>
Although CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL is usually on, logically
kvm_arch_vcpu_async_ioctl() definition should be wrapped with
CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL #ifdef.
This patch adds the #ifdef surround.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
arch/mips/kvm/mips.c | 2 ++
arch/powerpc/kvm/powerpc.c | 2 ++
arch/s390/kvm/kvm-s390.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 2549fdd..4d593e5 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -903,6 +903,7 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
return r;
}
+#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
long kvm_arch_vcpu_async_ioctl(struct file *filp, unsigned int ioctl,
unsigned long arg)
{
@@ -922,6 +923,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp, unsigned int ioctl,
return -ENOIOCTLCMD;
}
+#endif
long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
unsigned long arg)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 403e642..2adca3c 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1757,6 +1757,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
return -EINVAL;
}
+#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
long kvm_arch_vcpu_async_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -1771,6 +1772,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
}
return -ENOIOCTLCMD;
}
+#endif
long kvm_arch_vcpu_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 77d7818..c499396 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3784,6 +3784,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
return r;
}
+#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
long kvm_arch_vcpu_async_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
{
@@ -3811,6 +3812,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
}
return -ENOIOCTLCMD;
}
+#endif
long kvm_arch_vcpu_ioctl(struct file *filp,
unsigned int ioctl, unsigned long arg)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Balbir Singh @ 2018-02-28 3:08 UTC (permalink / raw)
To: Alistair Popple; +Cc: linuxppc-dev, Mark Hairgrove, mpe
In-Reply-To: <20180228003814.32124-1-alistair@popple.id.au>
On Wed, 28 Feb 2018 11:38:14 +1100
Alistair Popple <alistair@popple.id.au> wrote:
> When sending TLB invalidates to the NPU we need to send extra flushes due
> to a hardware issue. The original implementation would lock the all the
> ATSD MMIO registers sequentially before unlocking and relocking each of
> them sequentially to do the extra flush.
>
> This introduced a deadlock as it is possible for one thread to hold one
> ATSD register whilst waiting for another register to be freed while the
> other thread is holding that register waiting for the one in the first
> thread to be freed.
>
> For example if there are two threads and two ATSD registers:
>
> Thread A Thread B
> Acquire 1
> Acquire 2
> Release 1 Acquire 1
> Wait 1 Wait 2
>
> Both threads will be stuck waiting to acquire a register resulting in an
> RCU stall warning or soft lockup.
>
> This patch solves the deadlock by refactoring the code to ensure registers
> are not released between flushes and to ensure all registers are either
> acquired or released together and in order.
>
> Fixes: bbd5ff50afff ("powerpc/powernv/npu-dma: Add explicit flush when sending an ATSD")
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>
> ---
>
> v2: Added memory barriers around ->npdev[] and ATSD register aquisition/release
>
> arch/powerpc/platforms/powernv/npu-dma.c | 203 +++++++++++++++++--------------
> 1 file changed, 113 insertions(+), 90 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index 0a253b64ac5f..2fed4b116e19 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -410,6 +410,11 @@ struct npu_context {
> void *priv;
> };
>
> +struct mmio_atsd_reg {
> + struct npu *npu;
> + int reg;
> +};
> +
> /*
> * Find a free MMIO ATSD register and mark it in use. Return -ENOSPC
> * if none are available.
> @@ -419,7 +424,7 @@ static int get_mmio_atsd_reg(struct npu *npu)
> int i;
>
> for (i = 0; i < npu->mmio_atsd_count; i++) {
> - if (!test_and_set_bit(i, &npu->mmio_atsd_usage))
> + if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage))
> return i;
> }
>
> @@ -428,86 +433,90 @@ static int get_mmio_atsd_reg(struct npu *npu)
>
> static void put_mmio_atsd_reg(struct npu *npu, int reg)
> {
> - clear_bit(reg, &npu->mmio_atsd_usage);
> + clear_bit_unlock(reg, &npu->mmio_atsd_usage);
> }
I think we need to document in the code that we have a hard reliance on
the order of locks being incremental sequential and that any optimizations
otherwise will result in probable deadlocks.
>
> /* MMIO ATSD register offsets */
> #define XTS_ATSD_AVA 1
> #define XTS_ATSD_STAT 2
>
> -static int mmio_launch_invalidate(struct npu *npu, unsigned long launch,
> - unsigned long va)
> +static void mmio_launch_invalidate(struct mmio_atsd_reg *mmio_atsd_reg,
> + unsigned long launch, unsigned long va)
> {
> - int mmio_atsd_reg;
> -
> - do {
> - mmio_atsd_reg = get_mmio_atsd_reg(npu);
> - cpu_relax();
> - } while (mmio_atsd_reg < 0);
> + struct npu *npu = mmio_atsd_reg->npu;
> + int reg = mmio_atsd_reg->reg;
>
> __raw_writeq(cpu_to_be64(va),
> - npu->mmio_atsd_regs[mmio_atsd_reg] + XTS_ATSD_AVA);
> + npu->mmio_atsd_regs[reg] + XTS_ATSD_AVA);
> eieio();
> - __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[mmio_atsd_reg]);
> -
> - return mmio_atsd_reg;
> + __raw_writeq(cpu_to_be64(launch), npu->mmio_atsd_regs[reg]);
> }
>
> -static int mmio_invalidate_pid(struct npu *npu, unsigned long pid, bool flush)
> +static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> + unsigned long pid, bool flush)
> {
> + int i;
> unsigned long launch;
>
> - /* IS set to invalidate matching PID */
> - launch = PPC_BIT(12);
> + for (i = 0; i <= max_npu2_index; i++) {
> + if (mmio_atsd_reg[i].reg < 0)
> + continue;
> +
> + /* IS set to invalidate matching PID */
> + launch = PPC_BIT(12);
>
> - /* PRS set to process-scoped */
> - launch |= PPC_BIT(13);
> + /* PRS set to process-scoped */
> + launch |= PPC_BIT(13);
>
> - /* AP */
> - launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
> + /* AP */
> + launch |= (u64)
> + mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
>
> - /* PID */
> - launch |= pid << PPC_BITLSHIFT(38);
> + /* PID */
> + launch |= pid << PPC_BITLSHIFT(38);
>
> - /* No flush */
> - launch |= !flush << PPC_BITLSHIFT(39);
> + /* No flush */
> + launch |= !flush << PPC_BITLSHIFT(39);
>
> - /* Invalidating the entire process doesn't use a va */
> - return mmio_launch_invalidate(npu, launch, 0);
> + /* Invalidating the entire process doesn't use a va */
> + mmio_launch_invalidate(&mmio_atsd_reg[i], launch, 0);
> + }
> }
>
> -static int mmio_invalidate_va(struct npu *npu, unsigned long va,
> - unsigned long pid, bool flush)
> +static void mmio_invalidate_va(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
> + unsigned long va, unsigned long pid, bool flush)
> {
> + int i;
> unsigned long launch;
>
> - /* IS set to invalidate target VA */
> - launch = 0;
> + for (i = 0; i <= max_npu2_index; i++) {
> + if (mmio_atsd_reg[i].reg < 0)
> + continue;
> +
> + /* IS set to invalidate target VA */
> + launch = 0;
>
> - /* PRS set to process scoped */
> - launch |= PPC_BIT(13);
> + /* PRS set to process scoped */
> + launch |= PPC_BIT(13);
>
> - /* AP */
> - launch |= (u64) mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
> + /* AP */
> + launch |= (u64)
> + mmu_get_ap(mmu_virtual_psize) << PPC_BITLSHIFT(17);
>
> - /* PID */
> - launch |= pid << PPC_BITLSHIFT(38);
> + /* PID */
> + launch |= pid << PPC_BITLSHIFT(38);
>
> - /* No flush */
> - launch |= !flush << PPC_BITLSHIFT(39);
> + /* No flush */
> + launch |= !flush << PPC_BITLSHIFT(39);
>
> - return mmio_launch_invalidate(npu, launch, va);
> + mmio_launch_invalidate(&mmio_atsd_reg[i], launch, va);
> + }
> }
>
> #define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
>
> -struct mmio_atsd_reg {
> - struct npu *npu;
> - int reg;
> -};
> -
> static void mmio_invalidate_wait(
> - struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS], bool flush)
> + struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> {
> struct npu *npu;
> int i, reg;
> @@ -522,16 +531,46 @@ static void mmio_invalidate_wait(
> reg = mmio_atsd_reg[i].reg;
> while (__raw_readq(npu->mmio_atsd_regs[reg] + XTS_ATSD_STAT))
> cpu_relax();
> + }
> +}
>
> - put_mmio_atsd_reg(npu, reg);
> +static void acquire_atsd_reg(struct npu_context *npu_context,
> + struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
> +{
> + int i, j;
> + struct npu *npu;
> + struct pci_dev *npdev;
> + struct pnv_phb *nphb;
>
> - /*
> - * The GPU requires two flush ATSDs to ensure all entries have
> - * been flushed. We use PID 0 as it will never be used for a
> - * process on the GPU.
> - */
> - if (flush)
> - mmio_invalidate_pid(npu, 0, true);
> + for (i = 0; i <= max_npu2_index; i++) {
> + mmio_atsd_reg[i].reg = -1;
> + for (j = 0; j < NV_MAX_LINKS; j++) {
> + npdev = READ_ONCE(npu_context->npdev[i][j]);
> + if (!npdev)
> + continue;
> +
> + nphb = pci_bus_to_host(npdev->bus)->private_data;
> + npu = &nphb->npu;
> + mmio_atsd_reg[i].npu = npu;
> + mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
> + while (mmio_atsd_reg[i].reg < 0) {
> + mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
> + cpu_relax();
I guess a softlockup will occur if we don't get the atsd resource in time
and that might aid debugging.
Looks good to me otherwise
Acked-by: Balbir Singh <bsingharora@gmail.com>
^ permalink raw reply
* [PATCH] selftests/vm: Update max va test to check for high address return.
From: Aneesh Kumar K.V @ 2018-02-28 3:58 UTC (permalink / raw)
To: akpm, Kirill A . Shutemov
Cc: linux-mm, linux-kernel, linuxppc-dev, Aneesh Kumar K.V
mmap(-1,..) is expected to search from max supported VA top down. It should find
an address above ADDR_SWITCH_HINT. Explicitly check for this.
Also derefer the address even if we failed the addr check.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
tools/testing/selftests/vm/va_128TBswitch.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/vm/va_128TBswitch.c b/tools/testing/selftests/vm/va_128TBswitch.c
index e7fe734c374f..f68fa4bd8179 100644
--- a/tools/testing/selftests/vm/va_128TBswitch.c
+++ b/tools/testing/selftests/vm/va_128TBswitch.c
@@ -44,6 +44,7 @@ struct testcase {
unsigned long flags;
const char *msg;
unsigned int low_addr_required:1;
+ unsigned int high_addr_required:1;
unsigned int keep_mapped:1;
};
@@ -108,6 +109,7 @@ static struct testcase testcases[] = {
.flags = MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(HIGH_ADDR)",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = HIGH_ADDR,
@@ -115,12 +117,14 @@ static struct testcase testcases[] = {
.flags = MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(HIGH_ADDR) again",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = HIGH_ADDR,
.size = 2 * PAGE_SIZE,
.flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
.msg = "mmap(HIGH_ADDR, MAP_FIXED)",
+ .high_addr_required = 1,
},
{
.addr = (void *) -1,
@@ -128,12 +132,14 @@ static struct testcase testcases[] = {
.flags = MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(-1)",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = (void *) -1,
.size = 2 * PAGE_SIZE,
.flags = MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(-1) again",
+ .high_addr_required = 1,
},
{
.addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
@@ -193,6 +199,7 @@ static struct testcase hugetlb_testcases[] = {
.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(HIGH_ADDR, MAP_HUGETLB)",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = HIGH_ADDR,
@@ -200,12 +207,14 @@ static struct testcase hugetlb_testcases[] = {
.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(HIGH_ADDR, MAP_HUGETLB) again",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = HIGH_ADDR,
.size = HUGETLB_SIZE,
.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
.msg = "mmap(HIGH_ADDR, MAP_FIXED | MAP_HUGETLB)",
+ .high_addr_required = 1,
},
{
.addr = (void *) -1,
@@ -213,12 +222,14 @@ static struct testcase hugetlb_testcases[] = {
.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(-1, MAP_HUGETLB)",
.keep_mapped = 1,
+ .high_addr_required = 1,
},
{
.addr = (void *) -1,
.size = HUGETLB_SIZE,
.flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
.msg = "mmap(-1, MAP_HUGETLB) again",
+ .high_addr_required = 1,
},
{
.addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE),
@@ -257,14 +268,16 @@ static int run_test(struct testcase *test, int count)
if (t->low_addr_required && p >= (void *)(ADDR_SWITCH_HINT)) {
printf("FAILED\n");
ret = 1;
- } else {
- /*
- * Do a dereference of the address returned so that we catch
- * bugs in page fault handling
- */
- memset(p, 0, t->size);
+ } else if (t->high_addr_required && p < (void *)(ADDR_SWITCH_HINT)) {
+ printf("FAILED\n");
+ ret = 1;
+ } else
printf("OK\n");
- }
+ /*
+ * Do a dereference of the address returned so that we catch
+ * bugs in page fault handling
+ */
+ memset(p, 0, t->size);
if (!t->keep_mapped)
munmap(p, t->size);
}
--
2.14.3
^ permalink raw reply related
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Nicholas Piggin @ 2018-02-28 6:53 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <878tbe7ggs.fsf@linux.vnet.ibm.com>
On Tue, 27 Feb 2018 18:11:07 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > On Tue, 27 Feb 2018 14:31:07 +0530
> > "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >
> >> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> >>
> >> > The number of high slices a process might use now depends on its
> >> > address space size, and what allocation address it has requested.
> >> >
> >> > This patch uses that limit throughout call chains where possible,
> >> > rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
> >> > This saves some cost for processes that don't use very large address
> >> > spaces.
> >>
> >> I haven't really looked at the final code. One of the issue we had was
> >> with the below scenario.
> >>
> >> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
> >> sure we build the mask such that we don't find the addr available.
> >
> > We should run it through the mmap regression tests. I *think* we moved
> > all of that logic from the slice code to get_ummapped_area before going
> > in to slices. I may have missed something though, it would be good to
> > have more eyes on it.
> >
>
> mmap(-1,...) failed with the test. Something like below fix it
>
> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
> mm->context.low_slices_psize = lpsizes;
>
> hpsizes = mm->context.high_slices_psize;
> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
> + high_slices = SLICE_NUM_HIGH;
> for (i = 0; i < high_slices; i++) {
> mask_index = i & 0x1;
> index = i >> 1;
>
> I guess for everything in the mm_context_t, we should compute it till
> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
> slice mask cached in mm_context on slb_addr_limit, it was still derived
> from the high_slices_psizes which was computed with lower value.
Okay thanks for catching that Aneesh. I guess that's a slow path so it
should be okay. Christophe if you're taking care of the series can you
fold it in? Otherwise I'll do that after yours gets merged.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Aneesh Kumar K.V @ 2018-02-28 6:59 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Christophe Leroy, linux-kernel, linuxppc-dev
In-Reply-To: <20180228165331.6e09959d@roar.ozlabs.ibm.com>
On 02/28/2018 12:23 PM, Nicholas Piggin wrote:
> On Tue, 27 Feb 2018 18:11:07 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>> Nicholas Piggin <npiggin@gmail.com> writes:
>>
>>> On Tue, 27 Feb 2018 14:31:07 +0530
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>>
>>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>>
>>>>> The number of high slices a process might use now depends on its
>>>>> address space size, and what allocation address it has requested.
>>>>>
>>>>> This patch uses that limit throughout call chains where possible,
>>>>> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
>>>>> This saves some cost for processes that don't use very large address
>>>>> spaces.
>>>>
>>>> I haven't really looked at the final code. One of the issue we had was
>>>> with the below scenario.
>>>>
>>>> mmap(addr, len) where addr < 128TB and addr+len > 128TB We want to make
>>>> sure we build the mask such that we don't find the addr available.
>>>
>>> We should run it through the mmap regression tests. I *think* we moved
>>> all of that logic from the slice code to get_ummapped_area before going
>>> in to slices. I may have missed something though, it would be good to
>>> have more eyes on it.
>>>
>>
>> mmap(-1,...) failed with the test. Something like below fix it
>>
>> @@ -756,7 +770,7 @@ void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
>> mm->context.low_slices_psize = lpsizes;
>>
>> hpsizes = mm->context.high_slices_psize;
>> - high_slices = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>> + high_slices = SLICE_NUM_HIGH;
>> for (i = 0; i < high_slices; i++) {
>> mask_index = i & 0x1;
>> index = i >> 1;
>>
>> I guess for everything in the mm_context_t, we should compute it till
>> SLICE_NUM_HIGH. The reason for failure was, even though we recompute the
>> slice mask cached in mm_context on slb_addr_limit, it was still derived
>> from the high_slices_psizes which was computed with lower value.
>
> Okay thanks for catching that Aneesh. I guess that's a slow path so it
> should be okay. Christophe if you're taking care of the series can you
> fold it in? Otherwise I'll do that after yours gets merged.
>
should we also compute the mm_context_t.slice_mask using SLICE_NUM_HIGH
and skip the recalc_slice_mask_cache when we change the addr limit?
-aneesh
^ permalink raw reply
* Re: [PATCH] KVM: surround kvm_arch_vcpu_async_ioctl() with #ifdef
From: Paolo Bonzini @ 2018-02-28 6:53 UTC (permalink / raw)
To: wei.guo.simon, kvm; +Cc: linux-s390, linuxppc-dev, linux-mips
In-Reply-To: <1519785099-13808-1-git-send-email-wei.guo.simon@gmail.com>
On 28/02/2018 03:31, wei.guo.simon@gmail.com wrote:
> From: Simon Guo <wei.guo.simon@gmail.com>
>
> Although CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL is usually on, logically
> kvm_arch_vcpu_async_ioctl() definition should be wrapped with
> CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL #ifdef.
No, the symbol is defined by Kconfig. It is a bug if it is not #defined.
Paolo
> This patch adds the #ifdef surround.
>
> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> ---
> arch/mips/kvm/mips.c | 2 ++
> arch/powerpc/kvm/powerpc.c | 2 ++
> arch/s390/kvm/kvm-s390.c | 2 ++
> 3 files changed, 6 insertions(+)
>
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 2549fdd..4d593e5 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -903,6 +903,7 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> return r;
> }
>
> +#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
> long kvm_arch_vcpu_async_ioctl(struct file *filp, unsigned int ioctl,
> unsigned long arg)
> {
> @@ -922,6 +923,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp, unsigned int ioctl,
>
> return -ENOIOCTLCMD;
> }
> +#endif
>
> long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
> unsigned long arg)
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 403e642..2adca3c 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -1757,6 +1757,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
> long kvm_arch_vcpu_async_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -1771,6 +1772,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
> }
> return -ENOIOCTLCMD;
> }
> +#endif
>
> long kvm_arch_vcpu_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 77d7818..c499396 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3784,6 +3784,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> return r;
> }
>
> +#ifdef CONFIG_HAVE_KVM_VCPU_ASYNC_IOCTL
> long kvm_arch_vcpu_async_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -3811,6 +3812,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
> }
> return -ENOIOCTLCMD;
> }
> +#endif
>
> long kvm_arch_vcpu_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
>
^ permalink raw reply
* Re: [RFC REBASED 5/5] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Aneesh Kumar K.V @ 2018-02-28 7:07 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <5badd882663833576c10b8aafe235fe1e443f119.1518382747.git.christophe.leroy@c-s.fr>
I also noticed that the slice mask printing use wrong variables now. I
guess this should take care of it
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index fef3f36b0b73..6b3575c39668 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -535,8 +535,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* already
*/
maskp = slice_mask_for_size(mm, psize);
- slice_print_mask(" good_mask", &good_mask);
-
/*
* Here "good" means slices that are already the right page size,
* "compat" means slices that have a compatible page size (i.e.
@@ -569,6 +567,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
{
slice_copy_mask(&good_mask, maskp, high_slices);
}
+ slice_print_mask(" good_mask", &good_mask);
+ slice_print_mask(" compat_mask", compat_maskp);
/* First check hint if it's valid or if we have MAP_FIXED */
if (addr || fixed) {
@@ -646,7 +646,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
slice_range_to_mask(addr, len, &potential_mask, high_slices);
slice_dbg(" found potential area at 0x%lx\n", addr);
- slice_print_mask(" mask", maskp);
+ slice_print_mask(" mask", &potential_mask);
convert:
slice_andnot_mask(&potential_mask, &potential_mask, &good_mask, high_slices);
@@ -836,13 +836,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
-
-#if 0 /* too verbose */
- slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
- mm, addr, len);
- slice_print_mask(" mask", &mask);
- slice_print_mask(" available", &available);
-#endif
return !slice_check_range_fits(mm, maskp, addr, len);
}
#endif
^ permalink raw reply related
* Re: [PATCH] selftests/vm: Update max va test to check for high address return.
From: Aneesh Kumar K.V @ 2018-02-28 8:08 UTC (permalink / raw)
To: akpm, Kirill A . Shutemov; +Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <20180228035830.10089-1-aneesh.kumar@linux.vnet.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> mmap(-1,..) is expected to search from max supported VA top down. It should find
> an address above ADDR_SWITCH_HINT. Explicitly check for this.
>
> Also derefer the address even if we failed the addr check.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
One issue I noticed is how to make this conditional so that we can still
run the test on x86 with 4 level page table?
> ---
> tools/testing/selftests/vm/va_128TBswitch.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/va_128TBswitch.c b/tools/testing/selftests/vm/va_128TBswitch.c
> index e7fe734c374f..f68fa4bd8179 100644
> --- a/tools/testing/selftests/vm/va_128TBswitch.c
> +++ b/tools/testing/selftests/vm/va_128TBswitch.c
> @@ -44,6 +44,7 @@ struct testcase {
> unsigned long flags;
> const char *msg;
> unsigned int low_addr_required:1;
> + unsigned int high_addr_required:1;
> unsigned int keep_mapped:1;
> };
>
> @@ -108,6 +109,7 @@ static struct testcase testcases[] = {
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR)",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> @@ -115,12 +117,14 @@ static struct testcase testcases[] = {
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR) again",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(HIGH_ADDR, MAP_FIXED)",
> + .high_addr_required = 1,
> },
> {
> .addr = (void *) -1,
> @@ -128,12 +132,14 @@ static struct testcase testcases[] = {
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1)",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = (void *) -1,
> .size = 2 * PAGE_SIZE,
> .flags = MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1) again",
> + .high_addr_required = 1,
> },
> {
> .addr = ((void *)(ADDR_SWITCH_HINT - PAGE_SIZE)),
> @@ -193,6 +199,7 @@ static struct testcase hugetlb_testcases[] = {
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR, MAP_HUGETLB)",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> @@ -200,12 +207,14 @@ static struct testcase hugetlb_testcases[] = {
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(HIGH_ADDR, MAP_HUGETLB) again",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = HIGH_ADDR,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
> .msg = "mmap(HIGH_ADDR, MAP_FIXED | MAP_HUGETLB)",
> + .high_addr_required = 1,
> },
> {
> .addr = (void *) -1,
> @@ -213,12 +222,14 @@ static struct testcase hugetlb_testcases[] = {
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1, MAP_HUGETLB)",
> .keep_mapped = 1,
> + .high_addr_required = 1,
> },
> {
> .addr = (void *) -1,
> .size = HUGETLB_SIZE,
> .flags = MAP_HUGETLB | MAP_PRIVATE | MAP_ANONYMOUS,
> .msg = "mmap(-1, MAP_HUGETLB) again",
> + .high_addr_required = 1,
> },
> {
> .addr = (void *)(ADDR_SWITCH_HINT - PAGE_SIZE),
> @@ -257,14 +268,16 @@ static int run_test(struct testcase *test, int count)
> if (t->low_addr_required && p >= (void *)(ADDR_SWITCH_HINT)) {
> printf("FAILED\n");
> ret = 1;
> - } else {
> - /*
> - * Do a dereference of the address returned so that we catch
> - * bugs in page fault handling
> - */
> - memset(p, 0, t->size);
> + } else if (t->high_addr_required && p < (void *)(ADDR_SWITCH_HINT)) {
> + printf("FAILED\n");
> + ret = 1;
> + } else
> printf("OK\n");
> - }
> + /*
> + * Do a dereference of the address returned so that we catch
> + * bugs in page fault handling
> + */
> + memset(p, 0, t->size);
> if (!t->keep_mapped)
> munmap(p, t->size);
> }
> --
> 2.14.3
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] selftests/powerpc: Fix missing clean of pmu/lib.o
From: Michael Ellerman @ 2018-02-28 9:32 UTC (permalink / raw)
To: linuxppc-dev
The tm-resched-dscr test links against pmu/lib.o, but we don't have a
rule to clean pmu/lib.o. This can lead to a build break if you build
for big endian and then little, or vice versa.
Fix it by making tm-resched-dscr depend on pmu/lib.c, causing the code
to be built directly in, meaning no .o is generated.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/tm/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index a23453943ad2..5c72ff978f27 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -16,7 +16,7 @@ $(OUTPUT)/tm-syscall: tm-syscall-asm.S
$(OUTPUT)/tm-syscall: CFLAGS += -I../../../../../usr/include
$(OUTPUT)/tm-tmspr: CFLAGS += -pthread
$(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64
-$(OUTPUT)/tm-resched-dscr: ../pmu/lib.o
+$(OUTPUT)/tm-resched-dscr: ../pmu/lib.c
$(OUTPUT)/tm-unavailable: CFLAGS += -O0 -pthread -m64 -Wno-error=uninitialized -mvsx
$(OUTPUT)/tm-trap: CFLAGS += -O0 -pthread -m64
--
2.14.1
^ permalink raw reply related
* Re: powerpc/powernv/mce: Don't silently restart the machine
From: Michael Ellerman @ 2018-02-28 9:49 UTC (permalink / raw)
To: Balbir Singh, linuxppc-dev; +Cc: npiggin, Balbir Singh
In-Reply-To: <20180228010636.22772-1-bsingharora@gmail.com>
Balbir Singh <bsingharora@gmail.com> writes:
> On MCE the current code will restart the machine with
> ppc_md.restart(). This case was extremely unlikely since
> prior to that a skiboot call is made and that resulted in
> a checkstop for analysis.
>
> With newer skiboots, on P9 we don't checkstop the box by
> default, instead we return back to the kernel to extract
> useful information at the time of the MCE. While we still
> get this information, this patch converts the restart to
> a panic(), so that if configured a dump can be taken and
> we can track and probably debug the potential issue causing
> the MCE.
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/powernv/opal.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 69b5263fc9e3..b510a6f41b00 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -500,9 +500,12 @@ void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
^^^^^^^^^^^^^^^
Why don't we use the msg ..
> * opal to trigger checkstop explicitly for error analysis.
> * The FSP PRD component would have already got notified
> * about this error through other channels.
> + * 4. We are running on a newer skiboot that by default does
> + * not cause a checkstop, drops us back to the kernel to
> + * extract context and state at the time of the error.
> */
>
> - ppc_md.restart(NULL);
> + panic("PowerNV Unrecovered Machine Check");
^
Here.
Because we can get here from a HMI so it's confusing to print "Machine
Check" in that case, and we have the msg already.
So just:
> + panic(msg);
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/npu-dma.c: Fix deadlock in mmio_invalidate
From: Michael Ellerman @ 2018-02-28 10:14 UTC (permalink / raw)
To: Alistair Popple, linuxppc-dev
Cc: Balbir Singh, Mark Hairgrove, Alistair Popple
In-Reply-To: <20180228003814.32124-1-alistair@popple.id.au>
Alistair Popple <alistair@popple.id.au> writes:
> When sending TLB invalidates to the NPU we need to send extra flushes due
> to a hardware issue. The original implementation would lock the all the
> ATSD MMIO registers sequentially before unlocking and relocking each of
> them sequentially to do the extra flush.
>
> This introduced a deadlock as it is possible for one thread to hold one
> ATSD register whilst waiting for another register to be freed while the
> other thread is holding that register waiting for the one in the first
> thread to be freed.
>
> For example if there are two threads and two ATSD registers:
>
> Thread A Thread B
> Acquire 1
> Acquire 2
> Release 1 Acquire 1
> Wait 1 Wait 2
>
> Both threads will be stuck waiting to acquire a register resulting in an
> RCU stall warning or soft lockup.
>
> This patch solves the deadlock by refactoring the code to ensure registers
> are not released between flushes and to ensure all registers are either
> acquired or released together and in order.
>
> Fixes: bbd5ff50afff ("powerpc/powernv/npu-dma: Add explicit flush when sending an ATSD")
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
>
> ---
>
> v2: Added memory barriers around ->npdev[] and ATSD register aquisition/release
Apologies to Balbir who was standing nearby when I read this patch this
afternoon and copped a bit of a rant as a result ...
But READ_ONCE/WRITE_ONCE are not memory barriers, they are only compiler
barriers.
So the READ_ONCE/WRITE_ONCE usage in here may be necessary, but it's
probably not sufficient, we probably *also* need actual memory barriers.
But I could be wrong, my main gripe is that the locking/ordering in here
is not very obvious.
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index 0a253b64ac5f..2fed4b116e19 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -726,7 +749,7 @@ struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
> if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
> &nvlink_index)))
> return ERR_PTR(-ENODEV);
> - npu_context->npdev[npu->index][nvlink_index] = npdev;
> + WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
When you're publishing a struct via a pointer you would typically have a
barrier between the stores that set the fields of the struct, and the
store that publishes the struct. Otherwise the reader can see a
partially setup struct.
I think here the npdev was setup somewhere else, and maybe there has
been an intervening barrier, but it's not clear. A comment at least
would be good.
In general I feel like a spinlock or two could significantly simply the
locking/ordering in this code, and given we're doing MMIOs anyway would
not affect performance.
</rant>
cheers
^ permalink raw reply
* Re: [PATCH V2] powerpc: Don't do runtime futex_cmpxchg test
From: Michael Ellerman @ 2018-02-28 10:44 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180226140950.12111-1-aneesh.kumar@linux.vnet.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> futex_detect_cmpxchg() does a cmpxchg_futex_value_locked on a NULL user addr to
> runtime detect whether architecture implements atomic cmpxchg for futex. POWER
> do implement the feature and hence we can enable the config instead of depending
> on runtime detection.
>
> We could possible enable this on everything. For now limitted to book3s_64
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/Kconfig.cputype | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index a429d859f15d..31bc2bd5dfd1 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -75,6 +75,7 @@ config PPC_BOOK3S_64
> select ARCH_SUPPORTS_NUMA_BALANCING
> select IRQ_WORK
> select HAVE_KERNEL_XZ
> + select HAVE_FUTEX_CMPXCHG if FUTEX
I'd rather it was under config PPC with an if BOOK3S_64 check. Just so
that all those top-level style configs are in once place.
eg.
config PPC
...
select HAVE_FUTEX_CMPXCHG if FUTEX && PPC_BOOK3S_64
cheers
^ permalink raw reply
* Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice
From: Michael Ellerman @ 2018-02-28 10:45 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Benjamin Herrenschmidt, Paul Mackerras, linux-pm,
Vincent Guittot, linuxppc-dev
In-Reply-To: <20180227034527.GQ26947@vireshk-i7>
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 26-02-18, 22:53, Michael Ellerman wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>> > Subject: Re: [PATCH 15/27] cpufreq: powerenv: Don't validate the frequency table twice
>> ^
>> powernv
>>
>> > The cpufreq core is already validating the CPU frequency table after
>> > calling the ->init() callback of the cpufreq drivers and the drivers
>> > don't need to do the same anymore. Though they need to set the
>> > policy->freq_table field directly from the ->init() callback now.
>> >
>> > Stop validating the frequency table from powerenv driver.
>
> powernv :)
>
> Will fix both of them.
Thanks :)
cheers
^ permalink raw reply
* Re: powerpc/powernv/mce: Don't silently restart the machine
From: Balbir Singh @ 2018-02-28 10:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Nicholas Piggin
In-Reply-To: <87a7vt4f6b.fsf@concordia.ellerman.id.au>
On Wed, Feb 28, 2018 at 8:49 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Balbir Singh <bsingharora@gmail.com> writes:
>
>> On MCE the current code will restart the machine with
>> ppc_md.restart(). This case was extremely unlikely since
>> prior to that a skiboot call is made and that resulted in
>> a checkstop for analysis.
>>
>> With newer skiboots, on P9 we don't checkstop the box by
>> default, instead we return back to the kernel to extract
>> useful information at the time of the MCE. While we still
>> get this information, this patch converts the restart to
>> a panic(), so that if configured a dump can be taken and
>> we can track and probably debug the potential issue causing
>> the MCE.
>>
>> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/platforms/powernv/opal.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index 69b5263fc9e3..b510a6f41b00 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -500,9 +500,12 @@ void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
> ^^^^^^^^^^^^^^^
> Why don't we use the msg ..
>
>> * opal to trigger checkstop explicitly for error analysis.
>> * The FSP PRD component would have already got notified
>> * about this error through other channels.
>> + * 4. We are running on a newer skiboot that by default does
>> + * not cause a checkstop, drops us back to the kernel to
>> + * extract context and state at the time of the error.
>> */
>>
>> - ppc_md.restart(NULL);
>> + panic("PowerNV Unrecovered Machine Check");
> ^
> Here.
>
> Because we can get here from a HMI so it's confusing to print "Machine
> Check" in that case, and we have the msg already.
>
> So just:
>
>> + panic(msg);
>
My bad, we used to have two of these one in opal and opal-hmi and the
diff from the previous change showed this message. Resending
Thanks,
Balbir
^ permalink raw reply
* Re: [PATCH v2] powerpc/npu: Cleanup MMIO ATSD flushing
From: Michael Ellerman @ 2018-02-28 10:50 UTC (permalink / raw)
To: Alistair Popple, linuxppc-dev; +Cc: Balbir Singh, Aneesh Kumar KV
In-Reply-To: <2478842.lxjFpUTe5s@new-mexico>
Alistair Popple <alistair@popple.id.au> writes:
>> @aneesh can you please look at this? @mpe can we pick this up if there
>> are no objections?
>
> @mpe any objections to picking this up for this release? Or do you want to wait
> for the next one? (there are likely more bugfixes coming for ATS support).
I can grab it for this release if it's fixing a bug, the subject makes
it sound like a cleanup. Do we have a Fixes tag for it?
I don't particularly like the potentially very long loop in
pnv_npu2_invalidate_helper() which spends most of its time with
interrupts off. I worry we're going to see soft lockups pointing there
if we get a very large invalidate.
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox