All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] KVM/arm64 fixes for 3.13
@ 2013-11-08 10:07 Marc Zyngier
  2013-11-08 10:07 ` [PATCH 1/3] arm64: KVM: Yield CPU when vcpu executes a WFE Marc Zyngier
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Marc Zyngier @ 2013-11-08 10:07 UTC (permalink / raw)
  To: Gleb Natapov, Paolo Bonzini; +Cc: kvm, kvmarm, Christoffer Dall

The following changes since commit 4a10c2ac2f368583138b774ca41fac4207911983:

  Linux 3.12-rc2 (2013-09-23 15:41:09 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git tags/kvm-arm64/for-3.13-1

for you to fetch changes up to ce94fe93d566bf381c6ecbd45010d36c5f04d692:

  arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu (2013-11-07 19:09:08 +0000)

----------------------------------------------------------------
A handful of fixes for KVM/arm64:

- A couple a basic fixes for running BE guests on a LE host
- A performance improvement for overcommitted VMs (same as the equivalent
  patch for ARM)

----------------------------------------------------------------

Marc Zyngier (3):
  arm64: KVM: Yield CPU when vcpu executes a WFE
  arm/arm64: KVM: MMIO support for BE guest
  arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu

 arch/arm/include/asm/kvm_emulate.h   | 46 +++++++++++++++++++
 arch/arm/kvm/mmio.c                  | 86 +++++++++++++++++++++++++++++++-----
 arch/arm/kvm/psci.c                  |  4 ++
 arch/arm64/include/asm/kvm_arm.h     |  8 +++-
 arch/arm64/include/asm/kvm_emulate.h | 56 +++++++++++++++++++++++
 arch/arm64/kvm/Kconfig               |  1 +
 arch/arm64/kvm/handle_exit.c         | 18 +++++---
 7 files changed, 201 insertions(+), 18 deletions(-)

-- 
1.8.2.3



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/3] arm64: KVM: Yield CPU when vcpu executes a WFE
  2013-11-08 10:07 [GIT PULL] KVM/arm64 fixes for 3.13 Marc Zyngier
@ 2013-11-08 10:07 ` Marc Zyngier
  2013-11-08 10:07 ` [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest Marc Zyngier
  2013-11-08 10:07 ` [PATCH 3/3] arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu Marc Zyngier
  2 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2013-11-08 10:07 UTC (permalink / raw)
  To: Gleb Natapov, Paolo Bonzini; +Cc: kvm, kvmarm, Christoffer Dall

On an (even slightly) oversubscribed system, spinlocks are quickly
becoming a bottleneck, as some vcpus are spinning, waiting for a
lock to be released, while the vcpu holding the lock may not be
running at all.

The solution is to trap blocking WFEs and tell KVM that we're
now spinning. This ensures that other vpus will get a scheduling
boost, allowing the lock to be released more quickly. Also, using
CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT slightly improves the performance
when the VM is severely overcommited.

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/kvm_arm.h |  8 ++++++--
 arch/arm64/kvm/Kconfig           |  1 +
 arch/arm64/kvm/handle_exit.c     | 18 +++++++++++++-----
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index a5f28e2..c98ef47 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -63,6 +63,7 @@
  * TAC:		Trap ACTLR
  * TSC:		Trap SMC
  * TSW:		Trap cache operations by set/way
+ * TWE:		Trap WFE
  * TWI:		Trap WFI
  * TIDCP:	Trap L2CTLR/L2ECTLR
  * BSU_IS:	Upgrade barriers to the inner shareable domain
@@ -72,8 +73,9 @@
  * FMO:		Override CPSR.F and enable signaling with VF
  * SWIO:	Turn set/way invalidates into set/way clean+invalidate
  */
-#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
-			 HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
+#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
+			 HCR_BSU_IS | HCR_FB | HCR_TAC | \
+			 HCR_AMO | HCR_IMO | HCR_FMO | \
 			 HCR_SWIO | HCR_TIDCP | HCR_RW)
 #define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF)
 
@@ -242,4 +244,6 @@
 
 #define ESR_EL2_EC_xABT_xFSR_EXTABT	0x10
 
+#define ESR_EL2_EC_WFI_ISS_WFE	(1 << 0)
+
 #endif /* __ARM64_KVM_ARM_H__ */
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 21e9082..4480ab3 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -21,6 +21,7 @@ config KVM
 	select MMU_NOTIFIER
 	select PREEMPT_NOTIFIERS
 	select ANON_INODES
+	select HAVE_KVM_CPU_RELAX_INTERCEPT
 	select KVM_MMIO
 	select KVM_ARM_HOST
 	select KVM_ARM_VGIC
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 9beaca03..8da5606 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -47,21 +47,29 @@ static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
 }
 
 /**
- * kvm_handle_wfi - handle a wait-for-interrupts instruction executed by a guest
+ * kvm_handle_wfx - handle a wait-for-interrupts or wait-for-event
+ *		    instruction executed by a guest
+ *
  * @vcpu:	the vcpu pointer
  *
- * Simply call kvm_vcpu_block(), which will halt execution of
+ * WFE: Yield the CPU and come back to this vcpu when the scheduler
+ * decides to.
+ * WFI: Simply call kvm_vcpu_block(), which will halt execution of
  * world-switches and schedule other host processes until there is an
  * incoming IRQ or FIQ to the VM.
  */
