[PATCH 0/2] KVM: PPC: Save/restore complete Book3S HV guest register state

[PATCH 0/2] KVM: PPC: Save/restore complete Book3S HV guest register state

[Paul Mackerras]

This series of 2 patches makes it possible for userspace to save and
restore the complete register state of a Book3S HV guest.  Currently
it is not possible to save/restore several SPRs or any of the
floating-point state (including VMX/Altivec and VSX state).  This
series extends the [GS]ET_ONE_REG ioctl to handle the missing SPRs,
and implements kvm_arch_vcpu_ioctl_[gs]et_fpu() and extends struct
kvm_fpu to handle the floating-point state.

These patches are against the kvm-ppc-next branch of Alex Graf's
linux-2.6.git repository on github.

Paul.

[PATCH 1/2] KVM: PPC: Book3S HV: Get/set guest SPRs using the GET/SET_ONE_REG interface

[Paul Mackerras]

This enables userspace to get and set various SPRs (special-purpose
registers) using the KVM_[GS]ET_ONE_REG ioctls.  With this, userspace
can get and set all the SPRs that are part of the guest state, either
through the KVM_[GS]ET_REGS ioctls, the KVM_[GS]ET_SREGS ioctls, or
the KVM_[GS]ET_ONE_REG ioctls.

The SPRs that are added here are:

