* [PATCH 0/5] aarch64 BE kvm support
@ 2014-02-12 5:57 Victor Kamensky
2014-02-12 5:57 ` [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code Victor Kamensky
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Guys,
Here is series that enable KVM in V8 BE image. It is addition
on top of previously posted V7 BE KVM support [1].
It was tested on aarch64 fastmodels and APM mustang board.
It was tested only with kvmtool at this point. In case of V8
BE KVM host was tested that V8 BE guest runs fine and V8 LE
guest runs too. Also V8 LE KVM regression was tested on both
V8 LE guest and V8 BE guest. Note for mixed mode Marc's
kvmtool was used and guest image had minor change that treats
all virtio in LE form.
Note first two patches are similar to V7 BE KVM patches. Last
three are new specific for V8 image.
Thanks,
Victor
[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-February/009446.html
Victor Kamensky (5):
ARM64: KVM: MMIO support BE host running LE code
ARM64: KVM: set and get of sys registers in BE case
ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word
ARM64: KVM: vgic_elrsr and vgic_eisr need to by byteswapped in BE case
ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case
arch/arm64/include/asm/kvm_emulate.h | 22 ++++++++++++++++++++
arch/arm64/kvm/hyp.S | 9 ++++++++-
arch/arm64/kvm/sys_regs.c | 39 ++++++++++++++++++++++++++++++------
virt/kvm/arm/vgic.c | 27 +++++++++++++++++++++++--
4 files changed, 88 insertions(+), 9 deletions(-)
--
1.8.1.4
Thanks,
Victor
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
@ 2014-02-12 5:57 ` Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case Victor Kamensky
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
In case of guest CPU running in LE mode and host runs in
BE mode we need byteswap data, so read/write is emulated correctly.
Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
arch/arm64/include/asm/kvm_emulate.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index dd8ecfc..fdc3e21 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -213,6 +213,17 @@ static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
default:
return be64_to_cpu(data);
}
+ } else {
+ switch (len) {
+ case 1:
+ return data & 0xff;
+ case 2:
+ return le16_to_cpu(data & 0xffff);
+ case 4:
+ return le32_to_cpu(data & 0xffffffff);
+ default:
+ return le64_to_cpu(data);
+ }
}
return data; /* Leave LE untouched */
@@ -233,6 +244,17 @@ static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
default:
return cpu_to_be64(data);
}
+ } else {
+ switch (len) {
+ case 1:
+ return data & 0xff;
+ case 2:
+ return cpu_to_le16(data & 0xffff);
+ case 4:
+ return cpu_to_le32(data & 0xffffffff);
+ default:
+ return cpu_to_le64(data);
+ }
}
return data; /* Leave LE untouched */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
2014-02-12 5:57 ` [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code Victor Kamensky
@ 2014-02-12 5:57 ` Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word Victor Kamensky
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
This patch fixes issue of reading and writing V8 sys registers in
BE case. It is similar to V7 "ARM: kvm one_reg coproc set and get
BE fixes" patch.
It changes reg_from_user and reg_to_user functions to have strong
typed 'u64 *val' argument. And it uses endian angnostic way to
pick up righ word from '*val' in case when register size is 4 bytes.
Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
arch/arm64/kvm/sys_regs.c | 39 +++++++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 02e9d09..e7c3e24 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -701,18 +701,45 @@ static struct sys_reg_desc invariant_sys_regs[] = {
NULL, get_ctr_el0 },
};
-static int reg_from_user(void *val, const void __user *uaddr, u64 id)
+static int reg_from_user(u64 *val, const void __user *uaddr, u64 id)
{
- /* This Just Works because we are little endian. */
- if (copy_from_user(val, uaddr, KVM_REG_SIZE(id)) != 0)
+ unsigned long regsize = KVM_REG_SIZE(id);
+ union {
+ u32 word;
+ u64 dword;
+ } tmp = {0};
+
+ if (copy_from_user(&tmp, uaddr, regsize) != 0)
return -EFAULT;
+ switch (regsize) {
+ case 4:
+ *val = tmp.word;
+ break;
+ case 8:
+ *val = tmp.dword;
+ break;
+ }
return 0;
}
-static int reg_to_user(void __user *uaddr, const void *val, u64 id)
+static int reg_to_user(void __user *uaddr, const u64 *val, u64 id)
{
- /* This Just Works because we are little endian. */
- if (copy_to_user(uaddr, val, KVM_REG_SIZE(id)) != 0)
+ unsigned long regsize = KVM_REG_SIZE(id);
+ union {
+ u32 word;
+ u64 dword;
+ } tmp;
+
+ switch (regsize) {
+ case 4:
+ tmp.word = *val;
+ break;
+ case 8:
+ tmp.dword = *val;
+ break;
+ }
+
+ if (copy_to_user(uaddr, &tmp, regsize) != 0)
return -EFAULT;
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
2014-02-12 5:57 ` [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code Victor Kamensky
2014-02-12 5:57 ` [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case Victor Kamensky
@ 2014-02-12 5:57 ` Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case Victor Kamensky
2014-02-12 5:57 ` [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case Victor Kamensky
4 siblings, 1 reply; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
esr_el2 field of struct kvm_vcpu_fault_info has u32 type.
It should be stored as word. Current code works in LE case
because existing puts least significant word of x1 into
esr_el2, and it puts most significant work of x1 into next
field, which accidentally is OK because it is updated again
by next instruction. But existing code breaks in BE case.
Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
arch/arm64/kvm/hyp.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 3b47c36..104216c 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -801,7 +801,7 @@ el1_trap:
mrs x2, far_el2
2: mrs x0, tpidr_el2
- str x1, [x0, #VCPU_ESR_EL2]
+ str w1, [x0, #VCPU_ESR_EL2]
str x2, [x0, #VCPU_FAR_EL2]
str x3, [x0, #VCPU_HPFAR_EL2]
--
1.8.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
` (2 preceding siblings ...)
2014-02-12 5:57 ` [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word Victor Kamensky
@ 2014-02-12 5:57 ` Victor Kamensky
2014-02-12 7:15 ` Alexander Graf
2014-03-20 3:42 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case Victor Kamensky
4 siblings, 2 replies; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
On arm64 'u32 vgic_eisr[2];' and 'u32 vgic_elrsr[2]' are accessed as
one 'unsigned long *' bit fields, which has 64bit size. So we need to
swap least significant word with most significant word when code reads
those registers from h/w.
Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
arch/arm64/kvm/hyp.S | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 104216c..667293f 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -415,10 +415,17 @@ CPU_BE( rev w11, w11 )
str w4, [x3, #VGIC_CPU_HCR]
str w5, [x3, #VGIC_CPU_VMCR]
str w6, [x3, #VGIC_CPU_MISR]
+#ifndef CONFIG_CPU_BIG_ENDIAN
str w7, [x3, #VGIC_CPU_EISR]
str w8, [x3, #(VGIC_CPU_EISR + 4)]
str w9, [x3, #VGIC_CPU_ELRSR]
str w10, [x3, #(VGIC_CPU_ELRSR + 4)]
+#else
+ str w7, [x3, #(VGIC_CPU_EISR + 4)]
+ str w8, [x3, #VGIC_CPU_EISR]
+ str w9, [x3, #(VGIC_CPU_ELRSR + 4)]
+ str w10, [x3, #VGIC_CPU_ELRSR]
+#endif
str w11, [x3, #VGIC_CPU_APR]
/* Clear GICH_HCR */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
` (3 preceding siblings ...)
2014-02-12 5:57 ` [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case Victor Kamensky
@ 2014-02-12 5:57 ` Victor Kamensky
2014-03-20 3:43 ` Christoffer Dall
4 siblings, 1 reply; 12+ messages in thread
From: Victor Kamensky @ 2014-02-12 5:57 UTC (permalink / raw)
To: linux-arm-kernel
Fix vgic_bitmap_get_reg function to return 'right' word address of
'unsigned long' bitmap value in case of BE 64bit image.
Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
virt/kvm/arm/vgic.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 7e11458..e56c5f8 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -96,14 +96,37 @@ static u32 vgic_nr_lr;
static unsigned int vgic_maint_irq;
+/*
+ * struct vgic_bitmap is union that provides two view of
+ * the same data. In one case it is array of registers of
+ * u32 type (.reg). And in another it is bitmap, which is
+ * array of 'unsgined long' (.reg_ul). It works all well in
+ * case of 32bit (u32 and 'unsigned long' have the same size).
+ * It works ok in 64bit LE case, where 'unsigned long'
+ * size is 8 bytes, while u32 is 4 bytes, and least siginificant
+ * word of 'unsigned long' matches lower index of .reg array.
+ * It breaks in 64bit BE case. In this case word sized
+ * register of even index actually resides in least significant
+ * word of 'unsigned long' which has address at offset plus 4
+ * bytes. And word sized register of odd index resides at most
+ * significant of 'unsigned long' which has offset minus 4
+ * bytes. Define REG_OFFSET_SWIZZLE that would help to
+ * change offset of register in case of BE 64bit system.
+ */
+#if defined(CONFIG_CPU_BIG_ENDIAN) && BITS_PER_LONG == 64
+#define REG_OFFSET_SWIZZLE 1
+#else
+#define REG_OFFSET_SWIZZLE 0
+#endif
+
static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
int cpuid, u32 offset)
{
offset >>= 2;
if (!offset)
- return x->percpu[cpuid].reg;
+ return x->percpu[cpuid].reg + (offset^REG_OFFSET_SWIZZLE);
else
- return x->shared.reg + offset - 1;
+ return x->shared.reg + ((offset - 1)^REG_OFFSET_SWIZZLE);
}
static int vgic_bitmap_get_irq_val(struct vgic_bitmap *x,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case
2014-02-12 5:57 ` [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case Victor Kamensky
@ 2014-02-12 7:15 ` Alexander Graf
2014-03-20 3:42 ` Christoffer Dall
1 sibling, 0 replies; 12+ messages in thread
From: Alexander Graf @ 2014-02-12 7:15 UTC (permalink / raw)
To: linux-arm-kernel
On 12.02.2014, at 06:57, Victor Kamensky <victor.kamensky@linaro.org> wrote:
> On arm64 'u32 vgic_eisr[2];' and 'u32 vgic_elrsr[2]' are accessed as
> one 'unsigned long *' bit fields, which has 64bit size. So we need to
> swap least significant word with most significant word when code reads
> those registers from h/w.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> arch/arm64/kvm/hyp.S | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 104216c..667293f 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -415,10 +415,17 @@ CPU_BE( rev w11, w11 )
> str w4, [x3, #VGIC_CPU_HCR]
> str w5, [x3, #VGIC_CPU_VMCR]
> str w6, [x3, #VGIC_CPU_MISR]
> +#ifndef CONFIG_CPU_BIG_ENDIAN
> str w7, [x3, #VGIC_CPU_EISR]
> str w8, [x3, #(VGIC_CPU_EISR + 4)]
> str w9, [x3, #VGIC_CPU_ELRSR]
> str w10, [x3, #(VGIC_CPU_ELRSR + 4)]
> +#else
> + str w7, [x3, #(VGIC_CPU_EISR + 4)]
> + str w8, [x3, #VGIC_CPU_EISR]
> + str w9, [x3, #(VGIC_CPU_ELRSR + 4)]
> + str w10, [x3, #VGIC_CPU_ELRSR]
> +#endif
How about you introduce an asm macro like STRW_64(w7, w8, x3, VGIC_CPU_EISR) which could expand to the two respectively swapped instructions?
Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code
2014-02-12 5:57 ` [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code Victor Kamensky
@ 2014-03-20 3:41 ` Christoffer Dall
0 siblings, 0 replies; 12+ messages in thread
From: Christoffer Dall @ 2014-03-20 3:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 09:57:19PM -0800, Victor Kamensky wrote:
> In case of guest CPU running in LE mode and host runs in
> BE mode we need byteswap data, so read/write is emulated correctly.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index dd8ecfc..fdc3e21 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -213,6 +213,17 @@ static inline unsigned long vcpu_data_guest_to_host(struct kvm_vcpu *vcpu,
> default:
> return be64_to_cpu(data);
> }
> + } else {
> + switch (len) {
> + case 1:
> + return data & 0xff;
> + case 2:
> + return le16_to_cpu(data & 0xffff);
> + case 4:
> + return le32_to_cpu(data & 0xffffffff);
> + default:
> + return le64_to_cpu(data);
> + }
> }
>
> return data; /* Leave LE untouched */
> @@ -233,6 +244,17 @@ static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> default:
> return cpu_to_be64(data);
> }
> + } else {
> + switch (len) {
> + case 1:
> + return data & 0xff;
> + case 2:
> + return cpu_to_le16(data & 0xffff);
> + case 4:
> + return cpu_to_le32(data & 0xffffffff);
> + default:
> + return cpu_to_le64(data);
> + }
> }
>
> return data; /* Leave LE untouched */
> --
> 1.8.1.4
>
Again, given the ABI merge:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case
2014-02-12 5:57 ` [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case Victor Kamensky
@ 2014-03-20 3:41 ` Christoffer Dall
0 siblings, 0 replies; 12+ messages in thread
From: Christoffer Dall @ 2014-03-20 3:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 09:57:20PM -0800, Victor Kamensky wrote:
> This patch fixes issue of reading and writing V8 sys registers in
> BE case. It is similar to V7 "ARM: kvm one_reg coproc set and get
> BE fixes" patch.
>
> It changes reg_from_user and reg_to_user functions to have strong
> typed 'u64 *val' argument. And it uses endian angnostic way to
> pick up righ word from '*val' in case when register size is 4 bytes.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> arch/arm64/kvm/sys_regs.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 02e9d09..e7c3e24 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -701,18 +701,45 @@ static struct sys_reg_desc invariant_sys_regs[] = {
> NULL, get_ctr_el0 },
> };
>
> -static int reg_from_user(void *val, const void __user *uaddr, u64 id)
> +static int reg_from_user(u64 *val, const void __user *uaddr, u64 id)
> {
> - /* This Just Works because we are little endian. */
> - if (copy_from_user(val, uaddr, KVM_REG_SIZE(id)) != 0)
> + unsigned long regsize = KVM_REG_SIZE(id);
> + union {
> + u32 word;
> + u64 dword;
> + } tmp = {0};
> +
> + if (copy_from_user(&tmp, uaddr, regsize) != 0)
> return -EFAULT;
> + switch (regsize) {
> + case 4:
> + *val = tmp.word;
> + break;
This should never happen for arm64, right? IIRC, we expose all system
registers, even the aarch32 ones, as 64-bit versions with padded zeros,
just like in the ARM ARM...
> + case 8:
> + *val = tmp.dword;
> + break;
> + }
> return 0;
> }
>
> -static int reg_to_user(void __user *uaddr, const void *val, u64 id)
> +static int reg_to_user(void __user *uaddr, const u64 *val, u64 id)
> {
> - /* This Just Works because we are little endian. */
> - if (copy_to_user(uaddr, val, KVM_REG_SIZE(id)) != 0)
> + unsigned long regsize = KVM_REG_SIZE(id);
> + union {
> + u32 word;
> + u64 dword;
> + } tmp;
> +
> + switch (regsize) {
> + case 4:
> + tmp.word = *val;
> + break;
same
> + case 8:
> + tmp.dword = *val;
> + break;
> + }
> +
> + if (copy_to_user(uaddr, &tmp, regsize) != 0)
> return -EFAULT;
> return 0;
> }
> --
> 1.8.1.4
>
--
Christoffer
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word
2014-02-12 5:57 ` [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word Victor Kamensky
@ 2014-03-20 3:41 ` Christoffer Dall
0 siblings, 0 replies; 12+ messages in thread
From: Christoffer Dall @ 2014-03-20 3:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 09:57:21PM -0800, Victor Kamensky wrote:
> esr_el2 field of struct kvm_vcpu_fault_info has u32 type.
> It should be stored as word. Current code works in LE case
> because existing puts least significant word of x1 into
> esr_el2, and it puts most significant work of x1 into next
> field, which accidentally is OK because it is updated again
> by next instruction. But existing code breaks in BE case.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> arch/arm64/kvm/hyp.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 3b47c36..104216c 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -801,7 +801,7 @@ el1_trap:
> mrs x2, far_el2
>
> 2: mrs x0, tpidr_el2
> - str x1, [x0, #VCPU_ESR_EL2]
> + str w1, [x0, #VCPU_ESR_EL2]
> str x2, [x0, #VCPU_FAR_EL2]
> str x3, [x0, #VCPU_HPFAR_EL2]
>
> --
> 1.8.1.4
>
Ouch,
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case
2014-02-12 5:57 ` [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case Victor Kamensky
2014-02-12 7:15 ` Alexander Graf
@ 2014-03-20 3:42 ` Christoffer Dall
1 sibling, 0 replies; 12+ messages in thread
From: Christoffer Dall @ 2014-03-20 3:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 09:57:22PM -0800, Victor Kamensky wrote:
> On arm64 'u32 vgic_eisr[2];' and 'u32 vgic_elrsr[2]' are accessed as
> one 'unsigned long *' bit fields, which has 64bit size. So we need to
> swap least significant word with most significant word when code reads
> those registers from h/w.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> arch/arm64/kvm/hyp.S | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 104216c..667293f 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -415,10 +415,17 @@ CPU_BE( rev w11, w11 )
> str w4, [x3, #VGIC_CPU_HCR]
> str w5, [x3, #VGIC_CPU_VMCR]
> str w6, [x3, #VGIC_CPU_MISR]
> +#ifndef CONFIG_CPU_BIG_ENDIAN
> str w7, [x3, #VGIC_CPU_EISR]
> str w8, [x3, #(VGIC_CPU_EISR + 4)]
> str w9, [x3, #VGIC_CPU_ELRSR]
> str w10, [x3, #(VGIC_CPU_ELRSR + 4)]
> +#else
> + str w7, [x3, #(VGIC_CPU_EISR + 4)]
> + str w8, [x3, #VGIC_CPU_EISR]
> + str w9, [x3, #(VGIC_CPU_ELRSR + 4)]
> + str w10, [x3, #VGIC_CPU_ELRSR]
> +#endif
> str w11, [x3, #VGIC_CPU_APR]
>
> /* Clear GICH_HCR */
> --
> 1.8.1.4
>
Isn't it the vgic emulation code that's incorrect then? The GICv2
hardware defines two registers, GICH_ELRSR0 and GICH_ELRSR1 (and
GICH_EISR0 and GICH_EISR1) and I would find it most logic that
vgic_cpu->elrsr[0] == GICH_ELRSR0, always.
Marc?
-Christoffer
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case
2014-02-12 5:57 ` [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case Victor Kamensky
@ 2014-03-20 3:43 ` Christoffer Dall
0 siblings, 0 replies; 12+ messages in thread
From: Christoffer Dall @ 2014-03-20 3:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 09:57:23PM -0800, Victor Kamensky wrote:
> Fix vgic_bitmap_get_reg function to return 'right' word address of
> 'unsigned long' bitmap value in case of BE 64bit image.
>
> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
> ---
> virt/kvm/arm/vgic.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 7e11458..e56c5f8 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -96,14 +96,37 @@ static u32 vgic_nr_lr;
>
> static unsigned int vgic_maint_irq;
>
> +/*
> + * struct vgic_bitmap is union that provides two view of
contains untions that provide two views
> + * the same data. In one case it is array of registers of
an array
> + * u32 type (.reg). And in another it is bitmap, which is
, and in the other case it is a bitmap, which is an array...
> + * array of 'unsgined long' (.reg_ul). It works all well in
It all works well on 32-bit
> + * case of 32bit (u32 and 'unsigned long' have the same size).
> + * It works ok in 64bit LE case, where 'unsigned long'
It also works well on 64-bit LE, but breaks on 64-bit BE.
> + * size is 8 bytes, while u32 is 4 bytes, and least siginificant
> + * word of 'unsigned long' matches lower index of .reg array.
Drop these two lines.
> + * It breaks in 64bit BE case. In this case word sized
> + * register of even index actually resides in least significant
> + * word of 'unsigned long' which has address at offset plus 4
> + * bytes. And word sized register of odd index resides at most
> + * significant of 'unsigned long' which has offset minus 4
> + * bytes. Define REG_OFFSET_SWIZZLE that would help to
> + * change offset of register in case of BE 64bit system.
> + */
> +#if defined(CONFIG_CPU_BIG_ENDIAN) && BITS_PER_LONG == 64
> +#define REG_OFFSET_SWIZZLE 1
> +#else
> +#define REG_OFFSET_SWIZZLE 0
> +#endif
> +
Wondering if it's worth the trouble in this case of having the union;
the union is there only for this function to be simpler, but if it
doesn't work for BE, then maybe it's not worth it?
> static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
> int cpuid, u32 offset)
> {
> offset >>= 2;
> if (!offset)
> - return x->percpu[cpuid].reg;
> + return x->percpu[cpuid].reg + (offset^REG_OFFSET_SWIZZLE);
you need spaces around the '^' according to CodingStyle.
> else
> - return x->shared.reg + offset - 1;
> + return x->shared.reg + ((offset - 1)^REG_OFFSET_SWIZZLE);
ditto
> }
>
> static int vgic_bitmap_get_irq_val(struct vgic_bitmap *x,
> --
> 1.8.1.4
>
Functionally, this looks correct to me.
-Christoffer
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-03-20 3:43 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-12 5:57 [PATCH 0/5] aarch64 BE kvm support Victor Kamensky
2014-02-12 5:57 ` [PATCH 1/5] ARM64: KVM: MMIO support BE host running LE code Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 2/5] ARM64: KVM: set and get of sys registers in BE case Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 3/5] ARM64: KVM: store kvm_vcpu_fault_info est_el2 as word Victor Kamensky
2014-03-20 3:41 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 4/5] ARM64: KVM: vgic_elrsr and vgic_eisr need to be byteswapped in BE case Victor Kamensky
2014-02-12 7:15 ` Alexander Graf
2014-03-20 3:42 ` Christoffer Dall
2014-02-12 5:57 ` [PATCH 5/5] ARM64: KVM: fix vgic_bitmap_get_reg function for BE 64bit case Victor Kamensky
2014-03-20 3:43 ` Christoffer Dall
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.