-static int kvm_handle_wfi(struct kvm_vcpu *vcpu, struct kvm_run *run)
+static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	kvm_vcpu_block(vcpu);
+	if (kvm_vcpu_get_hsr(vcpu) & ESR_EL2_EC_WFI_ISS_WFE)
+		kvm_vcpu_on_spin(vcpu);
+	else
+		kvm_vcpu_block(vcpu);
+
 	return 1;
 }
 
 static exit_handle_fn arm_exit_handlers[] = {
-	[ESR_EL2_EC_WFI]	= kvm_handle_wfi,
+	[ESR_EL2_EC_WFI]	= kvm_handle_wfx,
 	[ESR_EL2_EC_CP15_32]	= kvm_handle_cp15_32,
 	[ESR_EL2_EC_CP15_64]	= kvm_handle_cp15_64,
 	[ESR_EL2_EC_CP14_MR]	= kvm_handle_cp14_access,
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-08 10:07 [GIT PULL] KVM/arm64 fixes for 3.13 Marc Zyngier
  2013-11-08 10:07 ` [PATCH 1/3] arm64: KVM: Yield CPU when vcpu executes a WFE Marc Zyngier
@ 2013-11-08 10:07 ` Marc Zyngier
  2013-11-11 11:04   ` Paolo Bonzini
  2013-11-08 10:07 ` [PATCH 3/3] arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu Marc Zyngier
  2 siblings, 1 reply; 19+ messages in thread
From: Marc Zyngier @ 2013-11-08 10:07 UTC (permalink / raw)
  To: Gleb Natapov, Paolo Bonzini; +Cc: kvm, kvmarm, Christoffer Dall

Do the necessary byteswap when host and guest have different
views of the universe. Actually, the only case we need to take
care of is when the guest is BE. All the other cases are naturally
handled.

Also be careful about endianness when the data is being memcopy-ed
from/to the run buffer.

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_emulate.h   | 41 +++++++++++++++++
 arch/arm/kvm/mmio.c                  | 86 +++++++++++++++++++++++++++++++-----
 arch/arm64/include/asm/kvm_emulate.h | 48 ++++++++++++++++++++
 3 files changed, 164 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h
index a464e8d..8a6be05 100644
--- a/arch/arm/include/asm/kvm_emulate.h
+++ b/arch/arm/include/asm/kvm_emulate.h
@@ -157,4 +157,45 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
+static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
+{
+	return !!(*vcpu_cpsr(vcpu) & PSR_E_BIT);
+}
+
+static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
+						    unsigned long data,
+						    unsigned int len)
+{
+	if (kvm_vcpu_is_be(vcpu)) {
+		switch (len) {
+		case 1:
+			return data & 0xff;
+		case 2:
+			return be16_to_cpu(data & 0xffff);
+		default:
+			return be32_to_cpu(data);
+		}
+	}
+
+	return data;		/* Leave LE untouched */
+}
+
+static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
+						    unsigned long data,
+						    unsigned int len)
+{
+	if (kvm_vcpu_is_be(vcpu)) {
+		switch (len) {
+		case 1:
+			return data & 0xff;
+		case 2:
+			return cpu_to_be16(data & 0xffff);
+		default:
+			return cpu_to_be32(data);
+		}
+	}
+
+	return data;		/* Leave LE untouched */
+}
+
 #endif /* __ARM_KVM_EMULATE_H__ */
diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index 0c25d94..4cb5a93 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -23,6 +23,68 @@
 
 #include "trace.h"
 
+static void mmio_write_buf(char *buf, unsigned int len, unsigned long data)
+{
+	void *datap = NULL;
+	union {
+		u8	byte;
+		u16	hword;
+		u32	word;
+		u64	dword;
+	} tmp;
+
+	switch (len) {
+	case 1:
+		tmp.byte	= data;
+		datap		= &tmp.byte;
+		break;
+	case 2:
+		tmp.hword	= data;
+		datap		= &tmp.hword;
+		break;
+	case 4:
+		tmp.word	= data;
+		datap		= &tmp.word;
+		break;
+	case 8:
+		tmp.dword	= data;
+		datap		= &tmp.dword;
+		break;
+	}
+
+	memcpy(buf, datap, len);
+}
+
+static unsigned long mmio_read_buf(char *buf, unsigned int len)
+{
+	unsigned long data = 0;
+	union {
+		u16	hword;
+		u32	word;
+		u64	dword;
+	} tmp;
+
+	switch (len) {
+	case 1:
+		data = buf[0];
+		break;
+	case 2:
+		memcpy(&tmp.hword, buf, len);
+		data = tmp.hword;
+		break;
+	case 4:
+		memcpy(&tmp.word, buf, len);
+		data = tmp.word;
+		break;
+	case 8:
+		memcpy(&tmp.dword, buf, len);
+		data = tmp.dword;
+		break;
+	}
+
+	return data;
+}
+
 /**
  * kvm_handle_mmio_return -- Handle MMIO loads after user space emulation
  * @vcpu: The VCPU pointer
@@ -33,28 +95,27 @@
  */
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
-	unsigned long *dest;
+	unsigned long data;
 	unsigned int len;
 	int mask;
 
 	if (!run->mmio.is_write) {
-		dest = vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt);
-		*dest = 0;
-
 		len = run->mmio.len;
 		if (len > sizeof(unsigned long))
 			return -EINVAL;
 
-		memcpy(dest, run->mmio.data, len);
-
-		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
-				*((u64 *)run->mmio.data));
+		data = mmio_read_buf(run->mmio.data, len);
 
 		if (vcpu->arch.mmio_decode.sign_extend &&
 		    len < sizeof(unsigned long)) {
 			mask = 1U << ((len * 8) - 1);
-			*dest = (*dest ^ mask) - mask;
+			data = (data ^ mask) - mask;
 		}
+
+		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
+			       data);
+		data = vcpu_data_host_to_guest(vcpu, data, len);
+		*vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt) = data;
 	}
 
 	return 0;
@@ -105,6 +166,7 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa)
 {
 	struct kvm_exit_mmio mmio;
+	unsigned long data;
 	unsigned long rt;
 	int ret;
 
@@ -125,13 +187,15 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	}
 
 	rt = vcpu->arch.mmio_decode.rt;
+	data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt), mmio.len);
+
 	trace_kvm_mmio((mmio.is_write) ? KVM_TRACE_MMIO_WRITE :
 					 KVM_TRACE_MMIO_READ_UNSATISFIED,
 			mmio.len, fault_ipa,
-			(mmio.is_write) ? *vcpu_reg(vcpu, rt) : 0);
+			(mmio.is_write) ? data : 0);
 
 	if (mmio.is_write)
