Re: [Qemu-devel] [PATCH] target-arm: Add guest cpu endianness determination for virtio in KVM ARM64

[Greg Kurz]

On Tue, 28 Oct 2014 12:08:01 +0530
Pranavkumar Sawargaonkar <address@hidden> wrote:

> This patch implements a fucntion pointer virtio_is_big_endian()
> from "CPUClass" structure for arm64.
> Function aarch64_cpu_virtio_endianness() is added to determine and
> returns the guest cpu endianness to virtio.
> This is required for running cross endian guests with virtio on ARM64.
> 
> Signed-off-by: Pranavkumar Sawargaonkar <address@hidden>
> ---
>  include/hw/virtio/virtio-access.h |  2 ++
>  target-arm/cpu64.c                | 41 
> +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/include/hw/virtio/virtio-access.h 
> b/include/hw/virtio/virtio-access.h
> index 46456fd..84fa701 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -23,6 +23,8 @@ static inline bool virtio_access_is_big_endian(VirtIODevice 
> *vdev)
>      return virtio_is_big_endian(vdev);
>  #elif defined(TARGET_WORDS_BIGENDIAN)
>      return true;
> +#elif defined(TARGET_AARCH64)
> +    return virtio_is_big_endian(vdev);

This is code duplication of the TARGET_IS_BIENDIAN case. To be consistent
with what was done for ppc64, you should have something like this instead:

--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -27,6 +27,7 @@
   /* AArch64 definitions */
 #  define TARGET_LONG_BITS 64
 #  define ELF_MACHINE EM_AARCH64
+#  define TARGET_IS_BIENDIAN 1
 #else
 #  define TARGET_LONG_BITS 32
 #  define ELF_MACHINE EM_ARM

>  #else
>      return false;
>  #endif
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index c30f47e..789f886 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -192,6 +192,43 @@ static void aarch64_cpu_set_pc(CPUState *cs, vaddr value)
>      }
>  }
> 
> +#ifndef CONFIG_USER_ONLY
> +
> +#define KVM_REG_ARM64_SCTLR_EL1 3, 0, 1, 0, 0
> +
> +static bool aarch64_cpu_virtio_endianness(CPUState *cs)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    CPUARMState *env = &cpu->env;
> +    struct kvm_one_reg reg;
> +    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;
> +
> +    /* Ideally we do not need to call IOCTL again to read SCTLR_EL1 value.
> +     * cpu_synchronize_state() should fill the env->cp15.c1_sys
> +     * to get this value but this path is currently not implemented for 
> arm64.
> +     * Hence this is a temporary fix.
> +     */
> +
> +    reg.id = ARM64_SYS_REG(KVM_REG_ARM64_SCTLR_EL1);
> +    reg.addr = (uint64_t) &sctlr;
> +    kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
> +
> +    if ((env->pstate & 0xf) == PSTATE_MODE_EL0t)
> +        sctlr &= (1U <<24);
> +    else
> +        sctlr &= (1U <<25);
> +
> +    /* If BIG-ENDIAN return 1 */
> +    return sctlr ? 1 : 0;
> +}
> +#endif
> +
>  static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CPUClass *cc = CPU_CLASS(oc);
> @@ -203,6 +240,10 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void 
> *data)
>      cc->gdb_write_register = aarch64_cpu_gdb_write_register;
>      cc->gdb_num_core_regs = 34;
>      cc->gdb_core_xml_file = "aarch64-core.xml";
> +#ifndef CONFIG_USER_ONLY
> +    cc->virtio_is_big_endian = aarch64_cpu_virtio_endianness;
> +#endif
> +
>  }
> 
>  static void aarch64_cpu_register(const ARMCPUInfo *info)