Re: [Qemu-devel] [PATCH V3 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64
Hi PMM,
On 5 February 2015 at 17:18, Peter Maydell wrote:
> On 5 February 2015 at 11:43, Peter Maydell wrote:
>> On 5 February 2015 at 09:59, Pranavkumar Sawargaonkar
>> wrote:
>>> +
>>> +/* In 32bit guest endianess is determined by looking at CPSR's E bit */
>>> +if (!is_a64(env)) {
>>> +return (env->pstate & CPSR_E) ? 1 : 0;
>>
>> This is wrong, because if we're not 32-bit then the CPSR
>> isn't in env->pstate but in env->cpsr_uncached. (I'm guessing
>> you didn't test 32-bit guests.)
>
> Actually thinking about it your code would have worked for the
> common 32-bit guest case, since if we fall through to looking
> at SCTLR then (assuming the guest is at EL1 which it will be when
> it's messing with the virtio device) we'll end up checking the
> 32-bit SCTLR EE bit, which will be the same as the current
> guest endianness for any sane guest kernel. So I apologise
> for suggesting you didn't test that case.
Actually I have not not tested patch with 32bit guests.
Thanks for pulling the patches and 32bit guest case fix.
Thanks,
Pranav
>
> -- PMM
Re: [Qemu-devel] [PATCH V3 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64
On 5 February 2015 at 11:43, Peter Maydell wrote:
> On 5 February 2015 at 09:59, Pranavkumar Sawargaonkar
> wrote:
>> +
>> +/* In 32bit guest endianess is determined by looking at CPSR's E bit */
>> +if (!is_a64(env)) {
>> +return (env->pstate & CPSR_E) ? 1 : 0;
>
> This is wrong, because if we're not 32-bit then the CPSR
> isn't in env->pstate but in env->cpsr_uncached. (I'm guessing
> you didn't test 32-bit guests.)
Actually thinking about it your code would have worked for the
common 32-bit guest case, since if we fall through to looking
at SCTLR then (assuming the guest is at EL1 which it will be when
it's messing with the virtio device) we'll end up checking the
32-bit SCTLR EE bit, which will be the same as the current
guest endianness for any sane guest kernel. So I apologise
for suggesting you didn't test that case.
-- PMM
Re: [Qemu-devel] [PATCH V3 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64
On 5 February 2015 at 09:59, Pranavkumar Sawargaonkar
wrote:
> This patch implements a fucntion pointer "virtio_is_big_endian"
> from "CPUClass" structure for arm/arm64.
> Function arm_cpu_is_big_endian() is added to determine and
> return the guest cpu endianness to virtio.
> This is required for running cross endian guests with virtio on ARM/ARM64.
>
> Signed-off-by: Pranavkumar Sawargaonkar
> ---
> target-arm/cpu.c | 24
> target-arm/cpu.h | 2 ++
> 2 files changed, 26 insertions(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 285947f..4d9cded 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -320,6 +320,29 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq,
> int level)
> kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
> #endif
> }
> +
> +static bool arm_cpu_is_big_endian(CPUState *cs)
> +{
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = &cpu->env;
> +int cur_el;
> +
> +cpu_synchronize_state(cs);
> +
> +/* In 32bit guest endianess is determined by looking at CPSR's E bit */
> +if (!is_a64(env)) {
> +return (env->pstate & CPSR_E) ? 1 : 0;
This is wrong, because if we're not 32-bit then the CPSR
isn't in env->pstate but in env->cpsr_uncached. (I'm guessing
you didn't test 32-bit guests.)
Since this is the only error in this patch, I'll just fix it
as I put the series into target-arm.next rather than forcing
you to respin it.
thanks
-- PMM
[Qemu-devel] [PATCH V3 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64
This patch implements a fucntion pointer "virtio_is_big_endian"
from "CPUClass" structure for arm/arm64.
Function arm_cpu_is_big_endian() is added to determine and
return the guest cpu endianness to virtio.
This is required for running cross endian guests with virtio on ARM/ARM64.
Signed-off-by: Pranavkumar Sawargaonkar
---
target-arm/cpu.c | 24
target-arm/cpu.h | 2 ++
2 files changed, 26 insertions(+)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 285947f..4d9cded 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -320,6 +320,29 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, int
level)
kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
#endif
}
+
+static bool arm_cpu_is_big_endian(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = &cpu->env;
+int cur_el;
+
+cpu_synchronize_state(cs);
+
+/* In 32bit guest endianess is determined by looking at CPSR's E bit */
+if (!is_a64(env)) {
+return (env->pstate & CPSR_E) ? 1 : 0;
+}
+
+cur_el = arm_current_el(env);
+
+if (cur_el == 0) {
+return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
+}
+
+return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
+}
+
#endif
static inline void set_feature(CPUARMState *env, int feature)
@@ -1189,6 +1212,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void
*data)
cc->do_interrupt = arm_cpu_do_interrupt;
cc->get_phys_page_debug = arm_cpu_get_phys_page_debug;
cc->vmsd = &vmstate_arm_cpu;
+cc->virtio_is_big_endian = arm_cpu_is_big_endian;
#endif
cc->gdb_num_core_regs = 26;
cc->gdb_core_xml_file = "arm-core.xml";
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cd7a9e8..317e801 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -32,6 +32,8 @@
# define ELF_MACHINE EM_ARM
#endif
+#define TARGET_IS_BIENDIAN 1
+
#define CPUArchState struct CPUARMState
#include "qemu-common.h"
--
1.9.1