-		memcpy(mmio.data, vcpu_reg(vcpu, rt), mmio.len);
+		mmio_write_buf(mmio.data, mmio.len, data);
 
 	if (vgic_handle_mmio(vcpu, run, &mmio))
 		return 1;
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index eec0738..b016577 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -177,4 +177,52 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
+static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
+{
+	if (vcpu_mode_is_32bit(vcpu))
+		return !!(*vcpu_cpsr(vcpu) & COMPAT_PSR_E_BIT);
+
+	return !!(vcpu_sys_reg(vcpu, SCTLR_EL1) & (1 << 25));
+}
+
+static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
+						    unsigned long data,
+						    unsigned int len)
+{
+	if (kvm_vcpu_is_be(vcpu)) {
+		switch (len) {
+		case 1:
+			return data & 0xff;
+		case 2:
+			return be16_to_cpu(data & 0xffff);
+		case 4:
+			return be32_to_cpu(data & 0xffffffff);
+		default:
+			return be64_to_cpu(data);
+		}
+	}
+
+	return data;		/* Leave LE untouched */
+}
+
+static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
+						    unsigned long data,
+						    unsigned int len)
+{
+	if (kvm_vcpu_is_be(vcpu)) {
+		switch (len) {
+		case 1:
+			return data & 0xff;
+		case 2:
+			return cpu_to_be16(data & 0xffff);
+		case 4:
+			return cpu_to_be32(data & 0xffffffff);
+		default:
+			return cpu_to_be64(data);
+		}
+	}
+
+	return data;		/* Leave LE untouched */
+}
+
 #endif /* __ARM64_KVM_EMULATE_H__ */
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/3] arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu
  2013-11-08 10:07 [GIT PULL] KVM/arm64 fixes for 3.13 Marc Zyngier
  2013-11-08 10:07 ` [PATCH 1/3] arm64: KVM: Yield CPU when vcpu executes a WFE Marc Zyngier
  2013-11-08 10:07 ` [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest Marc Zyngier
@ 2013-11-08 10:07 ` Marc Zyngier
  2 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2013-11-08 10:07 UTC (permalink / raw)
  To: Gleb Natapov, Paolo Bonzini; +Cc: kvm, kvmarm, Christoffer Dall

When booting a vcpu using PSCI, make sure we start it with the
endianness of the caller. Otherwise, secondaries can be pretty
unhappy to execute a BE kernel in LE mode...

This conforms to PSCI spec Rev B, 5.13.3.

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_emulate.h   | 5 +++++
 arch/arm/kvm/psci.c                  | 4 ++++
 arch/arm64/include/asm/kvm_emulate.h | 8 ++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h
index 8a6be05..e844b33 100644
--- a/arch/arm/include/asm/kvm_emulate.h
+++ b/arch/arm/include/asm/kvm_emulate.h
@@ -157,6 +157,11 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
+static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
+{
+	*vcpu_cpsr(vcpu) |= PSR_E_BIT;
+}
+
 static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
 {
 	return !!(*vcpu_cpsr(vcpu) & PSR_E_BIT);
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 86a693a..ae0e06b 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -62,6 +62,10 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 		vcpu_set_thumb(vcpu);
 	}
 
+	/* Propagate caller endianness */
+	if (kvm_vcpu_is_be(source_vcpu))
+		kvm_vcpu_set_be(vcpu);
+
 	*vcpu_pc(vcpu) = target_pc;
 	vcpu->arch.pause = false;
 	smp_mb();		/* Make sure the above is visible */
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index b016577..db80509 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -177,6 +177,14 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
+static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
+{
+	if (vcpu_mode_is_32bit(vcpu))
+		*vcpu_cpsr(vcpu) |= COMPAT_PSR_E_BIT;
+	else
+		vcpu_sys_reg(vcpu, SCTLR_EL1) |= (1 << 25);
+}
+
 static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
 {
 	if (vcpu_mode_is_32bit(vcpu))
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-08 10:07 ` [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest Marc Zyngier
@ 2013-11-11 11:04   ` Paolo Bonzini
  2013-11-11 14:56     ` Christoffer Dall
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-11 11:04 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Gleb Natapov, kvm, kvmarm, Christoffer Dall

Il 08/11/2013 11:07, Marc Zyngier ha scritto:
> Do the necessary byteswap when host and guest have different
> views of the universe. Actually, the only case we need to take
> care of is when the guest is BE. All the other cases are naturally
> handled.
> 
> Also be careful about endianness when the data is being memcopy-ed
> from/to the run buffer.
> 
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_emulate.h   | 41 +++++++++++++++++
>  arch/arm/kvm/mmio.c                  | 86 +++++++++++++++++++++++++++++++-----
>  arch/arm64/include/asm/kvm_emulate.h | 48 ++++++++++++++++++++
>  3 files changed, 164 insertions(+), 11 deletions(-)

Christoffer and Marc, please coordinate so that arm+arm64 patch go only
through one person.  The conflict between your two pull requests was not
really necessary.

I'm pulling both anyway.

Paolo

> diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h
> index a464e8d..8a6be05 100644
> --- a/arch/arm/include/asm/kvm_emulate.h
> +++ b/arch/arm/include/asm/kvm_emulate.h
> @@ -157,4 +157,45 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
>  }
>  
> +static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
> +{
> +	return !!(*vcpu_cpsr(vcpu) & PSR_E_BIT);
> +}
> +
> +static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return be16_to_cpu(data & 0xffff);
> +		default:
> +			return be32_to_cpu(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
> +static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return cpu_to_be16(data & 0xffff);
> +		default:
> +			return cpu_to_be32(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
>  #endif /* __ARM_KVM_EMULATE_H__ */
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index 0c25d94..4cb5a93 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -23,6 +23,68 @@
>  
>  #include "trace.h"
>  
> +static void mmio_write_buf(char *buf, unsigned int len, unsigned long data)
> +{
> +	void *datap = NULL;
> +	union {
> +		u8	byte;
> +		u16	hword;
> +		u32	word;
> +		u64	dword;
> +	} tmp;
> +
> +	switch (len) {
> +	case 1:
> +		tmp.byte	= data;
> +		datap		= &tmp.byte;
> +		break;
> +	case 2:
> +		tmp.hword	= data;
> +		datap		= &tmp.hword;
> +		break;
> +	case 4:
> +		tmp.word	= data;
> +		datap		= &tmp.word;
> +		break;
> +	case 8:
> +		tmp.dword	= data;
> +		datap		= &tmp.dword;
> +		break;
> +	}
> +
> +	memcpy(buf, datap, len);
> +}
> +
> +static unsigned long mmio_read_buf(char *buf, unsigned int len)
> +{
> +	unsigned long data = 0;
> +	union {
> +		u16	hword;
> +		u32	word;
> +		u64	dword;
> +	} tmp;
> +
> +	switch (len) {
> +	case 1:
> +		data = buf[0];
> +		break;
> +	case 2:
> +		memcpy(&tmp.hword, buf, len);
> +		data = tmp.hword;
> +		break;
> +	case 4:
> +		memcpy(&tmp.word, buf, len);
> +		data = tmp.word;
> +		break;
> +	case 8:
> +		memcpy(&tmp.dword, buf, len);
> +		data = tmp.dword;
> +		break;
> +	}
> +
> +	return data;
> +}
> +
>  /**
>   * kvm_handle_mmio_return -- Handle MMIO loads after user space emulation
>   * @vcpu: The VCPU pointer
> @@ -33,28 +95,27 @@
>   */
>  int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
> -	unsigned long *dest;
> +	unsigned long data;
>  	unsigned int len;
>  	int mask;
>  
>  	if (!run->mmio.is_write) {
> -		dest = vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt);
> -		*dest = 0;
> -
>  		len = run->mmio.len;
>  		if (len > sizeof(unsigned long))
>  			return -EINVAL;
>  
> -		memcpy(dest, run->mmio.data, len);
> -
> -		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
> -				*((u64 *)run->mmio.data));
> +		data = mmio_read_buf(run->mmio.data, len);
>  
>  		if (vcpu->arch.mmio_decode.sign_extend &&
>  		    len < sizeof(unsigned long)) {
>  			mask = 1U << ((len * 8) - 1);
> -			*dest = (*dest ^ mask) - mask;
> +			data = (data ^ mask) - mask;
>  		}
> +
> +		trace_kvm_mmio(KVM_TRACE_MMIO_READ, len, run->mmio.phys_addr,
> +			       data);
> +		data = vcpu_data_host_to_guest(vcpu, data, len);
> +		*vcpu_reg(vcpu, vcpu->arch.mmio_decode.rt) = data;
>  	}
>  
>  	return 0;
> @@ -105,6 +166,7 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		 phys_addr_t fault_ipa)
>  {
>  	struct kvm_exit_mmio mmio;
> +	unsigned long data;
>  	unsigned long rt;
>  	int ret;
>  
> @@ -125,13 +187,15 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  	}
>  
>  	rt = vcpu->arch.mmio_decode.rt;
> +	data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt), mmio.len);
> +
>  	trace_kvm_mmio((mmio.is_write) ? KVM_TRACE_MMIO_WRITE :
>  					 KVM_TRACE_MMIO_READ_UNSATISFIED,
>  			mmio.len, fault_ipa,
> -			(mmio.is_write) ? *vcpu_reg(vcpu, rt) : 0);
> +			(mmio.is_write) ? data : 0);
>  
>  	if (mmio.is_write)
> -		memcpy(mmio.data, vcpu_reg(vcpu, rt), mmio.len);
> +		mmio_write_buf(mmio.data, mmio.len, data);
>  
>  	if (vgic_handle_mmio(vcpu, run, &mmio))
>  		return 1;
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index eec0738..b016577 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -177,4 +177,52 @@ static inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
>  	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
>  }
>  
> +static inline bool kvm_vcpu_is_be(struct kvm_vcpu *vcpu)
> +{
> +	if (vcpu_mode_is_32bit(vcpu))
> +		return !!(*vcpu_cpsr(vcpu) & COMPAT_PSR_E_BIT);
> +
> +	return !!(vcpu_sys_reg(vcpu, SCTLR_EL1) & (1 << 25));
> +}
> +
> +static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return be16_to_cpu(data & 0xffff);
> +		case 4:
> +			return be32_to_cpu(data & 0xffffffff);
> +		default:
> +			return be64_to_cpu(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
> +static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> +						    unsigned long data,
> +						    unsigned int len)
> +{
> +	if (kvm_vcpu_is_be(vcpu)) {
> +		switch (len) {
> +		case 1:
> +			return data & 0xff;
> +		case 2:
> +			return cpu_to_be16(data & 0xffff);
> +		case 4:
> +			return cpu_to_be32(data & 0xffffffff);
> +		default:
> +			return cpu_to_be64(data);
> +		}
> +	}
> +
> +	return data;		/* Leave LE untouched */
> +}
> +
>  #endif /* __ARM64_KVM_EMULATE_H__ */
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 11:04   ` Paolo Bonzini
@ 2013-11-11 14:56     ` Christoffer Dall
  2013-11-11 15:10       ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Christoffer Dall @ 2013-11-11 14:56 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marc Zyngier, Gleb Natapov, kvm, kvmarm

