* [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Aneesh Kumar K.V @ 2014-06-05 12:08 UTC (permalink / raw)
To: agraf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc, Aneesh Kumar K.V
In-Reply-To: <1401970085-14493-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
virtual time base register is a per VM, per cpu register that needs
to be saved and restored on vm exit and entry. Writing to VTB is not
allowed in the privileged mode.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/include/asm/reg.h | 15 +++++++++++++++
arch/powerpc/include/asm/time.h | 9 +++++++++
arch/powerpc/kvm/book3s.c | 6 ++++++
arch/powerpc/kvm/book3s_emulate.c | 3 +++
arch/powerpc/kvm/book3s_hv.c | 6 ------
arch/powerpc/kvm/book3s_pr.c | 3 ++-
7 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 4a58731a0a72..bd3caeaeebe1 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -505,6 +505,7 @@ struct kvm_vcpu_arch {
#endif
/* Time base value when we entered the guest */
u64 entry_tb;
+ u64 entry_vtb;
u32 tcr;
ulong tsr; /* we need to perform set/clr_bits() which requires ulong */
u32 ivor[64];
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 4852bcf270f3..3e7085d8af90 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -25,6 +25,7 @@
#ifdef CONFIG_8xx
#include <asm/reg_8xx.h>
#endif /* CONFIG_8xx */
+#include <asm/bug.h>
#define MSR_SF_LG 63 /* Enable 64 bit mode */
#define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */
@@ -1193,6 +1194,20 @@
: "r" ((unsigned long)(v)) \
: "memory")
+static inline unsigned long mfvtb (void)
+{
+#ifdef CONFIG_PPC_BOOK3S_64
+ if (cpu_has_feature(CPU_FTR_ARCH_207S))
+ return mfspr(SPRN_VTB);
+#endif
+ /*
+ * The above mfspr will be a no-op on anything before Power8
+ * That can result in random values returned. We need to
+ * capture that.
+ */
+ BUG();
+}
+
#ifdef __powerpc64__
#if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
#define mftb() ({unsigned long rval; \
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 1d428e6007ca..03cbada59d3a 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -102,6 +102,15 @@ static inline u64 get_rtc(void)
return (u64)hi * 1000000000 + lo;
}
+static inline u64 get_vtb(void)
+{
+#ifdef CONFIG_PPC_BOOK3S_64
+ if (cpu_has_feature(CPU_FTR_ARCH_207S))
+ return mfvtb();
+#endif
+ return 0;
+}
+
#ifdef CONFIG_PPC64
static inline u64 get_tb(void)
{
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 52c654dbd41a..ae43e4178ecd 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -646,6 +646,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
case KVM_REG_PPC_BESCR:
val = get_reg_val(reg->id, vcpu->arch.bescr);
break;
+ case KVM_REG_PPC_VTB:
+ val = get_reg_val(reg->id, vcpu->arch.vtb);
+ break;
default:
r = -EINVAL;
break;
@@ -750,6 +753,9 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
case KVM_REG_PPC_BESCR:
vcpu->arch.bescr = set_reg_val(reg->id, val);
break;
+ case KVM_REG_PPC_VTB:
+ vcpu->arch.vtb = set_reg_val(reg->id, val);
+ break;
default:
r = -EINVAL;
break;
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 3565e775b61b..1bb16a59dcbc 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
*/
*spr_val = vcpu->arch.spurr;
break;
+ case SPRN_VTB:
+ *spr_val = vcpu->arch.vtb;
+ break;
case SPRN_GQR0:
case SPRN_GQR1:
case SPRN_GQR2:
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index aba05bbb3e74..f6ac58336b3f 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -897,9 +897,6 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_IC:
*val = get_reg_val(id, vcpu->arch.ic);
break;
- case KVM_REG_PPC_VTB:
- *val = get_reg_val(id, vcpu->arch.vtb);
- break;
case KVM_REG_PPC_CSIGR:
*val = get_reg_val(id, vcpu->arch.csigr);
break;
@@ -1097,9 +1094,6 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_IC:
vcpu->arch.ic = set_reg_val(id, *val);
break;
- case KVM_REG_PPC_VTB:
- vcpu->arch.vtb = set_reg_val(id, *val);
- break;
case KVM_REG_PPC_CSIGR:
vcpu->arch.csigr = set_reg_val(id, *val);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index ff15f8e31a27..96cdf89a8c86 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -125,6 +125,7 @@ void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
* to find the guest purr and spurr value.
*/
vcpu->arch.entry_tb = get_tb();
+ vcpu->arch.entry_vtb = get_vtb();
svcpu->in_use = true;
}
@@ -176,7 +177,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
*/
vcpu->arch.purr += get_tb() - vcpu->arch.entry_tb;
vcpu->arch.spurr += get_tb() - vcpu->arch.entry_tb;
-
+ vcpu->arch.vtb += get_vtb() - vcpu->arch.entry_vtb;
svcpu->in_use = false;
out:
--
1.9.1
^ permalink raw reply related
* [PATCH 3/4] KVM: PPC: BOOK3S: PR: Emulate DPDES register
From: Aneesh Kumar K.V @ 2014-06-05 12:08 UTC (permalink / raw)
To: agraf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc, Aneesh Kumar K.V
In-Reply-To: <1401970085-14493-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Since we don't support SMT yet, we should always find zero in
Directed privileged doorbell exception state register.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/kvm/book3s_emulate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index d6c87d085182..062b5da7786e 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -655,6 +655,7 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
case SPRN_MMCR1:
case SPRN_MMCR2:
case SPRN_TIR:
+ case SPRN_DPDES:
#endif
*spr_val = 0;
break;
--
1.9.1
^ permalink raw reply related
* [PATCH 4/4] KVM: PPC: BOOK3S: PR: Emulate instruction counter
From: Aneesh Kumar K.V @ 2014-06-05 12:08 UTC (permalink / raw)
To: agraf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc, Aneesh Kumar K.V
In-Reply-To: <1401970085-14493-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Writing to IC is not allowed in the privileged mode.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/kvm/book3s.c | 6 ++++++
arch/powerpc/kvm/book3s_emulate.c | 3 +++
arch/powerpc/kvm/book3s_hv.c | 6 ------
arch/powerpc/kvm/book3s_pr.c | 4 ++++
5 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index bd3caeaeebe1..f9ae69682ce1 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -506,6 +506,7 @@ struct kvm_vcpu_arch {
/* Time base value when we entered the guest */
u64 entry_tb;
u64 entry_vtb;
+ u64 entry_ic;
u32 tcr;
ulong tsr; /* we need to perform set/clr_bits() which requires ulong */
u32 ivor[64];
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index ae43e4178ecd..52c4c43900cb 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -649,6 +649,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
case KVM_REG_PPC_VTB:
val = get_reg_val(reg->id, vcpu->arch.vtb);
break;
+ case KVM_REG_PPC_IC:
+ val = get_reg_val(reg->id, vcpu->arch.ic);
+ break;
default:
r = -EINVAL;
break;
@@ -756,6 +759,9 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
case KVM_REG_PPC_VTB:
vcpu->arch.vtb = set_reg_val(reg->id, val);
break;
+ case KVM_REG_PPC_IC:
+ vcpu->arch.ic = set_reg_val(reg->id, val);
+ break;
default:
r = -EINVAL;
break;
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
index 062b5da7786e..e6912c618160 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -598,6 +598,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
case SPRN_VTB:
*spr_val = vcpu->arch.vtb;
break;
+ case SPRN_IC:
+ *spr_val = vcpu->arch.ic;
+ break;
case SPRN_GQR0:
case SPRN_GQR1:
case SPRN_GQR2:
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index f6ac58336b3f..c38cf9f836c0 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -894,9 +894,6 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_CIABR:
*val = get_reg_val(id, vcpu->arch.ciabr);
break;
- case KVM_REG_PPC_IC:
- *val = get_reg_val(id, vcpu->arch.ic);
- break;
case KVM_REG_PPC_CSIGR:
*val = get_reg_val(id, vcpu->arch.csigr);
break;
@@ -1091,9 +1088,6 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
if ((vcpu->arch.ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
vcpu->arch.ciabr &= ~CIABR_PRIV; /* disable */
break;
- case KVM_REG_PPC_IC:
- vcpu->arch.ic = set_reg_val(id, *val);
- break;
case KVM_REG_PPC_CSIGR:
vcpu->arch.csigr = set_reg_val(id, *val);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 96cdf89a8c86..03fc8847cd67 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -126,6 +126,8 @@ void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
*/
vcpu->arch.entry_tb = get_tb();
vcpu->arch.entry_vtb = get_vtb();
+ if (cpu_has_feature(CPU_FTR_ARCH_207S))
+ vcpu->arch.entry_ic = mfspr(SPRN_IC);
svcpu->in_use = true;
}
@@ -178,6 +180,8 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
vcpu->arch.purr += get_tb() - vcpu->arch.entry_tb;
vcpu->arch.spurr += get_tb() - vcpu->arch.entry_tb;
vcpu->arch.vtb += get_vtb() - vcpu->arch.entry_vtb;
+ if (cpu_has_feature(CPU_FTR_ARCH_207S))
+ vcpu->arch.ic += mfspr(SPRN_IC) - vcpu->arch.entry_ic;
svcpu->in_use = false;
out:
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Alexander Graf @ 2014-06-05 12:19 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1401970085-14493-2-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 05.06.14 14:08, Aneesh Kumar K.V wrote:
> virtual time base register is a per VM, per cpu register that needs
> to be saved and restored on vm exit and entry. Writing to VTB is not
> allowed in the privileged mode.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_host.h | 1 +
> arch/powerpc/include/asm/reg.h | 15 +++++++++++++++
> arch/powerpc/include/asm/time.h | 9 +++++++++
> arch/powerpc/kvm/book3s.c | 6 ++++++
> arch/powerpc/kvm/book3s_emulate.c | 3 +++
> arch/powerpc/kvm/book3s_hv.c | 6 ------
> arch/powerpc/kvm/book3s_pr.c | 3 ++-
> 7 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 4a58731a0a72..bd3caeaeebe1 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -505,6 +505,7 @@ struct kvm_vcpu_arch {
> #endif
> /* Time base value when we entered the guest */
> u64 entry_tb;
> + u64 entry_vtb;
> u32 tcr;
> ulong tsr; /* we need to perform set/clr_bits() which requires ulong */
> u32 ivor[64];
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 4852bcf270f3..3e7085d8af90 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -25,6 +25,7 @@
> #ifdef CONFIG_8xx
> #include <asm/reg_8xx.h>
> #endif /* CONFIG_8xx */
> +#include <asm/bug.h>
>
> #define MSR_SF_LG 63 /* Enable 64 bit mode */
> #define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */
> @@ -1193,6 +1194,20 @@
> : "r" ((unsigned long)(v)) \
> : "memory")
>
> +static inline unsigned long mfvtb (void)
> +{
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
> + return mfspr(SPRN_VTB);
> +#endif
> + /*
> + * The above mfspr will be a no-op on anything before Power8
> + * That can result in random values returned. We need to
> + * capture that.
> + */
> + BUG();
> +}
> +
> #ifdef __powerpc64__
> #if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
> #define mftb() ({unsigned long rval; \
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 1d428e6007ca..03cbada59d3a 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -102,6 +102,15 @@ static inline u64 get_rtc(void)
> return (u64)hi * 1000000000 + lo;
> }
>
> +static inline u64 get_vtb(void)
> +{
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
> + return mfvtb();
> +#endif
> + return 0;
> +}
> +
> #ifdef CONFIG_PPC64
> static inline u64 get_tb(void)
> {
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 52c654dbd41a..ae43e4178ecd 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -646,6 +646,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> case KVM_REG_PPC_BESCR:
> val = get_reg_val(reg->id, vcpu->arch.bescr);
> break;
> + case KVM_REG_PPC_VTB:
> + val = get_reg_val(reg->id, vcpu->arch.vtb);
> + break;
> default:
> r = -EINVAL;
> break;
> @@ -750,6 +753,9 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> case KVM_REG_PPC_BESCR:
> vcpu->arch.bescr = set_reg_val(reg->id, val);
> break;
> + case KVM_REG_PPC_VTB:
> + vcpu->arch.vtb = set_reg_val(reg->id, val);
> + break;
> default:
> r = -EINVAL;
> break;
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index 3565e775b61b..1bb16a59dcbc 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
> */
> *spr_val = vcpu->arch.spurr;
> break;
> + case SPRN_VTB:
> + *spr_val = vcpu->arch.vtb;
Doesn't this mean that vtb can be the same 2 when the guest reads it 2
times in a row without getting preempted?
Alex
> + break;
> case SPRN_GQR0:
> case SPRN_GQR1:
> case SPRN_GQR2:
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index aba05bbb3e74..f6ac58336b3f 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -897,9 +897,6 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
> case KVM_REG_PPC_IC:
> *val = get_reg_val(id, vcpu->arch.ic);
> break;
> - case KVM_REG_PPC_VTB:
> - *val = get_reg_val(id, vcpu->arch.vtb);
> - break;
> case KVM_REG_PPC_CSIGR:
> *val = get_reg_val(id, vcpu->arch.csigr);
> break;
> @@ -1097,9 +1094,6 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
> case KVM_REG_PPC_IC:
> vcpu->arch.ic = set_reg_val(id, *val);
> break;
> - case KVM_REG_PPC_VTB:
> - vcpu->arch.vtb = set_reg_val(id, *val);
> - break;
> case KVM_REG_PPC_CSIGR:
> vcpu->arch.csigr = set_reg_val(id, *val);
> break;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index ff15f8e31a27..96cdf89a8c86 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -125,6 +125,7 @@ void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
> * to find the guest purr and spurr value.
> */
> vcpu->arch.entry_tb = get_tb();
> + vcpu->arch.entry_vtb = get_vtb();
> svcpu->in_use = true;
> }
>
> @@ -176,7 +177,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
> */
> vcpu->arch.purr += get_tb() - vcpu->arch.entry_tb;
> vcpu->arch.spurr += get_tb() - vcpu->arch.entry_tb;
> -
> + vcpu->arch.vtb += get_vtb() - vcpu->arch.entry_vtb;
> svcpu->in_use = false;
>
> out:
^ permalink raw reply
* Re: [PATCH 2/4] KVM: PPC: BOOK3S: PR: Doorbell support
From: Alexander Graf @ 2014-06-05 12:21 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1401970085-14493-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 05.06.14 14:08, Aneesh Kumar K.V wrote:
> We don't have SMT support yet, hence we should not find a doorbell
> message generated
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/kvm/book3s_emulate.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
> index 1bb16a59dcbc..d6c87d085182 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -28,7 +28,9 @@
> #define OP_19_XOP_RFI 50
>
> #define OP_31_XOP_MFMSR 83
> +#define OP_31_XOP_MSGSNDP 142
> #define OP_31_XOP_MTMSR 146
> +#define OP_31_XOP_MSGCLRP 174
> #define OP_31_XOP_MTMSRD 178
> #define OP_31_XOP_MTSR 210
> #define OP_31_XOP_MTSRIN 242
> @@ -303,6 +305,22 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
>
> break;
> }
> + case OP_31_XOP_MSGSNDP:
> + {
> + /*
> + * PR KVM still don't support SMT mode. So we should
still?
> + * not see a MSGSNDP/MSGCLRP used with PR KVM
> + */
> + pr_info("KVM: MSGSNDP used in non SMT case\n");
> + emulated = EMULATE_FAIL;
What would happen on an HV guest with only 1 thread that MSGSNDs to
thread 0? Would the guest get an illegal instruction trap, a
self-interrupt or would this be a simple nop?
Alex
> + break;
> + }
> + case OP_31_XOP_MSGCLRP:
> + {
> + pr_info("KVM: MSGCLRP used in non SMT case\n");
> + emulated = EMULATE_FAIL;
> + break;
> + }
> default:
> emulated = EMULATE_FAIL;
> }
^ permalink raw reply
* Re: [PATCH 2/4] KVM: PPC: BOOK3S: PR: Doorbell support
From: Alexander Graf @ 2014-06-05 12:23 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <539060B5.9040808@suse.de>
On 05.06.14 14:21, Alexander Graf wrote:
>
> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>> We don't have SMT support yet, hence we should not find a doorbell
>> message generated
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s_emulate.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_emulate.c
>> b/arch/powerpc/kvm/book3s_emulate.c
>> index 1bb16a59dcbc..d6c87d085182 100644
>> --- a/arch/powerpc/kvm/book3s_emulate.c
>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>> @@ -28,7 +28,9 @@
>> #define OP_19_XOP_RFI 50
>> #define OP_31_XOP_MFMSR 83
>> +#define OP_31_XOP_MSGSNDP 142
>> #define OP_31_XOP_MTMSR 146
>> +#define OP_31_XOP_MSGCLRP 174
>> #define OP_31_XOP_MTMSRD 178
>> #define OP_31_XOP_MTSR 210
>> #define OP_31_XOP_MTSRIN 242
>> @@ -303,6 +305,22 @@ int kvmppc_core_emulate_op_pr(struct kvm_run
>> *run, struct kvm_vcpu *vcpu,
>> break;
>> }
>> + case OP_31_XOP_MSGSNDP:
>> + {
>> + /*
>> + * PR KVM still don't support SMT mode. So we should
>
> still?
>
>> + * not see a MSGSNDP/MSGCLRP used with PR KVM
>> + */
>> + pr_info("KVM: MSGSNDP used in non SMT case\n");
>> + emulated = EMULATE_FAIL;
>
> What would happen on an HV guest with only 1 thread that MSGSNDs to
> thread 0? Would the guest get an illegal instruction trap, a
> self-interrupt or would this be a simple nop?
What I'm trying to say here is that it's ok to treat it as illegal
instructions, but then we don't need this patch :).
Alex
^ permalink raw reply
* Re: [PATCH 3/3] PPC: KVM: Add support for 64bit TCE windows
From: Benjamin Herrenschmidt @ 2014-06-05 12:30 UTC (permalink / raw)
To: Alexander Graf
Cc: kvm, Alexey Kardashevskiy, linux-kernel, kvm-ppc, Gleb Natapov,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <53905ADB.8000100@suse.de>
On Thu, 2014-06-05 at 13:56 +0200, Alexander Graf wrote:
> What if we ask user space to give us a pointer to user space allocated
> memory along with the TCE registration? We would still ask user space to
> only use the returned fd for TCE modifications, but would have some
> nicely swappable memory we can store the TCE entries in.
That isn't going to work terribly well for VFIO :-) But yes, for
emulated devices, we could improve things a bit, including for
the 32-bit TCE tables.
For emulated, the real mode path could walk the page tables and fallback
to virtual mode & get_user if the page isn't present, thus operating
directly on qemu memory TCE tables instead of the current pinned stuff.
However that has a cost in performance, but since that's really only
used for emulated devices and PAPR VIOs, it might not be a huge issue.
But for VFIO we don't have much choice, we need to create something the
HW can access.
> In fact, the code as is today can allocate an arbitrary amount of pinned
> kernel memory from within user space without any checks.
Right. We should at least account it in the locked limit.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 3/3] PPC: KVM: Add support for 64bit TCE windows
From: Alexander Graf @ 2014-06-05 12:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, Alexey Kardashevskiy, linux-kernel, kvm-ppc, Gleb Natapov,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <1401971411.3247.132.camel@pasglop>
On 05.06.14 14:30, Benjamin Herrenschmidt wrote:
> On Thu, 2014-06-05 at 13:56 +0200, Alexander Graf wrote:
>> What if we ask user space to give us a pointer to user space allocated
>> memory along with the TCE registration? We would still ask user space to
>> only use the returned fd for TCE modifications, but would have some
>> nicely swappable memory we can store the TCE entries in.
> That isn't going to work terribly well for VFIO :-) But yes, for
> emulated devices, we could improve things a bit, including for
> the 32-bit TCE tables.
>
> For emulated, the real mode path could walk the page tables and fallback
> to virtual mode & get_user if the page isn't present, thus operating
> directly on qemu memory TCE tables instead of the current pinned stuff.
>
> However that has a cost in performance, but since that's really only
> used for emulated devices and PAPR VIOs, it might not be a huge issue.
>
> But for VFIO we don't have much choice, we need to create something the
> HW can access.
But we need to create separate tables for VFIO anyways, because these
TCE tables contain virtual addresses, no?
Alex
>
>> In fact, the code as is today can allocate an arbitrary amount of pinned
>> kernel memory from within user space without any checks.
> Right. We should at least account it in the locked limit.
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [PATCH v8 0/3] EEH Support for VFIO PCI Device
From: Alexander Graf @ 2014-06-05 12:54 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc, linuxppc-dev; +Cc: aik, alex.williamson, qiudayu
In-Reply-To: <1401950169-30206-1-git-send-email-gwshan@linux.vnet.ibm.com>
On 05.06.14 08:36, Gavin Shan wrote:
> The series of patches adds support EEH for PCI devices, which are passed
> through to PowerKVM based guest via VFIO. The implementation is straightforward
> based on the issues or problems we have to resolve to support EEH for PowerKVM
> based guest.
>
> - Emulation for EEH RTAS requests. All EEH RTAS requests goes to QEMU firstly.
> If QEMU can't handle it, the request will be sent to host via newly introduced
> VFIO container IOCTL command (VFIO_EEH_OP) and gets handled in host kernel.
>
> The series of patches requires corresponding QEMU changes.
Acked-by: Alexander Graf <agraf@suse.de>
Alex
^ permalink raw reply
* Re: [PATCH 3/3] PPC: KVM: Add support for 64bit TCE windows
From: Alexey Kardashevskiy @ 2014-06-05 13:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Alexander Graf
Cc: kvm, Gleb Natapov, linux-kernel, kvm-ppc, Paul Mackerras,
Paolo Bonzini, linuxppc-dev
In-Reply-To: <1401971411.3247.132.camel@pasglop>
On 06/05/2014 10:30 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-06-05 at 13:56 +0200, Alexander Graf wrote:
>> What if we ask user space to give us a pointer to user space allocated
>> memory along with the TCE registration? We would still ask user space to
>> only use the returned fd for TCE modifications, but would have some
>> nicely swappable memory we can store the TCE entries in.
>
> That isn't going to work terribly well for VFIO :-) But yes, for
> emulated devices, we could improve things a bit, including for
> the 32-bit TCE tables.
>
> For emulated, the real mode path could walk the page tables and fallback
> to virtual mode & get_user if the page isn't present, thus operating
> directly on qemu memory TCE tables instead of the current pinned stuff.
>
> However that has a cost in performance, but since that's really only
> used for emulated devices and PAPR VIOs, it might not be a huge issue.
>
> But for VFIO we don't have much choice, we need to create something the
> HW can access.
You are confusing things here.
There are 2 tables:
1. guest-visible TCE table, this is what is allocated for VIO or emulated PCI;
2. real HW DMA window, one exists already for DMA32 and one I will
allocated for a huge window.
I have just #2 for VFIO now but we will need both in order to implement
H_GET_TCE correctly, and this is the table I will allocate by this new ioctl.
>> In fact, the code as is today can allocate an arbitrary amount of pinned
>> kernel memory from within user space without any checks.
>
> Right. We should at least account it in the locked limit.
Yup. And (probably) this thing will keep a counter of how many windows were
created per KVM instance to avoid having multiple copies of the same table.
--
Alexey
^ permalink raw reply
* [PATCH 1/2] powerpc/powernv: include asm/smp.h to handle UP config
From: Shreyas B. Prabhu @ 2014-06-05 15:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras
Cc: Geert Uytterhoeven, linuxppc-dev, linux-kernel, Shreyas B. Prabhu
Build throws following errors when CONFIG_SMP=n
arch/powerpc/platforms/powernv/setup.c: In function ‘pnv_kexec_wait_secondaries_down’:
arch/powerpc/platforms/powernv/setup.c:179:4: error: implicit declaration of function ‘get_hard_smp_processor_id’
rc = opal_query_cpu_status(get_hard_smp_processor_id(i),
The usage of get_hard_smp_processor_id() needs the declaration from
<asm/smp.h>. The file setup.c includes <linux/sched.h>, which in-turn
includes <linux/smp.h>. However, <linux/smp.h> includes <asm/smp.h>
only on SMP configs and hence UP builds fail.
Fix this by directly including <asm/smp.h> in setup.c unconditionally.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 8c16a5f..678573c 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -35,6 +35,7 @@
#include <asm/rtas.h>
#include <asm/opal.h>
#include <asm/kexec.h>
+#include <asm/smp.h>
#include "powernv.h"
--
1.9.0
^ permalink raw reply related
* [PATCH 2/2] powerpc/powernv : Disable subcore for UP configs
From: Shreyas B. Prabhu @ 2014-06-05 15:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Shreyas B. Prabhu, linuxppc-dev, linux-kernel
In-Reply-To: <1401981695-3682-1-git-send-email-shreyas@linux.vnet.ibm.com>
Build throws following errors when CONFIG_SMP=n
arch/powerpc/platforms/powernv/subcore.c: In function ‘cpu_update_split_mode’:
arch/powerpc/platforms/powernv/subcore.c:274:15: error: ‘setup_max_cpus’ undeclared (first use in this function)
arch/powerpc/platforms/powernv/subcore.c:285:5: error: lvalue required as left operand of assignment
'setup_max_cpus' variable is relevant only on SMP, so there is no point
working around it for UP. Furthermore, subcore.c itself is relevant only
on SMP and hence the better solution is to exclude subcore.c for UP builds.
Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
---
This patch applies on top of ben/powerpc.git/next branch
arch/powerpc/platforms/powernv/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 4ad0d34..636d206 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,9 +1,9 @@
obj-y += setup.o opal-takeover.o opal-wrappers.o opal.o opal-async.o
obj-y += opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
obj-y += rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
-obj-y += opal-msglog.o subcore.o subcore-asm.o
+obj-y += opal-msglog.o subcore-asm.o
-obj-$(CONFIG_SMP) += smp.o
+obj-$(CONFIG_SMP) += smp.o subcore.o
obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o
obj-$(CONFIG_EEH) += eeh-ioda.o eeh-powernv.o
obj-$(CONFIG_PPC_SCOM) += opal-xscom.o
--
1.9.0
^ permalink raw reply related
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Aneesh Kumar K.V @ 2014-06-05 15:50 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <5390604C.4050704@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>> virtual time base register is a per VM, per cpu register that needs
>> to be saved and restored on vm exit and entry. Writing to VTB is not
>> allowed in the privileged mode.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/kvm_host.h | 1 +
>> arch/powerpc/include/asm/reg.h | 15 +++++++++++++++
>> arch/powerpc/include/asm/time.h | 9 +++++++++
>> arch/powerpc/kvm/book3s.c | 6 ++++++
>> arch/powerpc/kvm/book3s_emulate.c | 3 +++
>> arch/powerpc/kvm/book3s_hv.c | 6 ------
>> arch/powerpc/kvm/book3s_pr.c | 3 ++-
>> 7 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 4a58731a0a72..bd3caeaeebe1 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -505,6 +505,7 @@ struct kvm_vcpu_arch {
>> #endif
>> /* Time base value when we entered the guest */
>> u64 entry_tb;
>> + u64 entry_vtb;
>> u32 tcr;
>> ulong tsr; /* we need to perform set/clr_bits() which requires ulong */
>> u32 ivor[64];
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 4852bcf270f3..3e7085d8af90 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -25,6 +25,7 @@
>> #ifdef CONFIG_8xx
>> #include <asm/reg_8xx.h>
>> #endif /* CONFIG_8xx */
>> +#include <asm/bug.h>
>>
>> #define MSR_SF_LG 63 /* Enable 64 bit mode */
>> #define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */
>> @@ -1193,6 +1194,20 @@
>> : "r" ((unsigned long)(v)) \
>> : "memory")
>>
>> +static inline unsigned long mfvtb (void)
>> +{
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> + return mfspr(SPRN_VTB);
>> +#endif
>> + /*
>> + * The above mfspr will be a no-op on anything before Power8
>> + * That can result in random values returned. We need to
>> + * capture that.
>> + */
>> + BUG();
>> +}
>> +
>> #ifdef __powerpc64__
>> #if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
>> #define mftb() ({unsigned long rval; \
>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>> index 1d428e6007ca..03cbada59d3a 100644
>> --- a/arch/powerpc/include/asm/time.h
>> +++ b/arch/powerpc/include/asm/time.h
>> @@ -102,6 +102,15 @@ static inline u64 get_rtc(void)
>> return (u64)hi * 1000000000 + lo;
>> }
>>
>> +static inline u64 get_vtb(void)
>> +{
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> + return mfvtb();
>> +#endif
>> + return 0;
>> +}
>> +
>> #ifdef CONFIG_PPC64
>> static inline u64 get_tb(void)
>> {
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index 52c654dbd41a..ae43e4178ecd 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -646,6 +646,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
>> case KVM_REG_PPC_BESCR:
>> val = get_reg_val(reg->id, vcpu->arch.bescr);
>> break;
>> + case KVM_REG_PPC_VTB:
>> + val = get_reg_val(reg->id, vcpu->arch.vtb);
>> + break;
>> default:
>> r = -EINVAL;
>> break;
>> @@ -750,6 +753,9 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
>> case KVM_REG_PPC_BESCR:
>> vcpu->arch.bescr = set_reg_val(reg->id, val);
>> break;
>> + case KVM_REG_PPC_VTB:
>> + vcpu->arch.vtb = set_reg_val(reg->id, val);
>> + break;
>> default:
>> r = -EINVAL;
>> break;
>> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
>> index 3565e775b61b..1bb16a59dcbc 100644
>> --- a/arch/powerpc/kvm/book3s_emulate.c
>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
>> */
>> *spr_val = vcpu->arch.spurr;
>> break;
>> + case SPRN_VTB:
>> + *spr_val = vcpu->arch.vtb;
>
> Doesn't this mean that vtb can be the same 2 when the guest reads it 2
> times in a row without getting preempted?
But a mfspr will result in VM exit and that would make sure we
update vcpu->arch.vtb with the correct value.
-aneesh
^ permalink raw reply
* Re: [PATCH 2/4] KVM: PPC: BOOK3S: PR: Doorbell support
From: Aneesh Kumar K.V @ 2014-06-05 15:55 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <53906156.4090403@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 05.06.14 14:21, Alexander Graf wrote:
>>
>> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>>> We don't have SMT support yet, hence we should not find a doorbell
>>> message generated
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/kvm/book3s_emulate.c | 18 ++++++++++++++++++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c
>>> b/arch/powerpc/kvm/book3s_emulate.c
>>> index 1bb16a59dcbc..d6c87d085182 100644
>>> --- a/arch/powerpc/kvm/book3s_emulate.c
>>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>>> @@ -28,7 +28,9 @@
>>> #define OP_19_XOP_RFI 50
>>> #define OP_31_XOP_MFMSR 83
>>> +#define OP_31_XOP_MSGSNDP 142
>>> #define OP_31_XOP_MTMSR 146
>>> +#define OP_31_XOP_MSGCLRP 174
>>> #define OP_31_XOP_MTMSRD 178
>>> #define OP_31_XOP_MTSR 210
>>> #define OP_31_XOP_MTSRIN 242
>>> @@ -303,6 +305,22 @@ int kvmppc_core_emulate_op_pr(struct kvm_run
>>> *run, struct kvm_vcpu *vcpu,
>>> break;
>>> }
>>> + case OP_31_XOP_MSGSNDP:
>>> + {
>>> + /*
>>> + * PR KVM still don't support SMT mode. So we should
>>
>> still?
>>
>>> + * not see a MSGSNDP/MSGCLRP used with PR KVM
>>> + */
>>> + pr_info("KVM: MSGSNDP used in non SMT case\n");
>>> + emulated = EMULATE_FAIL;
>>
>> What would happen on an HV guest with only 1 thread that MSGSNDs to
>> thread 0? Would the guest get an illegal instruction trap, a
>> self-interrupt or would this be a simple nop?
>
> What I'm trying to say here is that it's ok to treat it as illegal
> instructions, but then we don't need this patch :).
>
Agreed. I will verify whether it is treated as a nop. If so will send an
updated patch.
-aneesh
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Alexander Graf @ 2014-06-05 16:53 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <87wqcvmk3f.fsf@linux.vnet.ibm.com>
On 05.06.14 17:50, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>>> virtual time base register is a per VM, per cpu register that needs
>>> to be saved and restored on vm exit and entry. Writing to VTB is not
>>> allowed in the privileged mode.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/include/asm/kvm_host.h | 1 +
>>> arch/powerpc/include/asm/reg.h | 15 +++++++++++++++
>>> arch/powerpc/include/asm/time.h | 9 +++++++++
>>> arch/powerpc/kvm/book3s.c | 6 ++++++
>>> arch/powerpc/kvm/book3s_emulate.c | 3 +++
>>> arch/powerpc/kvm/book3s_hv.c | 6 ------
>>> arch/powerpc/kvm/book3s_pr.c | 3 ++-
>>> 7 files changed, 36 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>>> index 4a58731a0a72..bd3caeaeebe1 100644
>>> --- a/arch/powerpc/include/asm/kvm_host.h
>>> +++ b/arch/powerpc/include/asm/kvm_host.h
>>> @@ -505,6 +505,7 @@ struct kvm_vcpu_arch {
>>> #endif
>>> /* Time base value when we entered the guest */
>>> u64 entry_tb;
>>> + u64 entry_vtb;
>>> u32 tcr;
>>> ulong tsr; /* we need to perform set/clr_bits() which requires ulong */
>>> u32 ivor[64];
>>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>>> index 4852bcf270f3..3e7085d8af90 100644
>>> --- a/arch/powerpc/include/asm/reg.h
>>> +++ b/arch/powerpc/include/asm/reg.h
>>> @@ -25,6 +25,7 @@
>>> #ifdef CONFIG_8xx
>>> #include <asm/reg_8xx.h>
>>> #endif /* CONFIG_8xx */
>>> +#include <asm/bug.h>
>>>
>>> #define MSR_SF_LG 63 /* Enable 64 bit mode */
>>> #define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */
>>> @@ -1193,6 +1194,20 @@
>>> : "r" ((unsigned long)(v)) \
>>> : "memory")
>>>
>>> +static inline unsigned long mfvtb (void)
>>> +{
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>> + return mfspr(SPRN_VTB);
>>> +#endif
>>> + /*
>>> + * The above mfspr will be a no-op on anything before Power8
>>> + * That can result in random values returned. We need to
>>> + * capture that.
>>> + */
>>> + BUG();
>>> +}
>>> +
>>> #ifdef __powerpc64__
>>> #if defined(CONFIG_PPC_CELL) || defined(CONFIG_PPC_FSL_BOOK3E)
>>> #define mftb() ({unsigned long rval; \
>>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>>> index 1d428e6007ca..03cbada59d3a 100644
>>> --- a/arch/powerpc/include/asm/time.h
>>> +++ b/arch/powerpc/include/asm/time.h
>>> @@ -102,6 +102,15 @@ static inline u64 get_rtc(void)
>>> return (u64)hi * 1000000000 + lo;
>>> }
>>>
>>> +static inline u64 get_vtb(void)
>>> +{
>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>> + return mfvtb();
>>> +#endif
>>> + return 0;
>>> +}
>>> +
>>> #ifdef CONFIG_PPC64
>>> static inline u64 get_tb(void)
>>> {
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index 52c654dbd41a..ae43e4178ecd 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -646,6 +646,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
>>> case KVM_REG_PPC_BESCR:
>>> val = get_reg_val(reg->id, vcpu->arch.bescr);
>>> break;
>>> + case KVM_REG_PPC_VTB:
>>> + val = get_reg_val(reg->id, vcpu->arch.vtb);
>>> + break;
>>> default:
>>> r = -EINVAL;
>>> break;
>>> @@ -750,6 +753,9 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
>>> case KVM_REG_PPC_BESCR:
>>> vcpu->arch.bescr = set_reg_val(reg->id, val);
>>> break;
>>> + case KVM_REG_PPC_VTB:
>>> + vcpu->arch.vtb = set_reg_val(reg->id, val);
>>> + break;
>>> default:
>>> r = -EINVAL;
>>> break;
>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
>>> index 3565e775b61b..1bb16a59dcbc 100644
>>> --- a/arch/powerpc/kvm/book3s_emulate.c
>>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>>> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
>>> */
>>> *spr_val = vcpu->arch.spurr;
>>> break;
>>> + case SPRN_VTB:
>>> + *spr_val = vcpu->arch.vtb;
>> Doesn't this mean that vtb can be the same 2 when the guest reads it 2
>> times in a row without getting preempted?
>
> But a mfspr will result in VM exit and that would make sure we
> update vcpu->arch.vtb with the correct value.
We only call kvmppc_core_vcpu_put_pr() when we context switch away from
KVM, so it won't be updated, no?
Alex
^ permalink raw reply
* Re: [RFC PATCH 3/3] PPC, KVM, CMA: use general CMA reserved area management framework
From: Aneesh Kumar K.V @ 2014-06-05 17:00 UTC (permalink / raw)
To: Paolo Bonzini, Michal Nazarewicz, Joonsoo Kim, Andrew Morton,
Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Alexander Graf, kvm-ppc, linux-kernel, Minchan Kim,
Paul Mackerras, Greg Kroah-Hartman, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <538D9363.7020203@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> Il 03/06/2014 09:02, Michal Nazarewicz ha scritto:
>> On Tue, Jun 03 2014, Joonsoo Kim wrote:
>>> Now, we have general CMA reserved area management framework,
>>> so use it for future maintainabilty. There is no functional change.
>>>
>>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>
>> Acked-by: Michal Nazarewicz <mina86@mina86.com>
>>
>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Aneesh, can you test this series?
Sorry for the late reply. I will test this and update here.
-aneesh
^ permalink raw reply
* Re: [PATCH v8 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-06-05 17:18 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, agraf, kvm-ppc, qiudayu, linuxppc-dev
In-Reply-To: <1401950169-30206-4-git-send-email-gwshan@linux.vnet.ibm.com>
On Thu, 2014-06-05 at 16:36 +1000, Gavin Shan wrote:
> The patch adds new IOCTL commands for sPAPR VFIO container device
> to support EEH functionality for PCI devices, which have been passed
> through from host to somebody else via VFIO.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> Documentation/vfio.txt | 87 ++++++++++++++++++++++++++++++++++--
> drivers/vfio/Makefile | 1 +
> drivers/vfio/pci/vfio_pci.c | 20 ++++++---
> drivers/vfio/vfio_iommu_spapr_tce.c | 17 ++++++-
> drivers/vfio/vfio_spapr_eeh.c | 89 +++++++++++++++++++++++++++++++++++++
> include/linux/vfio.h | 23 ++++++++++
> include/uapi/linux/vfio.h | 35 +++++++++++++++
> 7 files changed, 262 insertions(+), 10 deletions(-)
> create mode 100644 drivers/vfio/vfio_spapr_eeh.c
>
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index b9ca023..3fa4538 100644
> --- a/Documentation/vfio.txt
> +++ b/Documentation/vfio.txt
> @@ -305,7 +305,15 @@ faster, the map/unmap handling has been implemented in real mode which provides
> an excellent performance which has limitations such as inability to do
> locked pages accounting in real time.
>
> -So 3 additional ioctls have been added:
> +4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
> +subtree that can be treated as a unit for the purposes of partitioning and
> +error recovery. A PE may be a single or multi-function IOA (IO Adapter), a
> +function of a multi-function IOA, or multiple IOAs (possibly including switch
> +and bridge structures above the multiple IOAs). PPC64 guests detect PCI errors
> +and recover from them via EEH RTAS services, which works on the basis of
> +additional ioctl commands.
> +
> +So 4 additional ioctls have been added:
>
> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
> of the DMA window on the PCI bus.
> @@ -316,9 +324,12 @@ So 3 additional ioctls have been added:
>
> VFIO_IOMMU_DISABLE - disables the container.
>
> + VFIO_EEH_PE_OP - provides an API for EEH setup, error detection and recovery.
>
> The code flow from the example above should be slightly changed:
>
> + struct vfio_eeh_pe_op pe_op = { .argsz = sizeof(pe_op) };
> +
> .....
> /* Add the group to the container */
> ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
> @@ -342,9 +353,79 @@ The code flow from the example above should be slightly changed:
> dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
>
> /* Check here is .iova/.size are within DMA window from spapr_iommu_info */
> -
> ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
> - .....
> +
> + /* Get a file descriptor for the device */
> + device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
> +
> + ....
> +
> + /* Gratuitous device reset and go... */
> + ioctl(device, VFIO_DEVICE_RESET);
> +
> + /* Make sure EEH is supported */
> + ioctl(container, VFIO_CHECK_EXTENSION, VFIO_EEH);
> +
> + /* Enable the EEH functionality on the device */
> + pe_op.op = VFIO_EEH_PE_ENABLE;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /* You're suggested to create additional data struct to represent
> + * PE, and put child devices belonging to same IOMMU group to the
> + * PE instance for later reference.
> + */
> +
> + /* Check the PE's state and make sure it's in functional state */
> + pe_op.op = VFIO_EEH_PE_GET_STATE;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /* Save device state using pci_save_state().
> + * EEH should be enabled on the specified device.
> + */
> +
> + ....
> +
> + /* When 0xFF's returned from reading PCI config space or IO BARs
> + * of the PCI device. Check the PE's state to see if that has been
> + * frozen.
> + */
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /* Waiting for pending PCI transactions to be completed and don't
> + * produce any more PCI traffic from/to the affected PE until
> + * recovery is finished.
> + */
> +
> + /* Enable IO for the affected PE and collect logs. Usually, the
> + * standard part of PCI config space, AER registers are dumped
> + * as logs for further analysis.
> + */
> + pe_op.op = VFIO_EEH_PE_UNFREEZE_IO;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /*
> + * Issue PE reset: hot or fundamental reset. Usually, hot reset
> + * is enough. However, the firmware of some PCI adapters would
> + * require fundamental reset.
> + */
> + pe_op.op = VFIO_EEH_PE_RESET_HOT;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> + pe_op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /* Configure the PCI bridges for the affected PE */
> + pe_op.op = VFIO_EEH_PE_CONFIGURE;
> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
> +
> + /* Restored state we saved at initialization time. pci_restore_state()
> + * is good enough as an example.
> + */
> +
> + /* Hopefully, error is recovered successfully. Now, you can resume to
> + * start PCI traffic to/from the affected PE.
> + */
> +
> + ....
>
> -------------------------------------------------------------------------------
>
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 72bfabc..50e30bc 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_VFIO) += vfio.o
> obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
> +obj-$(CONFIG_EEH) += vfio_spapr_eeh.o
> obj-$(CONFIG_VFIO_PCI) += pci/
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 7ba0424..26f289d 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -156,8 +156,10 @@ static void vfio_pci_release(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
>
> - if (atomic_dec_and_test(&vdev->refcnt))
> + if (atomic_dec_and_test(&vdev->refcnt)) {
> + vfio_spapr_pci_eeh_release(vdev->pdev);
> vfio_pci_disable(vdev);
> + }
>
> module_put(THIS_MODULE);
> }
> @@ -165,19 +167,25 @@ static void vfio_pci_release(void *device_data)
> static int vfio_pci_open(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
> + int ret;
>
> if (!try_module_get(THIS_MODULE))
> return -ENODEV;
>
> if (atomic_inc_return(&vdev->refcnt) == 1) {
> - int ret = vfio_pci_enable(vdev);
> - if (ret) {
> - module_put(THIS_MODULE);
> - return ret;
> - }
> + ret = vfio_pci_enable(vdev);
> + if (ret)
> + goto error;
> +
> + ret = vfio_spapr_pci_eeh_open(vdev->pdev);
> + if (ret)
> + goto error;
Missing cleanup of vfio_pci_enable() on error
> }
>
> return 0;
> +error:
> + module_put(THIS_MODULE);
> + return ret;
> }
>
> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index a84788b..730b4ef 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -156,7 +156,16 @@ static long tce_iommu_ioctl(void *iommu_data,
>
> switch (cmd) {
> case VFIO_CHECK_EXTENSION:
> - return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
> + switch (arg) {
> + case VFIO_SPAPR_TCE_IOMMU:
> + ret = 1;
> + break;
> + default:
> + ret = vfio_spapr_iommu_eeh_ioctl(NULL, cmd, arg);
> + break;
> + }
> +
> + return (ret < 0) ? 0 : ret;
>
> case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
> struct vfio_iommu_spapr_tce_info info;
> @@ -283,6 +292,12 @@ static long tce_iommu_ioctl(void *iommu_data,
> tce_iommu_disable(container);
> mutex_unlock(&container->lock);
> return 0;
> + case VFIO_EEH_PE_OP:
> + if (!container->tbl || !container->tbl->it_group)
> + return -ENODEV;
> +
> + return vfio_spapr_iommu_eeh_ioctl(container->tbl->it_group,
> + cmd, arg);
> }
>
> return -ENOTTY;
> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
> new file mode 100644
> index 0000000..691f8d8
> --- /dev/null
> +++ b/drivers/vfio/vfio_spapr_eeh.c
> @@ -0,0 +1,89 @@
> +/*
> + * EEH functionality support for VFIO devices. The feature is only
> + * available on sPAPR compatible platforms.
> + *
> + * Copyright Gavin Shan, IBM Corporation 2014.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/iommu.h>
> +#include <linux/pci.h>
Hmm, doesn't seem like we need iommu.h & pci.h just to pass through a
pointer. Can these be removed?
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +#include <asm/eeh.h>
> +
> +/* We might build address mapping here for "fast" path later */
> +int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
> +{
> + return eeh_dev_open(pdev);
> +}
> +
> +void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
> +{
> + eeh_dev_release(pdev);
> +}
> +
> +long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
> + unsigned int cmd, unsigned long arg)
> +{
> + struct eeh_pe *pe;
> + struct vfio_eeh_pe_op op;
> + unsigned long minsz;
> + long ret = -EINVAL;
> +
> + switch (cmd) {
> + case VFIO_CHECK_EXTENSION:
> + if (arg == VFIO_EEH)
> + ret = eeh_enabled() ? 1 : 0;
> + else
> + ret = 0;
> + break;
> + case VFIO_EEH_PE_OP:
> + pe = eeh_iommu_group_to_pe(group);
> + if (!pe)
> + return -ENODEV;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_op, op);
> + if (copy_from_user(&op, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (op.argsz < minsz)
> + return -EINVAL;
> +
> + switch (op.op) {
> + case VFIO_EEH_PE_DISABLE:
> + ret = eeh_pe_set_option(pe, EEH_OPT_DISABLE);
> + break;
> + case VFIO_EEH_PE_ENABLE:
> + ret = eeh_pe_set_option(pe, EEH_OPT_ENABLE);
> + break;
> + case VFIO_EEH_PE_UNFREEZE_IO:
> + ret = eeh_pe_set_option(pe, EEH_OPT_THAW_MMIO);
> + break;
> + case VFIO_EEH_PE_UNFREEZE_DMA:
> + ret = eeh_pe_set_option(pe, EEH_OPT_THAW_DMA);
> + break;
> + case VFIO_EEH_PE_GET_STATE:
> + ret = eeh_pe_get_state(pe);
> + break;
> + case VFIO_EEH_PE_RESET_DEACTIVATE:
> + ret = eeh_pe_reset(pe, EEH_RESET_DEACTIVATE);
> + break;
> + case VFIO_EEH_PE_RESET_HOT:
> + ret = eeh_pe_reset(pe, EEH_RESET_HOT);
> + break;
> + case VFIO_EEH_PE_RESET_FUNDAMENTAL:
> + ret = eeh_pe_reset(pe, EEH_RESET_FUNDAMENTAL);
> + break;
> + case VFIO_EEH_PE_CONFIGURE:
> + ret = eeh_pe_configure(pe);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + }
> +
> + return ret;
> +}
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 81022a52..0d3bb8f 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -99,4 +99,27 @@ extern int vfio_external_user_iommu_id(struct vfio_group *group);
> extern long vfio_external_check_extension(struct vfio_group *group,
> unsigned long arg);
>
> +#ifdef CONFIG_EEH
> +extern int vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
> +extern void vfio_spapr_pci_eeh_release(struct pci_dev *pdev);
> +extern long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
> + unsigned int cmd,
> + unsigned long arg);
> +#else
> +static inline int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +
> +static inline void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
> +{
> +}
> +
> +static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
> + unsigned int cmd,
> + unsigned long arg)
> +{
> + return -ENOTTY;
> +}
> +#endif /* CONFIG_EEH */
> #endif /* VFIO_H */
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index cb9023d..b93c619 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -30,6 +30,9 @@
> */
> #define VFIO_DMA_CC_IOMMU 4
>
> +/* Check if EEH is supported */
> +#define VFIO_EEH 5
> +
> /*
> * The IOCTL interface is designed for extensibility by embedding the
> * structure length (argsz) and flags into structures passed between
> @@ -455,6 +458,38 @@ struct vfio_iommu_spapr_tce_info {
>
> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>
> +/*
> + * EEH PE operation struct provides ways to:
> + * - enable/disable EEH functionality;
> + * - unfreeze IO/DMA for frozen PE;
> + * - read PE state;
> + * - reset PE;
> + * - configure PE.
> + */
> +struct vfio_eeh_pe_op {
> + __u32 argsz;
> + __u32 flags;
> + __u32 op;
> +};
> +
> +#define VFIO_EEH_PE_DISABLE 0 /* Disable EEH functionality */
> +#define VFIO_EEH_PE_ENABLE 1 /* Enable EEH functionality */
> +#define VFIO_EEH_PE_UNFREEZE_IO 2 /* Enable IO for frozen PE */
> +#define VFIO_EEH_PE_UNFREEZE_DMA 3 /* Enable DMA for frozen PE */
> +#define VFIO_EEH_PE_GET_STATE 4 /* PE state retrieval */
> +#define VFIO_EEH_PE_RESET_DEACTIVATE 5 /* Deassert PE reset */
> +#define VFIO_EEH_PE_RESET_HOT 6 /* Assert hot reset */
> +#define VFIO_EEH_PE_RESET_FUNDAMENTAL 7 /* Assert fundamental reset */
> +#define VFIO_EEH_PE_CONFIGURE 8 /* PE configuration */
> +
> +#define VFIO_EEH_PE_STATE_NORMAL 0 /* PE in functional state */
> +#define VFIO_EEH_PE_STATE_RESET 1 /* PE reset in progress */
> +#define VFIO_EEH_PE_STATE_STOPPED 2 /* Stopped DMA and IO */
> +#define VFIO_EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA only */
> +#define VFIO_EEH_PE_STATE_UNAVAIL 5 /* State unavailable */
Please make it explicit that these are return values for
VFIO_EEH_PE_GET_STATE. They could be defined inline after
VFIO_EEH_PE_GET_STATE with indenting to make it clear. Thanks,
Alex
> +
> +#define VFIO_EEH_PE_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
> +
> /* ***************************************************************** */
>
> #endif /* _UAPIVFIO_H */
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Aneesh Kumar K.V @ 2014-06-05 17:33 UTC (permalink / raw)
To: Alexander Graf, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <5390A071.9040006@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 05.06.14 17:50, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>>>> virtual time base register is a per VM, per cpu register that needs
>>>> to be saved and restored on vm exit and entry. Writing to VTB is not
>>>> allowed in the privileged mode.
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
.......
>>>> break;
>>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
>>>> index 3565e775b61b..1bb16a59dcbc 100644
>>>> --- a/arch/powerpc/kvm/book3s_emulate.c
>>>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>>>> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
>>>> */
>>>> *spr_val = vcpu->arch.spurr;
>>>> break;
>>>> + case SPRN_VTB:
>>>> + *spr_val = vcpu->arch.vtb;
>>> Doesn't this mean that vtb can be the same 2 when the guest reads it 2
>>> times in a row without getting preempted?
>>
>> But a mfspr will result in VM exit and that would make sure we
>> update vcpu->arch.vtb with the correct value.
>
> We only call kvmppc_core_vcpu_put_pr() when we context switch away from
> KVM, so it won't be updated, no?
>
>
kvmppc_copy_from_svcpu is also called from VM exit path (book3s_interrupt.S)
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 1/3] CMA: generalize CMA reserved area management functionality
From: Aneesh Kumar K.V @ 2014-06-05 17:39 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Marek Szyprowski, Michal Nazarewicz
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1401757919-30018-2-git-send-email-iamjoonsoo.kim@lge.com>
Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
> Currently, there are two users on CMA functionality, one is the DMA
> subsystem and the other is the kvm on powerpc. They have their own code
> to manage CMA reserved area even if they looks really similar.
> From my guess, it is caused by some needs on bitmap management. Kvm side
> wants to maintain bitmap not for 1 page, but for more size. Eventually it
> use bitmap where one bit represents 64 pages.
>
> When I implement CMA related patches, I should change those two places
> to apply my change and it seem to be painful to me. I want to change
> this situation and reduce future code management overhead through
> this patch.
>
> This change could also help developer who want to use CMA in their
> new feature development, since they can use CMA easily without
> copying & pasting this reserved area management code.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
The way patches are split makes it difficult to review. Will it be
possible to make changes against one implementation and them move that
to generic code. That helps in finding out what exactly changed.
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 00e13ce..b3fe1cc 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -283,7 +283,7 @@ config CMA_ALIGNMENT
>
> If unsure, leave the default value "8".
>
> -config CMA_AREAS
> +config DMA_CMA_AREAS
> int "Maximum count of the CMA device-private areas"
> default 7
> help
for ex: The above can be a seperate patch along with further DMA related
cleanups .
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 83969f8..48cdac8 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -186,7 +186,7 @@ static int __init cma_activate_area(struct cma *cma)
> return 0;
> }
>
> -static struct cma cma_areas[MAX_CMA_AREAS];
> +static struct cma cma_areas[MAX_DMA_CMA_AREAS];
> static unsigned cma_area_count;
>
> static int __init cma_init_reserved_areas(void)
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> new file mode 100644
> index 0000000..60ba06f
> --- /dev/null
> +++ b/include/linux/cma.h
> @@ -0,0 +1,28 @@
> +/*
> + * Contiguous Memory Allocator
> + *
> + * Copyright LG Electronics Inc., 2014
> + * Written by:
> + * Joonsoo Kim <iamjoonsoo.kim@lge.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License or (at your optional) any later version of the license.
> + *
> + */
> +
> +#ifndef __CMA_H__
> +#define __CMA_H__
> +
> +struct cma;
> +
> +extern struct page *cma_alloc(struct cma *cma, unsigned long count,
> + unsigned long align);
> +extern bool cma_release(struct cma *cma, struct page *pages,
> + unsigned long count);
> +extern int __init cma_declare_contiguous(phys_addr_t size, phys_addr_t base,
> + phys_addr_t limit, phys_addr_t alignment,
> + unsigned long bitmap_shift, bool fixed,
> + struct cma **res_cma);
> +#endif
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index 772eab5..dfb1dc9 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -63,7 +63,7 @@ struct device;
> * There is always at least global CMA area and a few optional device
> * private areas configured in kernel .config.
> */
> -#define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)
> +#define MAX_DMA_CMA_AREAS (1 + CONFIG_DMA_CMA_AREAS)
>
> extern struct cma *dma_contiguous_default_area;
>
> @@ -123,7 +123,7 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages,
>
> #else
>
> -#define MAX_CMA_AREAS (0)
> +#define MAX_DMA_CMA_AREAS (0)
>
> static inline struct cma *dev_get_cma_area(struct device *dev)
> {
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 7511b4a..0877ddc 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -515,6 +515,17 @@ config CMA_DEBUG
> processing calls such as dma_alloc_from_contiguous().
> This option does not affect warning and error messages.
>
> +config CMA_AREAS
> + int "Maximum count of the CMA areas"
> + depends on CMA
> + default 7
> + help
> + CMA allows to create CMA areas for particular purpose, mainly,
> + used as device private area. This parameter sets the maximum
> + number of CMA area in the system.
> +
> + If unsure, leave the default value "7".
> +
If we have 7 declare continugous request, a kvm cma allocation request will fail ?
> config ZBUD
> tristate
> default n
> diff --git a/mm/Makefile b/mm/Makefile
> index 1eaa70b..bc0422b 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -62,3 +62,4 @@ obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
> obj-$(CONFIG_ZBUD) += zbud.o
> obj-$(CONFIG_ZSMALLOC) += zsmalloc.o
> obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
> +obj-$(CONFIG_CMA) += cma.o
> diff --git a/mm/cma.c b/mm/cma.c
> new file mode 100644
> index 0000000..0dae88d
> --- /dev/null
> +++ b/mm/cma.c
> @@ -0,0 +1,329 @@
> +/*
> + * Contiguous Memory Allocator
> + *
> + * Copyright (c) 2010-2011 by Samsung Electronics.
> + * Copyright IBM Corporation, 2013
> + * Copyright LG Electronics Inc., 2014
> + * Written by:
> + * Marek Szyprowski <m.szyprowski@samsung.com>
> + * Michal Nazarewicz <mina86@mina86.com>
> + * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> + * Joonsoo Kim <iamjoonsoo.kim@lge.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License or (at your optional) any later version of the license.
> + */
> +
> +#define pr_fmt(fmt) "cma: " fmt
> +
> +#ifdef CONFIG_CMA_DEBUG
> +#ifndef DEBUG
> +# define DEBUG
> +#endif
> +#endif
> +
> +#include <linux/memblock.h>
> +#include <linux/err.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.h>
> +
> +struct cma {
> + unsigned long base_pfn;
> + unsigned long count;
> + unsigned long *bitmap;
> + unsigned long bitmap_shift;
I guess this is added to accommodate the kvm specific alloc chunks. May
be you should do this as a patch against kvm implementation and then
move the code to generic ?
> + struct mutex lock;
> +};
> +
-aneesh
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Alexander Graf @ 2014-06-05 22:32 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <87sinjqn1g.fsf@linux.vnet.ibm.com>
On 05.06.14 19:33, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 05.06.14 17:50, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.de> writes:
>>>
>>>> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>>>>> virtual time base register is a per VM, per cpu register that needs
>>>>> to be saved and restored on vm exit and entry. Writing to VTB is not
>>>>> allowed in the privileged mode.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> .......
>
>>>>> break;
>>>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
>>>>> index 3565e775b61b..1bb16a59dcbc 100644
>>>>> --- a/arch/powerpc/kvm/book3s_emulate.c
>>>>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>>>>> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val
>>>>> */
>>>>> *spr_val = vcpu->arch.spurr;
>>>>> break;
>>>>> + case SPRN_VTB:
>>>>> + *spr_val = vcpu->arch.vtb;
>>>> Doesn't this mean that vtb can be the same 2 when the guest reads it 2
>>>> times in a row without getting preempted?
>>> But a mfspr will result in VM exit and that would make sure we
>>> update vcpu->arch.vtb with the correct value.
>> We only call kvmppc_core_vcpu_put_pr() when we context switch away from
>> KVM, so it won't be updated, no?
>>
>>
> kvmppc_copy_from_svcpu is also called from VM exit path (book3s_interrupt.S)
... where it will run into this code path:
/*
* Maybe we were already preempted and synced the svcpu from
* our preempt notifiers. Don't bother touching this svcpu then.
*/
if (!svcpu->in_use)
goto out;
Alex
^ permalink raw reply
* Re: [PATCH 1/4] KVM: PPC: BOOK3S: PR: Emulate virtual timebase register
From: Alexander Graf @ 2014-06-05 22:36 UTC (permalink / raw)
To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <5390EFF6.50504@suse.de>
On 06.06.14 00:32, Alexander Graf wrote:
>
> On 05.06.14 19:33, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 05.06.14 17:50, Aneesh Kumar K.V wrote:
>>>> Alexander Graf <agraf@suse.de> writes:
>>>>
>>>>> On 05.06.14 14:08, Aneesh Kumar K.V wrote:
>>>>>> virtual time base register is a per VM, per cpu register that needs
>>>>>> to be saved and restored on vm exit and entry. Writing to VTB is not
>>>>>> allowed in the privileged mode.
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> .......
>>
>>>>>> break;
>>>>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c
>>>>>> b/arch/powerpc/kvm/book3s_emulate.c
>>>>>> index 3565e775b61b..1bb16a59dcbc 100644
>>>>>> --- a/arch/powerpc/kvm/book3s_emulate.c
>>>>>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>>>>>> @@ -577,6 +577,9 @@ int kvmppc_core_emulate_mfspr_pr(struct
>>>>>> kvm_vcpu *vcpu, int sprn, ulong *spr_val
>>>>>> */
>>>>>> *spr_val = vcpu->arch.spurr;
>>>>>> break;
>>>>>> + case SPRN_VTB:
>>>>>> + *spr_val = vcpu->arch.vtb;
>>>>> Doesn't this mean that vtb can be the same 2 when the guest reads
>>>>> it 2
>>>>> times in a row without getting preempted?
>>>> But a mfspr will result in VM exit and that would make sure we
>>>> update vcpu->arch.vtb with the correct value.
>>> We only call kvmppc_core_vcpu_put_pr() when we context switch away from
>>> KVM, so it won't be updated, no?
>>>
>>>
>> kvmppc_copy_from_svcpu is also called from VM exit path
>> (book3s_interrupt.S)
>
> ... where it will run into this code path:
>
> /*
> * Maybe we were already preempted and synced the svcpu from
> * our preempt notifiers. Don't bother touching this svcpu then.
> */
> if (!svcpu->in_use)
> goto out;
Scratch that. We're always calling this on entry/exit, so you're right.
Alex
^ permalink raw reply
* Re: [PATCH 0/3] Prepare for in-kernel VFIO DMA operations acceleration
From: Alexey Kardashevskiy @ 2014-06-06 0:20 UTC (permalink / raw)
To: Alexander Graf, linuxppc-dev
Cc: kvm, Gleb Natapov, linux-kernel, kvm-ppc, Paul Mackerras,
Paolo Bonzini
In-Reply-To: <53905B14.5020204@suse.de>
On 06/05/2014 09:57 PM, Alexander Graf wrote:
>
> On 05.06.14 09:25, Alexey Kardashevskiy wrote:
>> This reserves 2 capability numbers.
>>
>> This implements an extended version of KVM_CREATE_SPAPR_TCE_64 ioctl.
>>
>> Please advise how to proceed with these patches as I suspect that
>> first two should go via Paolo's tree while the last one via Alex Graf's tree
>> (correct?).
>
> They would just go via my tree, but only be actually allocated (read:
> mergable to qemu) when they hit Paolo's tree.
>
> In fact, I don't think it makes sense to split them off at all.
So? Are these patches going anywhere? Thanks.
--
Alexey
^ permalink raw reply
* Re: [PATCH v8 2/3] powerpc/eeh: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-06-06 0:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Gavin Shan, kvm-ppc, agraf, alex.williamson, qiudayu,
linuxppc-dev
In-Reply-To: <1401951004.3247.114.camel@pasglop>
On Thu, Jun 05, 2014 at 04:50:04PM +1000, Benjamin Herrenschmidt wrote:
>On Thu, 2014-06-05 at 16:36 +1000, Gavin Shan wrote:
>> +#define EEH_OPT_GET_PE_ADDR 0 /* Get PE addr */
>> +#define EEH_OPT_GET_PE_MODE 1 /* Get PE mode */
>
>I assume that's just some leftover from the previous patches :-)
>
>Don't respin just yet, let's see what other comments come in.
>
Yep, I'll remove them in next revision. Thanks, Ben.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v8 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-06-06 0:55 UTC (permalink / raw)
To: Alex Williamson; +Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1401988714.9207.198.camel@ul30vt.home>
On Thu, Jun 05, 2014 at 11:18:34AM -0600, Alex Williamson wrote:
>On Thu, 2014-06-05 at 16:36 +1000, Gavin Shan wrote:
>> The patch adds new IOCTL commands for sPAPR VFIO container device
>> to support EEH functionality for PCI devices, which have been passed
>> through from host to somebody else via VFIO.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>> Documentation/vfio.txt | 87 ++++++++++++++++++++++++++++++++++--
>> drivers/vfio/Makefile | 1 +
>> drivers/vfio/pci/vfio_pci.c | 20 ++++++---
>> drivers/vfio/vfio_iommu_spapr_tce.c | 17 ++++++-
>> drivers/vfio/vfio_spapr_eeh.c | 89 +++++++++++++++++++++++++++++++++++++
>> include/linux/vfio.h | 23 ++++++++++
>> include/uapi/linux/vfio.h | 35 +++++++++++++++
>> 7 files changed, 262 insertions(+), 10 deletions(-)
>> create mode 100644 drivers/vfio/vfio_spapr_eeh.c
>>
>> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>> index b9ca023..3fa4538 100644
>> --- a/Documentation/vfio.txt
>> +++ b/Documentation/vfio.txt
>> @@ -305,7 +305,15 @@ faster, the map/unmap handling has been implemented in real mode which provides
>> an excellent performance which has limitations such as inability to do
>> locked pages accounting in real time.
>>
>> -So 3 additional ioctls have been added:
>> +4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
>> +subtree that can be treated as a unit for the purposes of partitioning and
>> +error recovery. A PE may be a single or multi-function IOA (IO Adapter), a
>> +function of a multi-function IOA, or multiple IOAs (possibly including switch
>> +and bridge structures above the multiple IOAs). PPC64 guests detect PCI errors
>> +and recover from them via EEH RTAS services, which works on the basis of
>> +additional ioctl commands.
>> +
>> +So 4 additional ioctls have been added:
>>
>> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
>> of the DMA window on the PCI bus.
>> @@ -316,9 +324,12 @@ So 3 additional ioctls have been added:
>>
>> VFIO_IOMMU_DISABLE - disables the container.
>>
>> + VFIO_EEH_PE_OP - provides an API for EEH setup, error detection and recovery.
>>
>> The code flow from the example above should be slightly changed:
>>
>> + struct vfio_eeh_pe_op pe_op = { .argsz = sizeof(pe_op) };
>> +
>> .....
>> /* Add the group to the container */
>> ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
>> @@ -342,9 +353,79 @@ The code flow from the example above should be slightly changed:
>> dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
>>
>> /* Check here is .iova/.size are within DMA window from spapr_iommu_info */
>> -
>> ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
>> - .....
>> +
>> + /* Get a file descriptor for the device */
>> + device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
>> +
>> + ....
>> +
>> + /* Gratuitous device reset and go... */
>> + ioctl(device, VFIO_DEVICE_RESET);
>> +
>> + /* Make sure EEH is supported */
>> + ioctl(container, VFIO_CHECK_EXTENSION, VFIO_EEH);
>> +
>> + /* Enable the EEH functionality on the device */
>> + pe_op.op = VFIO_EEH_PE_ENABLE;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /* You're suggested to create additional data struct to represent
>> + * PE, and put child devices belonging to same IOMMU group to the
>> + * PE instance for later reference.
>> + */
>> +
>> + /* Check the PE's state and make sure it's in functional state */
>> + pe_op.op = VFIO_EEH_PE_GET_STATE;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /* Save device state using pci_save_state().
>> + * EEH should be enabled on the specified device.
>> + */
>> +
>> + ....
>> +
>> + /* When 0xFF's returned from reading PCI config space or IO BARs
>> + * of the PCI device. Check the PE's state to see if that has been
>> + * frozen.
>> + */
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /* Waiting for pending PCI transactions to be completed and don't
>> + * produce any more PCI traffic from/to the affected PE until
>> + * recovery is finished.
>> + */
>> +
>> + /* Enable IO for the affected PE and collect logs. Usually, the
>> + * standard part of PCI config space, AER registers are dumped
>> + * as logs for further analysis.
>> + */
>> + pe_op.op = VFIO_EEH_PE_UNFREEZE_IO;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /*
>> + * Issue PE reset: hot or fundamental reset. Usually, hot reset
>> + * is enough. However, the firmware of some PCI adapters would
>> + * require fundamental reset.
>> + */
>> + pe_op.op = VFIO_EEH_PE_RESET_HOT;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> + pe_op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /* Configure the PCI bridges for the affected PE */
>> + pe_op.op = VFIO_EEH_PE_CONFIGURE;
>> + ioctl(container, VFIO_EEH_PE_OP, &pe_op);
>> +
>> + /* Restored state we saved at initialization time. pci_restore_state()
>> + * is good enough as an example.
>> + */
>> +
>> + /* Hopefully, error is recovered successfully. Now, you can resume to
>> + * start PCI traffic to/from the affected PE.
>> + */
>> +
>> + ....
>>
>> -------------------------------------------------------------------------------
>>
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 72bfabc..50e30bc 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -1,4 +1,5 @@
>> obj-$(CONFIG_VFIO) += vfio.o
>> obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>> obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>> +obj-$(CONFIG_EEH) += vfio_spapr_eeh.o
>> obj-$(CONFIG_VFIO_PCI) += pci/
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index 7ba0424..26f289d 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -156,8 +156,10 @@ static void vfio_pci_release(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>>
>> - if (atomic_dec_and_test(&vdev->refcnt))
>> + if (atomic_dec_and_test(&vdev->refcnt)) {
>> + vfio_spapr_pci_eeh_release(vdev->pdev);
>> vfio_pci_disable(vdev);
>> + }
>>
>> module_put(THIS_MODULE);
>> }
>> @@ -165,19 +167,25 @@ static void vfio_pci_release(void *device_data)
>> static int vfio_pci_open(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>> + int ret;
>>
>> if (!try_module_get(THIS_MODULE))
>> return -ENODEV;
>>
>> if (atomic_inc_return(&vdev->refcnt) == 1) {
>> - int ret = vfio_pci_enable(vdev);
>> - if (ret) {
>> - module_put(THIS_MODULE);
>> - return ret;
>> - }
>> + ret = vfio_pci_enable(vdev);
>> + if (ret)
>> + goto error;
>> +
>> + ret = vfio_spapr_pci_eeh_open(vdev->pdev);
>> + if (ret)
>> + goto error;
>
>Missing cleanup of vfio_pci_enable() on error
>
Yep, I need to call vfio_pci_disable(vdev) for the case. I'll fix, thanks.
>> }
>>
>> return 0;
>> +error:
>> + module_put(THIS_MODULE);
>> + return ret;
>> }
>>
>> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>> index a84788b..730b4ef 100644
>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>> @@ -156,7 +156,16 @@ static long tce_iommu_ioctl(void *iommu_data,
>>
>> switch (cmd) {
>> case VFIO_CHECK_EXTENSION:
>> - return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
>> + switch (arg) {
>> + case VFIO_SPAPR_TCE_IOMMU:
>> + ret = 1;
>> + break;
>> + default:
>> + ret = vfio_spapr_iommu_eeh_ioctl(NULL, cmd, arg);
>> + break;
>> + }
>> +
>> + return (ret < 0) ? 0 : ret;
>>
>> case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
>> struct vfio_iommu_spapr_tce_info info;
>> @@ -283,6 +292,12 @@ static long tce_iommu_ioctl(void *iommu_data,
>> tce_iommu_disable(container);
>> mutex_unlock(&container->lock);
>> return 0;
>> + case VFIO_EEH_PE_OP:
>> + if (!container->tbl || !container->tbl->it_group)
>> + return -ENODEV;
>> +
>> + return vfio_spapr_iommu_eeh_ioctl(container->tbl->it_group,
>> + cmd, arg);
>> }
>>
>> return -ENOTTY;
>> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
>> new file mode 100644
>> index 0000000..691f8d8
>> --- /dev/null
>> +++ b/drivers/vfio/vfio_spapr_eeh.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * EEH functionality support for VFIO devices. The feature is only
>> + * available on sPAPR compatible platforms.
>> + *
>> + * Copyright Gavin Shan, IBM Corporation 2014.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/iommu.h>
>> +#include <linux/pci.h>
>
>Hmm, doesn't seem like we need iommu.h & pci.h just to pass through a
>pointer. Can these be removed?
>
Yes, we can. I'll remove those in next revision. Thanks.
>> +#include <linux/uaccess.h>
>> +#include <linux/vfio.h>
>> +#include <asm/eeh.h>
>> +
>> +/* We might build address mapping here for "fast" path later */
>> +int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>> +{
>> + return eeh_dev_open(pdev);
>> +}
>> +
>> +void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>> +{
>> + eeh_dev_release(pdev);
>> +}
>> +
>> +long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>> + unsigned int cmd, unsigned long arg)
>> +{
>> + struct eeh_pe *pe;
>> + struct vfio_eeh_pe_op op;
>> + unsigned long minsz;
>> + long ret = -EINVAL;
>> +
>> + switch (cmd) {
>> + case VFIO_CHECK_EXTENSION:
>> + if (arg == VFIO_EEH)
>> + ret = eeh_enabled() ? 1 : 0;
>> + else
>> + ret = 0;
>> + break;
>> + case VFIO_EEH_PE_OP:
>> + pe = eeh_iommu_group_to_pe(group);
>> + if (!pe)
>> + return -ENODEV;
>> +
>> + minsz = offsetofend(struct vfio_eeh_pe_op, op);
>> + if (copy_from_user(&op, (void __user *)arg, minsz))
>> + return -EFAULT;
>> + if (op.argsz < minsz)
>> + return -EINVAL;
>> +
>> + switch (op.op) {
>> + case VFIO_EEH_PE_DISABLE:
>> + ret = eeh_pe_set_option(pe, EEH_OPT_DISABLE);
>> + break;
>> + case VFIO_EEH_PE_ENABLE:
>> + ret = eeh_pe_set_option(pe, EEH_OPT_ENABLE);
>> + break;
>> + case VFIO_EEH_PE_UNFREEZE_IO:
>> + ret = eeh_pe_set_option(pe, EEH_OPT_THAW_MMIO);
>> + break;
>> + case VFIO_EEH_PE_UNFREEZE_DMA:
>> + ret = eeh_pe_set_option(pe, EEH_OPT_THAW_DMA);
>> + break;
>> + case VFIO_EEH_PE_GET_STATE:
>> + ret = eeh_pe_get_state(pe);
>> + break;
>> + case VFIO_EEH_PE_RESET_DEACTIVATE:
>> + ret = eeh_pe_reset(pe, EEH_RESET_DEACTIVATE);
>> + break;
>> + case VFIO_EEH_PE_RESET_HOT:
>> + ret = eeh_pe_reset(pe, EEH_RESET_HOT);
>> + break;
>> + case VFIO_EEH_PE_RESET_FUNDAMENTAL:
>> + ret = eeh_pe_reset(pe, EEH_RESET_FUNDAMENTAL);
>> + break;
>> + case VFIO_EEH_PE_CONFIGURE:
>> + ret = eeh_pe_configure(pe);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index 81022a52..0d3bb8f 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -99,4 +99,27 @@ extern int vfio_external_user_iommu_id(struct vfio_group *group);
>> extern long vfio_external_check_extension(struct vfio_group *group,
>> unsigned long arg);
>>
>> +#ifdef CONFIG_EEH
>> +extern int vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
>> +extern void vfio_spapr_pci_eeh_release(struct pci_dev *pdev);
>> +extern long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>> + unsigned int cmd,
>> + unsigned long arg);
>> +#else
>> +static inline int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>> +{
>> +}
>> +
>> +static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>> + unsigned int cmd,
>> + unsigned long arg)
>> +{
>> + return -ENOTTY;
>> +}
>> +#endif /* CONFIG_EEH */
>> #endif /* VFIO_H */
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index cb9023d..b93c619 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -30,6 +30,9 @@
>> */
>> #define VFIO_DMA_CC_IOMMU 4
>>
>> +/* Check if EEH is supported */
>> +#define VFIO_EEH 5
>> +
>> /*
>> * The IOCTL interface is designed for extensibility by embedding the
>> * structure length (argsz) and flags into structures passed between
>> @@ -455,6 +458,38 @@ struct vfio_iommu_spapr_tce_info {
>>
>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>
>> +/*
>> + * EEH PE operation struct provides ways to:
>> + * - enable/disable EEH functionality;
>> + * - unfreeze IO/DMA for frozen PE;
>> + * - read PE state;
>> + * - reset PE;
>> + * - configure PE.
>> + */
>> +struct vfio_eeh_pe_op {
>> + __u32 argsz;
>> + __u32 flags;
>> + __u32 op;
>> +};
>> +
>> +#define VFIO_EEH_PE_DISABLE 0 /* Disable EEH functionality */
>> +#define VFIO_EEH_PE_ENABLE 1 /* Enable EEH functionality */
>> +#define VFIO_EEH_PE_UNFREEZE_IO 2 /* Enable IO for frozen PE */
>> +#define VFIO_EEH_PE_UNFREEZE_DMA 3 /* Enable DMA for frozen PE */
>> +#define VFIO_EEH_PE_GET_STATE 4 /* PE state retrieval */
>> +#define VFIO_EEH_PE_RESET_DEACTIVATE 5 /* Deassert PE reset */
>> +#define VFIO_EEH_PE_RESET_HOT 6 /* Assert hot reset */
>> +#define VFIO_EEH_PE_RESET_FUNDAMENTAL 7 /* Assert fundamental reset */
>> +#define VFIO_EEH_PE_CONFIGURE 8 /* PE configuration */
>> +
>> +#define VFIO_EEH_PE_STATE_NORMAL 0 /* PE in functional state */
>> +#define VFIO_EEH_PE_STATE_RESET 1 /* PE reset in progress */
>> +#define VFIO_EEH_PE_STATE_STOPPED 2 /* Stopped DMA and IO */
>> +#define VFIO_EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA only */
>> +#define VFIO_EEH_PE_STATE_UNAVAIL 5 /* State unavailable */
>
>Please make it explicit that these are return values for
>VFIO_EEH_PE_GET_STATE. They could be defined inline after
>VFIO_EEH_PE_GET_STATE with indenting to make it clear. Thanks,
>
Sure. I'll fix. Thanks :-)
Thanks,
Gavin
>> +
>> +#define VFIO_EEH_PE_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
>> +
>> /* ***************************************************************** */
>>
>> #endif /* _UAPIVFIO_H */
>
>
>
^ permalink raw reply
* Re: [PATCH v8 0/3] EEH Support for VFIO PCI Device
From: Gavin Shan @ 2014-06-06 0:56 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <53906897.4080209@suse.de>
On Thu, Jun 05, 2014 at 02:54:47PM +0200, Alexander Graf wrote:
>
>On 05.06.14 08:36, Gavin Shan wrote:
>>The series of patches adds support EEH for PCI devices, which are passed
>>through to PowerKVM based guest via VFIO. The implementation is straightforward
>>based on the issues or problems we have to resolve to support EEH for PowerKVM
>>based guest.
>>
>>- Emulation for EEH RTAS requests. All EEH RTAS requests goes to QEMU firstly.
>> If QEMU can't handle it, the request will be sent to host via newly introduced
>> VFIO container IOCTL command (VFIO_EEH_OP) and gets handled in host kernel.
>>
>>The series of patches requires corresponding QEMU changes.
>
>Acked-by: Alexander Graf <agraf@suse.de>
>
Thanks, Alex :)
>
>Alex
>
^ 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