Re: [Qemu-devel] [PATCH] target-ppc: ppc64 target's virtio can be either endian.

[Alexander Graf]

On 25.11.2013, at 16:35, Greg Kurz <address@hidden> wrote:

> We base it on the OS endian, as reflected by the endianness of the
> interrupt vectors (handled through the ILE bit in the LPCR register).
> 
> This patch does two things:
> - make LPCR a KVM register
> - implement virtio_get_byteswap() over LPCR
> 
> This patch requires to have the following defined in the linux headers:
> 
> $ grep LPCR linux-headers/asm-powerpc/kvm.h
> #define KVM_REG_PPC_LPCR        (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0xb5)
> 
> Suggested-by: Benjamin Herrenschmidt <address@hidden>
> Signed-off-by: Rusty Russell <address@hidden>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> target-ppc/kvm.c         |    4 ++++
> target-ppc/misc_helper.c |   14 ++++++++++++++
> 2 files changed, 18 insertions(+)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 10d0cd9..b450a22 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -869,6 +869,8 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>                 DPRINTF("Warning: Unable to set VPA information to KVM\n");
>             }
>         }
> +
> +        kvm_put_one_spr(cs, KVM_REG_PPC_LPCR, SPR_LPCR);
> #endif /* TARGET_PPC64 */
>     }
> 
> @@ -1091,6 +1093,8 @@ int kvm_arch_get_registers(CPUState *cs)
>                 DPRINTF("Warning: Unable to get VPA information from KVM\n");
>             }
>         }
> +
> +        kvm_get_one_spr(cs, KVM_REG_PPC_LPCR, SPR_LPCR);
> #endif
>     }
> 
> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
> index 616aab6..0e0743a 100644
> --- a/target-ppc/misc_helper.c
> +++ b/target-ppc/misc_helper.c
> @@ -20,6 +20,8 @@
> #include "helper.h"
> 
> #include "helper_regs.h"
> +#include "hw/virtio/virtio.h"
> +#include "sysemu/kvm.h"
> 
> /*****************************************************************************/
> /* SPR accesses */
> @@ -116,3 +118,15 @@ void ppc_store_msr(CPUPPCState *env, target_ulong value)
> {
>     hreg_store_msr(env, value, 0);
> }
> +
> +bool virtio_get_byteswap(void)
> +{
> +    PowerPCCPU *cp = POWERPC_CPU(first_cpu);
> +    CPUPPCState *env = &cp->env;
> +
> +    if (kvm_enabled()) { 
> +        kvm_arch_get_registers(first_cpu);

This function is not defined when CONFIG_KVM is disabled.

Please do

    cpu_synchronize_state(first_cpu);

instead.

Also can't virtio_get_byteswap pass in the CPU pointer of the CPU that's 
calling in this moment? I'm not sure how racy it is to synchronize the first 
cpu while we're not in the first cpu's execution thread.

Either way, if we do this "right" we don't even have to jump through these 
hoops, as little endian setting simply happens steered from QEMU, so QEMU will 
have all knowledge about the guest's little endian mode without the need to 
synchronize any state.


Alex