On 11 November 2013 03:04, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 08/11/2013 11:07, Marc Zyngier ha scritto:
>> Do the necessary byteswap when host and guest have different
>> views of the universe. Actually, the only case we need to take
>> care of is when the guest is BE. All the other cases are naturally
>> handled.
>>
>> Also be careful about endianness when the data is being memcopy-ed
>> from/to the run buffer.
>>
>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/include/asm/kvm_emulate.h   | 41 +++++++++++++++++
>>  arch/arm/kvm/mmio.c                  | 86 +++++++++++++++++++++++++++++++-----
>>  arch/arm64/include/asm/kvm_emulate.h | 48 ++++++++++++++++++++
>>  3 files changed, 164 insertions(+), 11 deletions(-)
>
> Christoffer and Marc, please coordinate so that arm+arm64 patch go only
> through one person.  The conflict between your two pull requests was not
> really necessary.
>
I don't think the same patch was in both pull requests, right?

Is what you're saying that any patch that touches arm and arm64 should
always come from one of us so that you reduce the chance of a merge
conflict?  There would still be the case where I carry those arm/arm64
patches but th arm64 changes conflict with those in Marc's tree, no?

-Christoffer

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 14:56     ` Christoffer Dall
@ 2013-11-11 15:10       ` Paolo Bonzini
  2013-11-11 15:49         ` Christoffer Dall
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-11 15:10 UTC (permalink / raw)
  To: Christoffer Dall; +Cc: Marc Zyngier, Gleb Natapov, kvm, kvmarm

Il 11/11/2013 15:56, Christoffer Dall ha scritto:
>> > Christoffer and Marc, please coordinate so that arm+arm64 patch go only
>> > through one person.  The conflict between your two pull requests was not
>> > really necessary.
>> >
> I don't think the same patch was in both pull requests, right?

No, this patch conflicted with "arm/arm64: KVM: PSCI: use MPIDR to
identify a target CPU" from your tree.  Both touched
arch/arm/include/asm/kvm_emulate.h and arch/arm64/include/asm/kvm_emulate.h

> Is what you're saying that any patch that touches arm and arm64 should
> always come from one of us so that you reduce the chance of a merge
> conflict?

Yes---note that it doesn't have to be *always* the same person; whatever
works best for you probably works best for me too.  In this case all of
them were written by Marc, it would have been even simpler for you to
just give your Acked-by instead of picking up "arm/arm64: KVM: PSCI: use
MPIDR to identify a target CPU".

> There would still be the case where I carry those arm/arm64
> patches but th arm64 changes conflict with those in Marc's tree, no?

Yes, that can still happen.  Conflicts are not bad, only inconsistencies
are.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 15:10       ` Paolo Bonzini
@ 2013-11-11 15:49         ` Christoffer Dall
  2013-11-11 17:56           ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Christoffer Dall @ 2013-11-11 15:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marc Zyngier, Gleb Natapov, kvm, kvmarm

On 11 November 2013 07:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 11/11/2013 15:56, Christoffer Dall ha scritto:
>>> > Christoffer and Marc, please coordinate so that arm+arm64 patch go only
>>> > through one person.  The conflict between your two pull requests was not
>>> > really necessary.
>>> >
>> I don't think the same patch was in both pull requests, right?
>
> No, this patch conflicted with "arm/arm64: KVM: PSCI: use MPIDR to
> identify a target CPU" from your tree.  Both touched
> arch/arm/include/asm/kvm_emulate.h and arch/arm64/include/asm/kvm_emulate.h
>
>> Is what you're saying that any patch that touches arm and arm64 should
>> always come from one of us so that you reduce the chance of a merge
>> conflict?
>
> Yes---note that it doesn't have to be *always* the same person; whatever
> works best for you probably works best for me too.  In this case all of
> them were written by Marc, it would have been even simpler for you to
> just give your Acked-by instead of picking up "arm/arm64: KVM: PSCI: use
> MPIDR to identify a target CPU".

I don't think it would have made much sense - that patch was part of a
series that was touching mainly arch/arm/kvm/* and therefore I
included it in my pull.  It would have been strange to have a
kvm-arm-next tree that included 75% of the functionality because Marc
happens to have another patch that touches arch/arm and arch/arm64 and
have two untestable trees until the merge window...

>
>> There would still be the case where I carry those arm/arm64
>> patches but the arm64 changes conflict with those in Marc's tree, no?
>
> Yes, that can still happen.  Conflicts are not bad, only inconsistencies
> are.
>
Not sure what you mean and where we could be more consistent to make
life easier for you.  You say it should always come from the same
person, but not necessary always from the same person?

Note: I have no problem giving my ack to patches or follow any
procedure that makes it easier, but I thought these pull requests were
quite clean (albeit a bit late).

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 15:49         ` Christoffer Dall
@ 2013-11-11 17:56           ` Paolo Bonzini
  2013-11-11 18:03             ` Peter Maydell
                               ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-11 17:56 UTC (permalink / raw)
  To: Christoffer Dall; +Cc: Marc Zyngier, Gleb Natapov, kvm, kvmarm

