On Mon, 9 Dec 2013 18:18:26 +0100
Alexander Graf <ag...@suse.de> wrote:
>
> On 09.12.2013, at 18:11, Greg Kurz <gk...@linux.vnet.ibm.com> wrote:
>
> > On Mon, 9 Dec 2013 16:33:59 +0100
> > Alexander Graf <ag...@suse.de> wrote:
> >>
> >> On 25.11.2013, at 16:35, Greg Kurz <gk...@linux.vnet.ibm.com> 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 <b...@kernel.crashing.org>
> >>> Signed-off-by: Rusty Russell <ru...@rustcorp.com.au>
> >>> Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com>
> >>> ---
> >>> 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.
> >>
> >
> > Oups... I'll fix that.
> >
> >> 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.
> >>
> >
> > I kept the choices made by Rusty in the original serie. According to
> > these, I am not sure if we can use current_cpu:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2013-08/msg01156.html
> > https://lists.gnu.org/archive/html/qemu-devel/2013-08/msg01504.html
> >
> >> 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.
> >>
> >
> > Sure. This is just primary work to get virtio working for cross-endian
> > cases. :)
>
> Yeah, but so far LE switching code is not upstream in KVM - and for TCG
> we don't need the cpu_synchronize_state() call at all.
>
>
> Alex
>
Sure, but for TCG, cpu_synchronize_state() does nothing since KVM is
not enabled... what would be wrong with that ?
Cheers.
--
Greg