- DABR:  Data address breakpoint register
- DSCR:  Data stream control register
- PURR:  Processor utilization of resources register
- SPURR: Scaled PURR
- DAR:   Data address register
- DSISR: Data storage interrupt status register
- AMR:   Authority mask register
- UAMOR: User authority mask override register
- MMCR0, MMCR1, MMCRA: Performance monitor unit control registers
- PMC1..PMC8: Performance monitor unit counter registers

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/include/asm/kvm.h |   21 ++++++++
 arch/powerpc/kvm/book3s_hv.c   |  106 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 3c14202..9557576 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -338,5 +338,26 @@ struct kvm_book3e_206_tlb_params {
 #define KVM_REG_PPC_IAC4	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
 #define KVM_REG_PPC_DAC1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
 #define KVM_REG_PPC_DAC2	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
+#define KVM_REG_PPC_DABR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8)
+#define KVM_REG_PPC_DSCR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x9)
+#define KVM_REG_PPC_PURR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xa)
+#define KVM_REG_PPC_SPURR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xb)
+#define KVM_REG_PPC_DAR		(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc)
+#define KVM_REG_PPC_DSISR	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0xd)
+#define KVM_REG_PPC_AMR		(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xe)
+#define KVM_REG_PPC_UAMOR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xf)
+
+#define KVM_REG_PPC_MMCR0	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
+#define KVM_REG_PPC_MMCR1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
+#define KVM_REG_PPC_MMCRA	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
+
+#define KVM_REG_PPC_PMC1	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x18)
+#define KVM_REG_PPC_PMC2	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x19)
+#define KVM_REG_PPC_PMC3	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1a)
+#define KVM_REG_PPC_PMC4	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1b)
+#define KVM_REG_PPC_PMC5	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1c)
+#define KVM_REG_PPC_PMC6	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1d)
+#define KVM_REG_PPC_PMC7	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1e)
+#define KVM_REG_PPC_PMC8	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1f)
 
 #endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 83e929e..7fe5c9a 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -538,11 +538,53 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 {
 	int r = -EINVAL;
+	long int i;
 
 	switch (reg->id) {
 	case KVM_REG_PPC_HIOR:
 		r = put_user(0, (u64 __user *)reg->addr);
 		break;
+	case KVM_REG_PPC_DABR:
+		r = put_user(vcpu->arch.dabr, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_DSCR:
+		r = put_user(vcpu->arch.dscr, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_PURR:
+		r = put_user(vcpu->arch.purr, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_SPURR:
+		r = put_user(vcpu->arch.spurr, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_DAR:
+		r = put_user(vcpu->arch.shregs.dar, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_DSISR:
+		r = put_user(vcpu->arch.shregs.dsisr, (u32 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_AMR:
+		r = put_user(vcpu->arch.amr, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_UAMOR:
+		r = put_user(vcpu->arch.uamor, (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_MMCR0:
+	case KVM_REG_PPC_MMCR1:
+	case KVM_REG_PPC_MMCRA:
+		i = reg->id - KVM_REG_PPC_MMCR0;
+		r = put_user(vcpu->arch.mmcr[i], (u64 __user *)reg->addr);
+		break;
+	case KVM_REG_PPC_PMC1:
+	case KVM_REG_PPC_PMC2:
+	case KVM_REG_PPC_PMC3:
+	case KVM_REG_PPC_PMC4:
+	case KVM_REG_PPC_PMC5:
+	case KVM_REG_PPC_PMC6:
+	case KVM_REG_PPC_PMC7:
+	case KVM_REG_PPC_PMC8:
+		i = reg->id - KVM_REG_PPC_PMC1;
+		r = put_user(vcpu->arch.pmc[i], (u32 __user *)reg->addr);
+		break;
 	default:
 		break;
 	}
@@ -553,6 +595,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 {
 	int r = -EINVAL;
+	u64 val;
+	u32 wval;
+	long int i;
 
 	switch (reg->id) {
 	case KVM_REG_PPC_HIOR:
@@ -564,6 +609,67 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 			r = -EINVAL;
 		break;
 	}
+	case KVM_REG_PPC_DABR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.dabr = val;
+		break;
+	case KVM_REG_PPC_DSCR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.dscr = val;
+		break;
+	case KVM_REG_PPC_PURR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.purr = val;
+		break;
+	case KVM_REG_PPC_SPURR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.spurr = val;
+		break;
+	case KVM_REG_PPC_DAR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.shregs.dar = val;
+		break;
+	case KVM_REG_PPC_DSISR:
+		r = get_user(wval, (u32 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.shregs.dsisr = wval;
+		break;
+	case KVM_REG_PPC_AMR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.amr = val;
+		break;
+	case KVM_REG_PPC_UAMOR:
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.uamor = val;
+		break;
+	case KVM_REG_PPC_MMCR0:
+	case KVM_REG_PPC_MMCR1:
+	case KVM_REG_PPC_MMCRA:
+		i = reg->id - KVM_REG_PPC_MMCR0;
+		r = get_user(val, (u64 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.mmcr[i] = val;
+		break;
+	case KVM_REG_PPC_PMC1:
+	case KVM_REG_PPC_PMC2:
+	case KVM_REG_PPC_PMC3:
+	case KVM_REG_PPC_PMC4:
+	case KVM_REG_PPC_PMC5:
+	case KVM_REG_PPC_PMC6:
+	case KVM_REG_PPC_PMC7:
+	case KVM_REG_PPC_PMC8:
+		i = reg->id - KVM_REG_PPC_PMC1;
+		r = get_user(wval, (u32 __user *)reg->addr);
+		if (!r)
+			vcpu->arch.pmc[i] = wval;
+		break;
 	default:
 		break;
 	}
-- 
1.7.10.rc3.219.g53414

[PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Paul Mackerras]

Currently the KVM_GET_FPU and KVM_SET_FPU ioctls return an EOPNOTSUPP
error on powerpc.  This implements them for Book 3S processors.  Since
those processors have more than just the 32 basic floating-point
registers, this extends the kvm_fpu structure to have space for the
additional registers -- the 32 vector registers (128 bits each) for
VMX/Altivec and the 32 additional 64-bit registers that were added
on POWER7 for the vector-scalar extension (VSX).  It also adds a
`valid' field, which is a bitmap indicating which elements contain
valid data.

The layout of the floating-point register data in the vcpu struct is
mostly the same between different flavors of KVM on Book 3S processors,
but the set of supported registers may differ depending on what the
CPU hardware supports and how much is emulated.  Therefore we have
a flavor-specific function to work out which set of registers to
supply for the get function.

On POWER7 processors using the Book 3S HV flavor of KVM, we save the
standard floating-point registers together with their corresponding
VSX extension register in the vcpu->arch.vsr[] array, since each
pair can be loaded or stored with one instruction.  This is different
to other flavors of KVM, and to other processors (i.e. PPC970) with
HV KVM, which store the standard FPRs in vcpu->arch.fpr[].  To cope
with this, we use the kvmppc_core_get_fpu_valid() and
kvmppc_core_set_fpu_valid() functions to sync between the arch.fpr[]
and arch.vsr[] arrays as needed.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/include/asm/kvm.h     |   11 ++++++++
 arch/powerpc/include/asm/kvm_ppc.h |    2 ++
 arch/powerpc/kvm/book3s.c          |   49 ++++++++++++++++++++++++++++++++++--
 arch/powerpc/kvm/book3s_hv.c       |   32 +++++++++++++++++++++++
 arch/powerpc/kvm/book3s_pr.c       |   19 ++++++++++++++
 5 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 9557576..5792024 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -261,9 +261,20 @@ struct kvm_sregs {
 };
 
 struct kvm_fpu {
+	__u64 valid;
+	__u64 fpscr;
 	__u64 fpr[32];
+	__vector128 vr[32];
+	__u32 _pad[3];
+	__u32 vscr;
+	__u64 vsr[32];
 };
 
+/* Values for kvm_fpu valid field */
+#define KVM_FPU_FPREGS	1	/* standard floating-point regs & FPSCR */
+#define KVM_FPU_VREGS	2	/* VMX/Altivec vector regs & VSCR */
+#define KVM_FPU_VSREGS	4	/* POWER7 vector/scalar additional regs */
+
 struct kvm_debug_exit_arch {
 };
 
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index c06a64b..5dccdc5 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -96,6 +96,8 @@ extern int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
 
 extern void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
+extern u64 kvmppc_core_get_fpu_valid(struct kvm_vcpu *vcpu);
+extern void kvmppc_core_set_fpu_valid(struct kvm_vcpu *vcpu, u64 valid);
 
 extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index e946665..c2278f6 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -477,12 +477,57 @@ int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-	return -ENOTSUPP;
+	u64 valid = kvmppc_core_get_fpu_valid(vcpu);
+
+	/* avoid leaking information to userspace */
+	memset(fpu, 0, sizeof(*fpu));
+
+	if (valid & KVM_FPU_FPREGS) {
+		memcpy(fpu->fpr, vcpu->arch.fpr, sizeof(fpu->fpr));
+		fpu->fpscr = vcpu->arch.fpscr;
+	}
+#ifdef CONFIG_ALTIVEC
+	if (valid & KVM_FPU_VREGS) {
+		memcpy(fpu->vr, vcpu->arch.vr, sizeof(fpu->vr));
+		fpu->vscr = vcpu->arch.vscr.u[3];
+	}
+#endif
+#ifdef CONFIG_VSX
+	if (valid & KVM_FPU_VSREGS) {
+		long i;
+
+		for (i = 0; i < ARRAY_SIZE(fpu->vsr); ++i)
+			fpu->vsr[i] = vcpu->arch.vsr[2*i + 1];
+	}
+#endif
+	fpu->valid = valid;
+	return 0;
 }
 
 int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-	return -ENOTSUPP;
+	u64 valid = fpu->valid;
+
+	if (valid & KVM_FPU_FPREGS) {
+		memcpy(vcpu->arch.fpr, fpu->fpr, sizeof(vcpu->arch.fpr));
+		vcpu->arch.fpscr = fpu->fpscr;
+	}
+#ifdef CONFIG_ALTIVEC
+	if (valid & KVM_FPU_VREGS) {
+		memcpy(vcpu->arch.vr, fpu->vr, sizeof(vcpu->arch.vr));
+		vcpu->arch.vscr.u[3] = fpu->vscr;
+	}
+#endif
+#ifdef CONFIG_VSX
+	if (valid & KVM_FPU_VSREGS) {
+		long i;
+
+		for (i = 0; i < ARRAY_SIZE(fpu->vsr); ++i)
+			vcpu->arch.vsr[2*i + 1] = fpu->vsr[i];
+	}
+#endif
+	kvmppc_core_set_fpu_valid(vcpu, valid);
+	return 0;
 }
 
 int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7fe5c9a..2c2b6b9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -677,6 +677,38 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 	return r;
 }
 
+u64 kvmppc_core_get_fpu_valid(struct kvm_vcpu *vcpu)
+{
+	u64 valid = KVM_FPU_FPREGS;
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC))
+		valid |= KVM_FPU_VREGS;
+#endif
+#ifdef CONFIG_VSX
+	if (cpu_has_feature(CPU_FTR_VSX)) {
+		long i;
+
+		for (i = 0; i < ARRAY_SIZE(vcpu->arch.fpr); ++i)
+			vcpu->arch.fpr[i] = vcpu->arch.vsr[2*i];
+		valid |= KVM_FPU_VSREGS;
+	}
+#endif
+	return valid;
+}
+
+void kvmppc_core_set_fpu_valid(struct kvm_vcpu *vcpu, u64 valid)
+{
+#ifdef CONFIG_VSX
+	if ((valid & KVM_FPU_FPREGS) && cpu_has_feature(CPU_FTR_VSX)) {
+		long i;
+
+		for (i = 0; i < ARRAY_SIZE(vcpu->arch.fpr); ++i)
+			vcpu->arch.vsr[2*i] = vcpu->arch.fpr[i];
+	}
+#endif
+}
+
 int kvmppc_core_check_processor_compat(void)
 {
 	if (cpu_has_feature(CPU_FTR_HVMODE))
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index b3c584f..e3b81a2 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -978,6 +978,25 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
 	return r;
 }
 
+u64 kvmppc_core_get_fpu_valid(struct kvm_vcpu *vcpu)
+{
+	u64 valid = KVM_FPU_FPREGS;
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC))
+		valid |= KVM_FPU_VREGS;
+#endif
+#ifdef CONFIG_VSX
+	if (cpu_has_feature(CPU_FTR_VSX))
+		valid |= KVM_FPU_VSREGS;
+#endif
+	return valid;
+}
+
+void kvmppc_core_set_fpu_valid(struct kvm_vcpu *vcpu, u64 valid)
+{
+}
+
 int kvmppc_core_check_processor_compat(void)
 {
 	return 0;
-- 
1.7.10.rc3.219.g53414

Re: [PATCH 1/2] KVM: PPC: Book3S HV: Get/set guest SPRs using the GET/SET_ONE_REG interface

[Alexander Graf]

On 12.09.2012, at 02:18, Paul Mackerras wrote:

> This enables userspace to get and set various SPRs (special-purpose
> registers) using the KVM_[GS]ET_ONE_REG ioctls.  With this, userspace
> can get and set all the SPRs that are part of the guest state, either
> through the KVM_[GS]ET_REGS ioctls, the KVM_[GS]ET_SREGS ioctls, or
> the KVM_[GS]ET_ONE_REG ioctls.
> 
> The SPRs that are added here are:
> 
> - DABR:  Data address breakpoint register
> - DSCR:  Data stream control register
> - PURR:  Processor utilization of resources register
> - SPURR: Scaled PURR
> - DAR:   Data address register
> - DSISR: Data storage interrupt status register
> - AMR:   Authority mask register
> - UAMOR: User authority mask override register
> - MMCR0, MMCR1, MMCRA: Performance monitor unit control registers
> - PMC1..PMC8: Performance monitor unit counter registers
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> arch/powerpc/include/asm/kvm.h |   21 ++++++++
> arch/powerpc/kvm/book3s_hv.c   |  106 ++++++++++++++++++++++++++++++++++++++++

Documentation/virtual/kvm/api.txt | +++++++++++

:)


Alex

> 2 files changed, 127 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
> index 3c14202..9557576 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -338,5 +338,26 @@ struct kvm_book3e_206_tlb_params {
> #define KVM_REG_PPC_IAC4	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
> #define KVM_REG_PPC_DAC1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
> #define KVM_REG_PPC_DAC2	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
> +#define KVM_REG_PPC_DABR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8)
> +#define KVM_REG_PPC_DSCR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x9)
> +#define KVM_REG_PPC_PURR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xa)
> +#define KVM_REG_PPC_SPURR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xb)
> +#define KVM_REG_PPC_DAR		(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc)
> +#define KVM_REG_PPC_DSISR	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0xd)
> +#define KVM_REG_PPC_AMR		(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xe)
> +#define KVM_REG_PPC_UAMOR	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xf)
> +
> +#define KVM_REG_PPC_MMCR0	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
> +#define KVM_REG_PPC_MMCR1	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
> +#define KVM_REG_PPC_MMCRA	(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
> +
> +#define KVM_REG_PPC_PMC1	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x18)
> +#define KVM_REG_PPC_PMC2	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x19)
> +#define KVM_REG_PPC_PMC3	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1a)
> +#define KVM_REG_PPC_PMC4	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1b)
> +#define KVM_REG_PPC_PMC5	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1c)
> +#define KVM_REG_PPC_PMC6	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1d)
> +#define KVM_REG_PPC_PMC7	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1e)
> +#define KVM_REG_PPC_PMC8	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1f)
> 
> #endif /* __LINUX_KVM_POWERPC_H */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 83e929e..7fe5c9a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -538,11 +538,53 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> {
> 	int r = -EINVAL;
> +	long int i;
> 
> 	switch (reg->id) {
> 	case KVM_REG_PPC_HIOR:
> 		r = put_user(0, (u64 __user *)reg->addr);
> 		break;
> +	case KVM_REG_PPC_DABR:
> +		r = put_user(vcpu->arch.dabr, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_DSCR:
> +		r = put_user(vcpu->arch.dscr, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_PURR:
> +		r = put_user(vcpu->arch.purr, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_SPURR:
> +		r = put_user(vcpu->arch.spurr, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_DAR:
> +		r = put_user(vcpu->arch.shregs.dar, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_DSISR:
> +		r = put_user(vcpu->arch.shregs.dsisr, (u32 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_AMR:
> +		r = put_user(vcpu->arch.amr, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_UAMOR:
> +		r = put_user(vcpu->arch.uamor, (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_MMCR0:
> +	case KVM_REG_PPC_MMCR1:
> +	case KVM_REG_PPC_MMCRA:
> +		i = reg->id - KVM_REG_PPC_MMCR0;
> +		r = put_user(vcpu->arch.mmcr[i], (u64 __user *)reg->addr);
> +		break;
> +	case KVM_REG_PPC_PMC1:
> +	case KVM_REG_PPC_PMC2:
> +	case KVM_REG_PPC_PMC3:
> +	case KVM_REG_PPC_PMC4:
> +	case KVM_REG_PPC_PMC5:
> +	case KVM_REG_PPC_PMC6:
> +	case KVM_REG_PPC_PMC7:
> +	case KVM_REG_PPC_PMC8:
> +		i = reg->id - KVM_REG_PPC_PMC1;
> +		r = put_user(vcpu->arch.pmc[i], (u32 __user *)reg->addr);
> +		break;
> 	default:
> 		break;
> 	}
> @@ -553,6 +595,9 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> {
> 	int r = -EINVAL;
> +	u64 val;
> +	u32 wval;
> +	long int i;
> 
> 	switch (reg->id) {
> 	case KVM_REG_PPC_HIOR:
> @@ -564,6 +609,67 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
> 			r = -EINVAL;
> 		break;
> 	}
> +	case KVM_REG_PPC_DABR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.dabr = val;
> +		break;
> +	case KVM_REG_PPC_DSCR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.dscr = val;
> +		break;
> +	case KVM_REG_PPC_PURR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.purr = val;
> +		break;
> +	case KVM_REG_PPC_SPURR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.spurr = val;
> +		break;
> +	case KVM_REG_PPC_DAR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.shregs.dar = val;
> +		break;
> +	case KVM_REG_PPC_DSISR:
> +		r = get_user(wval, (u32 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.shregs.dsisr = wval;
> +		break;
> +	case KVM_REG_PPC_AMR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.amr = val;
> +		break;
> +	case KVM_REG_PPC_UAMOR:
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.uamor = val;
> +		break;
> +	case KVM_REG_PPC_MMCR0:
> +	case KVM_REG_PPC_MMCR1:
> +	case KVM_REG_PPC_MMCRA:
> +		i = reg->id - KVM_REG_PPC_MMCR0;
> +		r = get_user(val, (u64 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.mmcr[i] = val;
> +		break;
> +	case KVM_REG_PPC_PMC1:
> +	case KVM_REG_PPC_PMC2:
> +	case KVM_REG_PPC_PMC3:
> +	case KVM_REG_PPC_PMC4:
> +	case KVM_REG_PPC_PMC5:
> +	case KVM_REG_PPC_PMC6:
> +	case KVM_REG_PPC_PMC7:
> +	case KVM_REG_PPC_PMC8:
> +		i = reg->id - KVM_REG_PPC_PMC1;
> +		r = get_user(wval, (u32 __user *)reg->addr);
> +		if (!r)
> +			vcpu->arch.pmc[i] = wval;
> +		break;
> 	default:
> 		break;
> 	}
> -- 
> 1.7.10.rc3.219.g53414
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Alexander Graf]

On 12.09.2012, at 02:19, Paul Mackerras wrote:

> Currently the KVM_GET_FPU and KVM_SET_FPU ioctls return an EOPNOTSUPP
> error on powerpc.  This implements them for Book 3S processors.  Since
> those processors have more than just the 32 basic floating-point
> registers, this extends the kvm_fpu structure to have space for the
> additional registers -- the 32 vector registers (128 bits each) for
> VMX/Altivec and the 32 additional 64-bit registers that were added
> on POWER7 for the vector-scalar extension (VSX).  It also adds a
> `valid' field, which is a bitmap indicating which elements contain
> valid data.
> 
> The layout of the floating-point register data in the vcpu struct is
> mostly the same between different flavors of KVM on Book 3S processors,
> but the set of supported registers may differ depending on what the
> CPU hardware supports and how much is emulated.  Therefore we have
> a flavor-specific function to work out which set of registers to
> supply for the get function.
> 
> On POWER7 processors using the Book 3S HV flavor of KVM, we save the
> standard floating-point registers together with their corresponding
> VSX extension register in the vcpu->arch.vsr[] array, since each
> pair can be loaded or stored with one instruction.  This is different
> to other flavors of KVM, and to other processors (i.e. PPC970) with
> HV KVM, which store the standard FPRs in vcpu->arch.fpr[].  To cope
> with this, we use the kvmppc_core_get_fpu_valid() and
> kvmppc_core_set_fpu_valid() functions to sync between the arch.fpr[]
> and arch.vsr[] arrays as needed.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>

Any reason to not use ONE_REG here?


Alex

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Paul Mackerras]

On Fri, Sep 14, 2012 at 01:30:51AM +0200, Alexander Graf wrote:
> 
> On 12.09.2012, at 02:19, Paul Mackerras wrote:
> 
> > Currently the KVM_GET_FPU and KVM_SET_FPU ioctls return an EOPNOTSUPP
> > error on powerpc.  This implements them for Book 3S processors.  Since
> > those processors have more than just the 32 basic floating-point
> > registers, this extends the kvm_fpu structure to have space for the
> > additional registers -- the 32 vector registers (128 bits each) for
> > VMX/Altivec and the 32 additional 64-bit registers that were added
> > on POWER7 for the vector-scalar extension (VSX).  It also adds a
> > `valid' field, which is a bitmap indicating which elements contain
> > valid data.
> > 
> > The layout of the floating-point register data in the vcpu struct is
> > mostly the same between different flavors of KVM on Book 3S processors,
> > but the set of supported registers may differ depending on what the
> > CPU hardware supports and how much is emulated.  Therefore we have
> > a flavor-specific function to work out which set of registers to
> > supply for the get function.
> > 
> > On POWER7 processors using the Book 3S HV flavor of KVM, we save the
> > standard floating-point registers together with their corresponding
> > VSX extension register in the vcpu->arch.vsr[] array, since each
> > pair can be loaded or stored with one instruction.  This is different
> > to other flavors of KVM, and to other processors (i.e. PPC970) with
> > HV KVM, which store the standard FPRs in vcpu->arch.fpr[].  To cope
> > with this, we use the kvmppc_core_get_fpu_valid() and
> > kvmppc_core_set_fpu_valid() functions to sync between the arch.fpr[]
> > and arch.vsr[] arrays as needed.
> > 
> > Signed-off-by: Paul Mackerras <paulus@samba.org>
> 
> Any reason to not use ONE_REG here?

Just consistency with x86 -- they have an xmm[][] field in their
struct kvm_fpu which looks like it contains their vector state.

Paul.

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Alexander Graf]

On 14.09.2012, at 01:58, Paul Mackerras wrote:

> On Fri, Sep 14, 2012 at 01:30:51AM +0200, Alexander Graf wrote:
>> 
>> On 12.09.2012, at 02:19, Paul Mackerras wrote:
>> 
>>> Currently the KVM_GET_FPU and KVM_SET_FPU ioctls return an EOPNOTSUPP
>>> error on powerpc.  This implements them for Book 3S processors.  Since
>>> those processors have more than just the 32 basic floating-point
>>> registers, this extends the kvm_fpu structure to have space for the
>>> additional registers -- the 32 vector registers (128 bits each) for
>>> VMX/Altivec and the 32 additional 64-bit registers that were added
>>> on POWER7 for the vector-scalar extension (VSX).  It also adds a
>>> `valid' field, which is a bitmap indicating which elements contain
>>> valid data.
>>> 
>>> The layout of the floating-point register data in the vcpu struct is
>>> mostly the same between different flavors of KVM on Book 3S processors,
>>> but the set of supported registers may differ depending on what the
>>> CPU hardware supports and how much is emulated.  Therefore we have
>>> a flavor-specific function to work out which set of registers to
>>> supply for the get function.
>>> 
>>> On POWER7 processors using the Book 3S HV flavor of KVM, we save the
>>> standard floating-point registers together with their corresponding
>>> VSX extension register in the vcpu->arch.vsr[] array, since each
>>> pair can be loaded or stored with one instruction.  This is different
>>> to other flavors of KVM, and to other processors (i.e. PPC970) with
>>> HV KVM, which store the standard FPRs in vcpu->arch.fpr[].  To cope
>>> with this, we use the kvmppc_core_get_fpu_valid() and
>>> kvmppc_core_set_fpu_valid() functions to sync between the arch.fpr[]
>>> and arch.vsr[] arrays as needed.
>>> 
>>> Signed-off-by: Paul Mackerras <paulus@samba.org>
>> 
>> Any reason to not use ONE_REG here?
> 
> Just consistency with x86 -- they have an xmm[][] field in their
> struct kvm_fpu which looks like it contains their vector state.

Yup, Considering how different the FPU state on differnet ppc cores is, I'd be more happy with shoving it into something that allows for more dynamic control. Otherwise we'd end up with yet another struct sregs that can contain SPE registers, altivec, and a dozen additions to it :).

Please just use one_reg for all of the register synchronization you want to add, unless there's a compelling reason to do it differently. It will make our live a lot easier in the future. If we need to transfer too much data and actually run into performance trouble, we can always add a GET_MANY_REG ioctl.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Paul Mackerras]

On Fri, Sep 14, 2012 at 02:03:15AM +0200, Alexander Graf wrote:
> 
> Yup, Considering how different the FPU state on differnet ppc cores is, I'd be more happy with shoving it into something that allows for more dynamic control. Otherwise we'd end up with yet another struct sregs that can contain SPE registers, altivec, and a dozen additions to it :).
> 
> Please just use one_reg for all of the register synchronization you want to add, unless there's a compelling reason to do it differently. It will make our live a lot easier in the future. If we need to transfer too much data and actually run into performance trouble, we can always add a GET_MANY_REG ioctl.

It just seems perverse to ignore the existing interface that every
other architecture uses, and instead do something unique that is
actually slower, but whatever...

Paul.

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Alexander Graf]

On 14.09.2012, at 03:36, Paul Mackerras wrote:

> On Fri, Sep 14, 2012 at 02:03:15AM +0200, Alexander Graf wrote:
>> 
>> Yup, Considering how different the FPU state on differnet ppc cores is, I'd be more happy with shoving it into something that allows for more dynamic control. Otherwise we'd end up with yet another struct sregs that can contain SPE registers, altivec, and a dozen additions to it :).
>> 
>> Please just use one_reg for all of the register synchronization you want to add, unless there's a compelling reason to do it differently. It will make our live a lot easier in the future. If we need to transfer too much data and actually run into performance trouble, we can always add a GET_MANY_REG ioctl.
> 
> It just seems perverse to ignore the existing interface that every
> other architecture uses, and instead do something unique that is
> actually slower, but whatever...

We're slowly moving towards ONE_REG. ARM is already going full steam ahead and I'd like to have every new register in PPC be modeled with it as well. The old interface broke on us one time too often now :).

As I said, if we run into performance problems, we will implement ways to improve performance. At the end of the day, x86 will be the odd one out.


Alex

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Alexander Graf]

On 14.09.2012, at 03:44, Alexander Graf wrote:

> 
> On 14.09.2012, at 03:36, Paul Mackerras wrote:
> 
>> On Fri, Sep 14, 2012 at 02:03:15AM +0200, Alexander Graf wrote:
>>> 
>>> Yup, Considering how different the FPU state on differnet ppc cores is, I'd be more happy with shoving it into something that allows for more dynamic control. Otherwise we'd end up with yet another struct sregs that can contain SPE registers, altivec, and a dozen additions to it :).
>>> 
>>> Please just use one_reg for all of the register synchronization you want to add, unless there's a compelling reason to do it differently. It will make our live a lot easier in the future. If we need to transfer too much data and actually run into performance trouble, we can always add a GET_MANY_REG ioctl.
>> 
>> It just seems perverse to ignore the existing interface that every
>> other architecture uses, and instead do something unique that is
>> actually slower, but whatever...
> 
> We're slowly moving towards ONE_REG. ARM is already going full steam ahead and I'd like to have every new register in PPC be modeled with it as well. The old interface broke on us one time too often now :).
> 
> As I said, if we run into performance problems, we will implement ways to improve performance. At the end of the day, x86 will be the odd one out.

(plus your patch breaks abi compatibility with old user space)


Alex

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Paul Mackerras]

On Fri, Sep 14, 2012 at 03:50:06AM +0200, Alexander Graf wrote:

> (plus your patch breaks abi compatibility with old user space)

Well, only to the extent that the old ioctl code would now return an
ENOTTY error rather than an EOPNOTSUPP error.  Note that the ioctl
code has the struct size embedded, so it's not like a program compiled
against the old definition would suddenly get more data than it had
made room for.

But since you insist, I'll hack up the ONE_REG interface and see what
it looks like...

Paul.

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Benjamin Herrenschmidt]

On Fri, 2012-09-14 at 03:44 +0200, Alexander Graf wrote:
> 
> We're slowly moving towards ONE_REG. ARM is already going full steam
> ahead and I'd like to have every new register in PPC be modeled with
> it as well. The old interface broke on us one time too often now :).
> 
> As I said, if we run into performance problems, we will implement ways
> to improve performance. At the end of the day, x86 will be the odd one
> out.

This is totally insane. In most cases, we care about the whole set of 32
registers. Doing 32 ioctl's for that is just plain stupid.

There is nothing wrong in allowing the "one reg" interface as well, but
it's totally ridiculous to *require* it.

Ben.

Re: [PATCH 2/2] KVM: PPC: Book3S: Implement floating-point state get/set functions

[Alexander Graf]

On 19.09.2012, at 05:37, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2012-09-14 at 03:44 +0200, Alexander Graf wrote:
>> 
>> We're slowly moving towards ONE_REG. ARM is already going full steam
>> ahead and I'd like to have every new register in PPC be modeled with
>> it as well. The old interface broke on us one time too often now :).
>> 
>> As I said, if we run into performance problems, we will implement ways
>> to improve performance. At the end of the day, x86 will be the odd one
>> out.
> 
> This is totally insane. In most cases, we care about the whole set of 32
> registers. Doing 32 ioctl's for that is just plain stupid.

No. In most cases we care about ~100 registers that we want to synchronize. Doing 10 ioctls for that is insane.

I want to move to a model where we can have only a single ioctl for all of the 100 regs. But the first step towards that direction is to split them into individual ioctls with explicit IDs for each, so we can then introduce sn ioctl that can write an arbitrary number of regs. The ioctl to read/write an array of registers is dead simple. Just ask Rusty - he already has a prototype in a branch :).

Also, don't only think about QEMU here. If you want to write a different user space for KVM, you might not want to keep a copy of the guest CPU's state in some struct. Instead, you could on every register access from user space simply call the one_reg API and leave all stare to kvm.

Either way, we only synchronize these registers quite seldomly. We do it when you check out registers on the monitor, when we live migrate, or when we have some odd hypercall that requires knowledge of more guest state than it should (vmport). But this really is not a fast path today.


Alex

> 
> There is nothing wrong in allowing the "one reg" interface as well, but
> it's totally ridiculous to *require* it.
> 
> Ben.
> 
>