Il 11/11/2013 16:49, Christoffer Dall ha scritto:
> I don't think it would have made much sense - that patch was part of a
> series that was touching mainly arch/arm/kvm/* and therefore I
> included it in my pull.  It would have been strange to have a
> kvm-arm-next tree that included 75% of the functionality because Marc
> happens to have another patch that touches arch/arm and arch/arm64 and
> have two untestable trees until the merge window...

Yes, I found the original series now at
http://thread.gmane.org/gmane.linux.ports.arm.kernel/274722/.

BTW, why did the arm/arm64 patch move from patch 1 in Marc's post to
patch 4 here?  Also, the description says "this also requires a patch to
kvmtool so the generated DT matches the expectations of the guest
(posted separately)".  Does this apply to QEMU as well?  If so, can you
please point me to the QEMU patch?  How does this patch affect guest
ABI, and is guest ABI not yet considered stable for KVM ARM?

Sorry for the question storm. :)

>>> There would still be the case where I carry those arm/arm64
>>> patches but the arm64 changes conflict with those in Marc's tree, no?
>>
>> Yes, that can still happen.  Conflicts are not bad, only inconsistencies
>> are.
>
> Not sure what you mean and where we could be more consistent to make
> life easier for you.  You say it should always come from the same
> person, but not necessary always from the same person?
> 
> Note: I have no problem giving my ack to patches or follow any
> procedure that makes it easier, but I thought these pull requests were
> quite clean (albeit a bit late).

The pull requests were clean and my life wasn't complicated much...  On
the other hand I'm trying to understand if there's something that can be
improved because the conflict surprised me.  Right now, in fact, it's
not even entirely clear to me why ARM and ARM64 have separate maintainers.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 17:56           ` Paolo Bonzini
@ 2013-11-11 18:03             ` Peter Maydell
  2013-11-11 18:05               ` Paolo Bonzini
  2013-11-11 18:26             ` Marc Zyngier
  2013-11-11 18:39             ` Christoffer Dall
  2 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2013-11-11 18:03 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Christoffer Dall, kvm, kvmarm

On 11 November 2013 17:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> BTW, why did the arm/arm64 patch move from patch 1 in Marc's post to
> patch 4 here?  Also, the description says "this also requires a patch to
> kvmtool so the generated DT matches the expectations of the guest
> (posted separately)".  Does this apply to QEMU as well?  If so, can you
> please point me to the QEMU patch?  How does this patch affect guest
> ABI, and is guest ABI not yet considered stable for KVM ARM?

QEMU currently insists on 4 CPUs max for A15, so the
question doesn't come up in that case.

-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 18:03             ` Peter Maydell
@ 2013-11-11 18:05               ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-11 18:05 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Christoffer Dall, kvm, kvmarm

Il 11/11/2013 19:03, Peter Maydell ha scritto:
>> How does this patch affect guest
>> ABI, and is guest ABI not yet considered stable for KVM ARM?
>
> QEMU currently insists on 4 CPUs max for A15, so the
> question doesn't come up in that case.

Still, does the guest ABI not matter only for kvmtool, or also for QEMU?

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 17:56           ` Paolo Bonzini
  2013-11-11 18:03             ` Peter Maydell
@ 2013-11-11 18:26             ` Marc Zyngier
  2013-11-11 18:41               ` Christoffer Dall
  2013-11-11 18:41               ` Paolo Bonzini
  2013-11-11 18:39             ` Christoffer Dall
  2 siblings, 2 replies; 19+ messages in thread
From: Marc Zyngier @ 2013-11-11 18:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Christoffer Dall, Gleb Natapov, kvm, kvmarm, Peter Maydell

Paolo,

On 11/11/13 17:56, Paolo Bonzini wrote:
> Il 11/11/2013 16:49, Christoffer Dall ha scritto:
>> I don't think it would have made much sense - that patch was part of a
>> series that was touching mainly arch/arm/kvm/* and therefore I
>> included it in my pull.  It would have been strange to have a
>> kvm-arm-next tree that included 75% of the functionality because Marc
>> happens to have another patch that touches arch/arm and arch/arm64 and
>> have two untestable trees until the merge window...
> 
> Yes, I found the original series now at
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/274722/.
> 
> BTW, why did the arm/arm64 patch move from patch 1 in Marc's post to
> patch 4 here?  Also, the description says "this also requires a patch to
> kvmtool so the generated DT matches the expectations of the guest
> (posted separately)".  Does this apply to QEMU as well?  If so, can you
> please point me to the QEMU patch?  How does this patch affect guest
> ABI, and is guest ABI not yet considered stable for KVM ARM?
> 
> Sorry for the question storm. :)

There is no QEMU patch so far, as Peter (CC-ed) doesn't want to support
such a configuration and prefers to stick to a setups for which an
actual platform exist in QEMU.

This doesn't really change the ABI:
- Configurations with more than 4 CPUs were rejected until now (KVM only
dealt with a single cluster), and are now accepted
- CPUs 4 to 7 are part of a second cluster, which is reflected in the
MPIDR register, just like on HW.
- The kvmtool patch only deals with DT generation, which was buggy and
didn't deal properly with multiple clusters.

So anything that was working before still is, and things that were
wrongly advertised as working are now fixed. All in all, this patch
series is more a set of bug-fixes than anything else.

>>>> There would still be the case where I carry those arm/arm64
>>>> patches but the arm64 changes conflict with those in Marc's tree, no?
>>>
>>> Yes, that can still happen.  Conflicts are not bad, only inconsistencies
>>> are.
>>
>> Not sure what you mean and where we could be more consistent to make
>> life easier for you.  You say it should always come from the same
>> person, but not necessary always from the same person?
>>
>> Note: I have no problem giving my ack to patches or follow any
>> procedure that makes it easier, but I thought these pull requests were
>> quite clean (albeit a bit late).
> 
> The pull requests were clean and my life wasn't complicated much...  On
> the other hand I'm trying to understand if there's something that can be
> improved because the conflict surprised me.  Right now, in fact, it's
> not even entirely clear to me why ARM and ARM64 have separate maintainers.

Mostly because arm64 was developed and merged before any kind of useful
documentation was publicly available. As I've written most of the code,
it was only logical that I'd assume responsibility for it.

Christoffer and I are actually working quite well together, and I don't
think there is much to improve, short of sharing a common git tree. And
to be perfectly clear, I wouldn't mind if we were written down as
co-maintainers for both ports...

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 17:56           ` Paolo Bonzini
  2013-11-11 18:03             ` Peter Maydell
  2013-11-11 18:26             ` Marc Zyngier
@ 2013-11-11 18:39             ` Christoffer Dall
  2 siblings, 0 replies; 19+ messages in thread
From: Christoffer Dall @ 2013-11-11 18:39 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marc Zyngier, Gleb Natapov, kvm, kvmarm

On 11 November 2013 09:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 11/11/2013 16:49, Christoffer Dall ha scritto:
>> I don't think it would have made much sense - that patch was part of a
>> series that was touching mainly arch/arm/kvm/* and therefore I
>> included it in my pull.  It would have been strange to have a
>> kvm-arm-next tree that included 75% of the functionality because Marc
>> happens to have another patch that touches arch/arm and arch/arm64 and
>> have two untestable trees until the merge window...
>
> Yes, I found the original series now at
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/274722/.
>
> BTW, why did the arm/arm64 patch move from patch 1 in Marc's post to
> patch 4 here?

hmm, probably I just goofed something up when exporting the mbox from
mutt - it made sense for me the psci part was the last one as well I
guess, but I can't say I applied my brain to the reordering.

> Also, the description says "this also requires a patch to
> kvmtool so the generated DT matches the expectations of the guest
> (posted separately)".  Does this apply to QEMU as well?  If so, can you
> please point me to the QEMU patch?  How does this patch affect guest
> ABI, and is guest ABI not yet considered stable for KVM ARM?
>
> Sorry for the question storm. :)
>
>>>> There would still be the case where I carry those arm/arm64
>>>> patches but the arm64 changes conflict with those in Marc's tree, no?
>>>
>>> Yes, that can still happen.  Conflicts are not bad, only inconsistencies
>>> are.
>>
>> Not sure what you mean and where we could be more consistent to make
>> life easier for you.  You say it should always come from the same
>> person, but not necessary always from the same person?
>>
>> Note: I have no problem giving my ack to patches or follow any
>> procedure that makes it easier, but I thought these pull requests were
>> quite clean (albeit a bit late).
>
> The pull requests were clean and my life wasn't complicated much...  On
> the other hand I'm trying to understand if there's something that can be
> improved because the conflict surprised me.  Right now, in fact, it's
> not even entirely clear to me why ARM and ARM64 have separate maintainers.
>
Well, KVM/ARM was my thing originally, I was, and am, the maintainer
when it was merged into Linux, then Marc started doing a lot of work
on there, and he did the ARM64 port and then we ended up this way.

-Christoffer

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 18:26             ` Marc Zyngier
@ 2013-11-11 18:41               ` Christoffer Dall
  2013-11-11 18:41               ` Paolo Bonzini
  1 sibling, 0 replies; 19+ messages in thread
From: Christoffer Dall @ 2013-11-11 18:41 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Paolo Bonzini, Gleb Natapov, kvm, kvmarm, Peter Maydell

On 11 November 2013 10:26, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Paolo,
>
> On 11/11/13 17:56, Paolo Bonzini wrote:
>> Il 11/11/2013 16:49, Christoffer Dall ha scritto:
>>> I don't think it would have made much sense - that patch was part of a
>>> series that was touching mainly arch/arm/kvm/* and therefore I
>>> included it in my pull.  It would have been strange to have a
>>> kvm-arm-next tree that included 75% of the functionality because Marc
>>> happens to have another patch that touches arch/arm and arch/arm64 and
>>> have two untestable trees until the merge window...
>>
>> Yes, I found the original series now at
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/274722/.
>>
>> BTW, why did the arm/arm64 patch move from patch 1 in Marc's post to
>> patch 4 here?  Also, the description says "this also requires a patch to
>> kvmtool so the generated DT matches the expectations of the guest
>> (posted separately)".  Does this apply to QEMU as well?  If so, can you
>> please point me to the QEMU patch?  How does this patch affect guest
>> ABI, and is guest ABI not yet considered stable for KVM ARM?
>>
>> Sorry for the question storm. :)
>
> There is no QEMU patch so far, as Peter (CC-ed) doesn't want to support
> such a configuration and prefers to stick to a setups for which an
> actual platform exist in QEMU.
>
> This doesn't really change the ABI:
> - Configurations with more than 4 CPUs were rejected until now (KVM only
> dealt with a single cluster), and are now accepted
> - CPUs 4 to 7 are part of a second cluster, which is reflected in the
> MPIDR register, just like on HW.
> - The kvmtool patch only deals with DT generation, which was buggy and
> didn't deal properly with multiple clusters.
>
> So anything that was working before still is, and things that were
> wrongly advertised as working are now fixed. All in all, this patch
> series is more a set of bug-fixes than anything else.
>
>>>>> There would still be the case where I carry those arm/arm64
>>>>> patches but the arm64 changes conflict with those in Marc's tree, no?
>>>>
>>>> Yes, that can still happen.  Conflicts are not bad, only inconsistencies
>>>> are.
>>>
>>> Not sure what you mean and where we could be more consistent to make
>>> life easier for you.  You say it should always come from the same
>>> person, but not necessary always from the same person?
>>>
>>> Note: I have no problem giving my ack to patches or follow any
>>> procedure that makes it easier, but I thought these pull requests were
>>> quite clean (albeit a bit late).
>>
>> The pull requests were clean and my life wasn't complicated much...  On
>> the other hand I'm trying to understand if there's something that can be
>> improved because the conflict surprised me.  Right now, in fact, it's
>> not even entirely clear to me why ARM and ARM64 have separate maintainers.
>
> Mostly because arm64 was developed and merged before any kind of useful
> documentation was publicly available. As I've written most of the code,
> it was only logical that I'd assume responsibility for it.
>
> Christoffer and I are actually working quite well together, and I don't
> think there is much to improve, short of sharing a common git tree. And
> to be perfectly clear, I wouldn't mind if we were written down as
> co-maintainers for both ports...
>
I completely agree, I feel the collaboration works well too, and we
can change the git workflow a bit and list us both as co-maintainers
if required.  It seems this is how it effectively works today, I don't
merge anything unless Marc gives his ack and I believe it's the other
way around too.

-Christoffer

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 18:26             ` Marc Zyngier
  2013-11-11 18:41               ` Christoffer Dall
@ 2013-11-11 18:41               ` Paolo Bonzini
  2013-11-12  9:41                 ` Andrew Jones
  1 sibling, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-11 18:41 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Christoffer Dall, Gleb Natapov, kvm, kvmarm, Peter Maydell

Il 11/11/2013 19:26, Marc Zyngier ha scritto:
>> > The pull requests were clean and my life wasn't complicated much...  On
>> > the other hand I'm trying to understand if there's something that can be
>> > improved because the conflict surprised me.  Right now, in fact, it's
>> > not even entirely clear to me why ARM and ARM64 have separate maintainers.
> Mostly because arm64 was developed and merged before any kind of useful
> documentation was publicly available. As I've written most of the code,
> it was only logical that I'd assume responsibility for it.

That was my understanding as well.

> Christoffer and I are actually working quite well together, and I don't
> think there is much to improve, short of sharing a common git tree. And
> to be perfectly clear, I wouldn't mind if we were written down as
> co-maintainers for both ports...

Then go for it. :)  Send a patch to MAINTAINERS, get an Acked-by from
Christoffer and I'll apply it.

Gleb and I share the git tree and hand it off "formally" by email every
1 or 2 weeks to the other person.  After the email is sent, the sender
should no longer push to the shared tree.  This however is by no means
the only way to proceed, having separate trees and sending separate pull
requests works well too.  I would not mind the occasional conflict, and
I'd be hardly surprised.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-11 18:41               ` Paolo Bonzini
@ 2013-11-12  9:41                 ` Andrew Jones
  2013-11-12 10:03                   ` Marc Zyngier
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Jones @ 2013-11-12  9:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marc Zyngier, kvmarm, kvm

On Mon, Nov 11, 2013 at 07:41:30PM +0100, Paolo Bonzini wrote:
> Il 11/11/2013 19:26, Marc Zyngier ha scritto:
> >> > The pull requests were clean and my life wasn't complicated much...  On
> >> > the other hand I'm trying to understand if there's something that can be
> >> > improved because the conflict surprised me.  Right now, in fact, it's
> >> > not even entirely clear to me why ARM and ARM64 have separate maintainers.
> > Mostly because arm64 was developed and merged before any kind of useful
> > documentation was publicly available. As I've written most of the code,
> > it was only logical that I'd assume responsibility for it.
> 
> That was my understanding as well.
> 
> > Christoffer and I are actually working quite well together, and I don't
> > think there is much to improve, short of sharing a common git tree. And
> > to be perfectly clear, I wouldn't mind if we were written down as
> > co-maintainers for both ports...
> 
> Then go for it. :)  Send a patch to MAINTAINERS, get an Acked-by from
> Christoffer and I'll apply it.
> 
> Gleb and I share the git tree and hand it off "formally" by email every
> 1 or 2 weeks to the other person.  After the email is sent, the sender
> should no longer push to the shared tree.  This however is by no means
> the only way to proceed, having separate trees and sending separate pull
> requests works well too.  I would not mind the occasional conflict, and
> I'd be hardly surprised.

I'd cast my vote (if I have one) towards the sharing a tree method. For
those of us scrambling to get caught up with kvmarm, a reduction in the
number of trees and branches we need to track would be a welcome change.

drew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-12  9:41                 ` Andrew Jones
@ 2013-11-12 10:03                   ` Marc Zyngier
  2013-11-12 10:07                     ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Marc Zyngier @ 2013-11-12 10:03 UTC (permalink / raw)
  To: Andrew Jones; +Cc: Paolo Bonzini, kvmarm, kvm

On 12/11/13 09:41, Andrew Jones wrote:
> On Mon, Nov 11, 2013 at 07:41:30PM +0100, Paolo Bonzini wrote:
>> Il 11/11/2013 19:26, Marc Zyngier ha scritto:
>>>>> The pull requests were clean and my life wasn't complicated much...  On
>>>>> the other hand I'm trying to understand if there's something that can be
>>>>> improved because the conflict surprised me.  Right now, in fact, it's
>>>>> not even entirely clear to me why ARM and ARM64 have separate maintainers.
>>> Mostly because arm64 was developed and merged before any kind of useful
>>> documentation was publicly available. As I've written most of the code,
>>> it was only logical that I'd assume responsibility for it.
>>
>> That was my understanding as well.
>>
>>> Christoffer and I are actually working quite well together, and I don't
>>> think there is much to improve, short of sharing a common git tree. And
>>> to be perfectly clear, I wouldn't mind if we were written down as
>>> co-maintainers for both ports...
>>
>> Then go for it. :)  Send a patch to MAINTAINERS, get an Acked-by from
>> Christoffer and I'll apply it.
>>
>> Gleb and I share the git tree and hand it off "formally" by email every
>> 1 or 2 weeks to the other person.  After the email is sent, the sender
>> should no longer push to the shared tree.  This however is by no means
>> the only way to proceed, having separate trees and sending separate pull
>> requests works well too.  I would not mind the occasional conflict, and
>> I'd be hardly surprised.
> 
> I'd cast my vote (if I have one) towards the sharing a tree method. For
> those of us scrambling to get caught up with kvmarm, a reduction in the
> number of trees and branches we need to track would be a welcome change.

Not sure what the benefit would be. We'd go from two trees with
respectively x and y branches, to a single tree with x+y branches.

Christoffer and I tend to work on separate topics, we track what the
other does, and we make sure we don't overlap. And if we do, we shove
the related patches in the same branch. Overall, whether or not we
switch to co-maintainership, I don't expect our workflow to change much.

	M.
-- 
Jazz is not dead. It just smells funny...


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-12 10:03                   ` Marc Zyngier
@ 2013-11-12 10:07                     ` Paolo Bonzini
  2013-11-12 16:30                       ` Christoffer Dall
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2013-11-12 10:07 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Andrew Jones, kvmarm, kvm

Il 12/11/2013 11:03, Marc Zyngier ha scritto:
>> > 
>> > I'd cast my vote (if I have one) towards the sharing a tree method. For
>> > those of us scrambling to get caught up with kvmarm, a reduction in the
>> > number of trees and branches we need to track would be a welcome change.
> Not sure what the benefit would be. We'd go from two trees with
> respectively x and y branches, to a single tree with x+y branches.
> 
> Christoffer and I tend to work on separate topics, we track what the
> other does, and we make sure we don't overlap. And if we do, we shove
> the related patches in the same branch. Overall, whether or not we
> switch to co-maintainership, I don't expect our workflow to change much.

Yes, I think your workflow is fine as is.

Andrew, with two co-maintainers Christoffer and Marc would probably send
more frequent pull requests.  You're probably better off sending them
patches based on kvm/next directly.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest
  2013-11-12 10:07                     ` Paolo Bonzini
@ 2013-11-12 16:30                       ` Christoffer Dall
  0 siblings, 0 replies; 19+ messages in thread
From: Christoffer Dall @ 2013-11-12 16:30 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marc Zyngier, kvmarm, kvm

On 12 November 2013 02:07, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 12/11/2013 11:03, Marc Zyngier ha scritto:
>>> >
>>> > I'd cast my vote (if I have one) towards the sharing a tree method. For
>>> > those of us scrambling to get caught up with kvmarm, a reduction in the
>>> > number of trees and branches we need to track would be a welcome change.
>> Not sure what the benefit would be. We'd go from two trees with
>> respectively x and y branches, to a single tree with x+y branches.
>>
>> Christoffer and I tend to work on separate topics, we track what the
>> other does, and we make sure we don't overlap. And if we do, we shove
>> the related patches in the same branch. Overall, whether or not we
>> switch to co-maintainership, I don't expect our workflow to change much.
>
> Yes, I think your workflow is fine as is.
>
> Andrew, with two co-maintainers Christoffer and Marc would probably send
> more frequent pull requests.  You're probably better off sending them
> patches based on kvm/next directly.
>
Or just ask us if you're working on a feature, and you want to know
where to get the latest sources or what to send pull requests against.

Usually, if it's 32-bit stuff, it's against kvm-arm-next in my tree,
which is equivalent to kvm/next workflow wise.

-Christoffer

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-11-12 16:30 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-08 10:07 [GIT PULL] KVM/arm64 fixes for 3.13 Marc Zyngier
2013-11-08 10:07 ` [PATCH 1/3] arm64: KVM: Yield CPU when vcpu executes a WFE Marc Zyngier
2013-11-08 10:07 ` [PATCH 2/3] arm/arm64: KVM: MMIO support for BE guest Marc Zyngier
2013-11-11 11:04   ` Paolo Bonzini
2013-11-11 14:56     ` Christoffer Dall
2013-11-11 15:10       ` Paolo Bonzini
2013-11-11 15:49         ` Christoffer Dall
2013-11-11 17:56           ` Paolo Bonzini
2013-11-11 18:03             ` Peter Maydell
2013-11-11 18:05               ` Paolo Bonzini
2013-11-11 18:26             ` Marc Zyngier
2013-11-11 18:41               ` Christoffer Dall
2013-11-11 18:41               ` Paolo Bonzini
2013-11-12  9:41                 ` Andrew Jones
2013-11-12 10:03                   ` Marc Zyngier
2013-11-12 10:07                     ` Paolo Bonzini
2013-11-12 16:30                       ` Christoffer Dall
2013-11-11 18:39             ` Christoffer Dall
2013-11-08 10:07 ` [PATCH 3/3] arm/arm64: KVM: PSCI: propagate caller endianness to the incoming vcpu Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.