[Qemu-devel] [PATCH V2 0/2] Add virtio endianness determination in KVM ARM/ARM64

[Qemu-devel] [PATCH V2 0/2] Add virtio endianness determination in KVM ARM/ARM64

[Pranavkumar Sawargaonkar]

In KVM ARM/ARM64 one can run cross-endian guset with respect to host kernel.
For virtio to work with this we need to make virtio code aware of 
guest's endianness.
This patchset adds the guest endianness determination for KVM ARM/ARM64
and related small fix in KVM ARM64.

V2:
- Change the patch to be used generically for both ARM and ARM64.
- Define TARGET_IS_BIENDIAN for target-arm
- Address the review comments received in v1.
- Rebased code on top of latest qemu code.

V1:
- First patch in this series.
- http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03321.html


Pranavkumar Sawargaonkar (2):
  target-arm: KVM64: Get and Sync up guest register state like kvm32.
  target-arm: Guest cpu endianness determination for virtio KVM
    ARM/ARM64

 target-arm/cpu.c   | 27 +++++++++++++++++++++++++++
 target-arm/cpu.h   |  2 ++
 target-arm/kvm64.c | 12 ++++++++++++
 3 files changed, 41 insertions(+)

-- 
1.9.1

[Qemu-devel] [PATCH V2 1/2] target-arm: KVM64: Get and Sync up guest register state like kvm32.

[Pranavkumar Sawargaonkar]

This patch adds:
1. Call write_kvmstate_to_list() and write_list_to_cpustate()
   in kvm_arch_get_registers() to sync guest register state.
2. Call write_list_to_kvmstate() in kvm_arch_put_registers()
   to sync guest register state.

These changes are already there for kvm32 in target-arm/kvm32.c.

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 target-arm/kvm64.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index ba16821..96148c6 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -193,6 +193,10 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         }
     }
 
+    if (!write_list_to_kvmstate(cpu)) {
+        return EINVAL;
+    }
+
     /* TODO:
      * FP state
      * system registers
@@ -269,6 +273,14 @@ int kvm_arch_get_registers(CPUState *cs)
         }
     }
 
+    if (!write_kvmstate_to_list(cpu)) {
+        return EINVAL;
+    }
+    /* Note that it's OK to have registers which aren't in CPUState,
+     * so we can ignore a failure return here.
+     */
+    write_list_to_cpustate(cpu);
+
     /* TODO: other registers */
     return ret;
 }
-- 
1.9.1

Re: [Qemu-devel] [PATCH V2 1/2] target-arm: KVM64: Get and Sync up guest register state like kvm32.

[Peter Maydell]

On 22 December 2014 at 09:04, Pranavkumar Sawargaonkar
<pranavkumar@linaro.org> wrote:
> This patch adds:
> 1. Call write_kvmstate_to_list() and write_list_to_cpustate()
>    in kvm_arch_get_registers() to sync guest register state.
> 2. Call write_list_to_kvmstate() in kvm_arch_put_registers()
>    to sync guest register state.
>
> These changes are already there for kvm32 in target-arm/kvm32.c.
>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  target-arm/kvm64.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index ba16821..96148c6 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -193,6 +193,10 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          }
>      }
>
> +    if (!write_list_to_kvmstate(cpu)) {
> +        return EINVAL;
> +    }
> +
>      /* TODO:
>       * FP state
>       * system registers

You should delete the "system registers" line from this TODO
comment, since you've now fixed that part.

> @@ -269,6 +273,14 @@ int kvm_arch_get_registers(CPUState *cs)
>          }
>      }
>
> +    if (!write_kvmstate_to_list(cpu)) {
> +        return EINVAL;
> +    }
> +    /* Note that it's OK to have registers which aren't in CPUState,
> +     * so we can ignore a failure return here.
> +     */
> +    write_list_to_cpustate(cpu);
> +
>      /* TODO: other registers */
>      return ret;
>  }

Otherwise:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH V2 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64

[Pranavkumar Sawargaonkar]

This patch implements a fucntion pointer virtio_is_big_endian()
from "CPUClass" structure for arm/arm64.
Function arm_cpu_virtio_endianness() 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.

Changelog:

V2:
- Change the patch to be used generically for both ARM and ARM64.
- Address the review comments received in v1.

V1:
- First patch in this series.
- http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03321.html

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 target-arm/cpu.c | 27 +++++++++++++++++++++++++++
 target-arm/cpu.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d3db279..7145e02 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -320,6 +320,32 @@ 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_virtio_endianness(CPUState *cs)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    CPUARMState *env = &cpu->env;
+    uint64_t sctlr;
+
+    cpu_synchronize_state(cs);
+
+    /* Check if we are running 32bit guest or not */
+    if (!is_a64(env)) {
+        return (env->pstate & CPSR_E) ? 1 : 0;
+    }
+
+    sctlr = env->cp15.sctlr_el[1];
+
+    if ((env->pstate & 0xf) == PSTATE_MODE_EL0t) {
+        sctlr &= SCTLR_E0E;
+    } else {
+        sctlr &= SCTLR_EE;
+    }
+
+    /* If BIG-ENDIAN return 1 */
+    return sctlr ? 1 : 0;
+}
+
 #endif
 
 static inline void set_feature(CPUARMState *env, int feature)
@@ -1157,6 +1183,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_virtio_endianness;
 #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 7ba55f0..f32a5b6 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

Re: [Qemu-devel] [PATCH V2 2/2] target-arm: Guest cpu endianness determination for virtio KVM ARM/ARM64

[Peter Maydell]

On 22 December 2014 at 09:04, Pranavkumar Sawargaonkar
<pranavkumar@linaro.org> wrote:
> This patch implements a fucntion pointer virtio_is_big_endian()
> from "CPUClass" structure for arm/arm64.
> Function arm_cpu_virtio_endianness() 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.
>
> Changelog:

The changelog should go below the "---" line, ideally.

> V2:
> - Change the patch to be used generically for both ARM and ARM64.
> - Address the review comments received in v1.
>
> V1:
> - First patch in this series.
> - http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03321.html
>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  target-arm/cpu.c | 27 +++++++++++++++++++++++++++
>  target-arm/cpu.h |  2 ++
>  2 files changed, 29 insertions(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d3db279..7145e02 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -320,6 +320,32 @@ 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_virtio_endianness(CPUState *cs)

You should call this arm_cpu_is_big_endian(), which lines
up with the PPC function name and is closer to the QOM
method name we're implementing.

> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    uint64_t sctlr;
> +
> +    cpu_synchronize_state(cs);
> +
> +    /* Check if we are running 32bit guest or not */

I don't think this comment is particularly informative.

> +    if (!is_a64(env)) {
> +        return (env->pstate & CPSR_E) ? 1 : 0;
> +    }
> +
> +    sctlr = env->cp15.sctlr_el[1];
> +
> +    if ((env->pstate & 0xf) == PSTATE_MODE_EL0t) {
> +        sctlr &= SCTLR_E0E;
> +    } else {
> +        sctlr &= SCTLR_EE;
> +    }

We might as well make this robust for when we implement EL2 and EL3:
    int cur_el;
    ...

    cur_el = arm_current_el(env);
    if (cur_el == 0) {
        return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
    } else {
        return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
    }

> +
> +    /* If BIG-ENDIAN return 1 */

...and with the new function name this comment isn't necessary
either, because it's obvious.

> +    return sctlr ? 1 : 0;
> +}
> +
>  #endif
>
>  static inline void set_feature(CPUARMState *env, int feature)
> @@ -1157,6 +1183,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_virtio_endianness;
>  #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 7ba55f0..f32a5b6 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

thanks
-- PMM