Re: [PATCHv2] kvmppc: Implement H_LOGICAL_CI_{LOAD,STORE} in KVM

David Gibson
2015-02-05T00:53:00Z

[-- Attachment #1: Type: text/plain, Size: 4851 bytes --]

On Wed, Feb 04, 2015 at 04:24:46PM +0100, Alexander Graf wrote:
>
>
> On 03.02.15 06:44, David Gibson wrote:
> > On POWER, storage caching is usually configured via the MMU - attributes
> > such as cache-inhibited are stored in the TLB and the hashed page table.
> >
> > This makes correctly performing cache inhibited IO accesses awkward when
> > the MMU is turned off (real mode). Some CPU models provide special
> > registers to control the cache attributes of real mode load and stores but
> > this is not at all consistent. This is a problem in particular for SLOF,
> > the firmware used on KVM guests, which runs entirely in real mode, but
> > which needs to do IO to load the kernel.
> >
> > To simplify this qemu implements two special hypercalls, H_LOGICAL_CI_LOAD
> > and H_LOGICAL_CI_STORE which simulate a cache-inhibited load or store to
> > a logical address (aka guest physical address). SLOF uses these for IO.
> >
> > However, because these are implemented within qemu, not the host kernel,
> > these bypass any IO devices emulated within KVM itself. The simplest way
> > to see this problem is to attempt to boot a KVM guest from a virtio-blk
> > device with iothread / dataplane enabled. The iothread code relies on an
> > in kernel implementation of the virtio queue notification, which is not
> > triggered by the IO hcalls, and so the guest will stall in SLOF unable to
> > load the guest OS.
> >
> > This patch addresses this by providing in-kernel implementations of the
> > 2 hypercalls, which correctly scan the KVM IO bus. Any access to an
> > address not handled by the KVM IO bus will cause a VM exit, hitting the
> > qemu implementation as before.
> >
> > Note that a userspace change is also required, in order to enable these
> > new hcall implementations with KVM_CAP_PPC_ENABLE_HCALL.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > arch/powerpc/include/asm/kvm_book3s.h | 3 ++
> > arch/powerpc/kvm/book3s.c | 76 +++++++++++++++++++++++++++++++++++
> > arch/powerpc/kvm/book3s_hv.c | 12 ++++++
> > arch/powerpc/kvm/book3s_pr_papr.c | 28 +++++++++++++
> > 4 files changed, 119 insertions(+)
> >
> > v2:
> > - Removed some debugging printk()s that were accidentally left in
> > - Fix endianness; like all PAPR hypercalls, these should always act
> > big-endian, even if the guest is little-endian (in practice this
> > makes no difference, since the only user is SLOF, which is always
> > big-endian)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> > index 942c7b1..578e550 100644
> > --- a/arch/powerpc/include/asm/kvm_book3s.h
> > +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > @@ -292,6 +292,9 @@ static inline bool kvmppc_supports_magic_page(struct kvm_vcpu *vcpu)
> > return !is_kvmppc_hv_enabled(vcpu->kvm);
> > }
> >
> > +extern int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu);
> > +extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
> > +
> > /* Magic register values loaded into r3 and r4 before the 'sc' assembly
> > * instruction for the OSI hypercalls */
> > #define OSI_SC_MAGIC_R3 0x113724FA
> > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> > index 888bf46..7b51492 100644
> > --- a/arch/powerpc/kvm/book3s.c
> > +++ b/arch/powerpc/kvm/book3s.c
> > @@ -820,6 +820,82 @@ void kvmppc_core_destroy_vm(struct kvm *kvm)
> > #endif
> > }
> >
> > +int kvmppc_h_logical_ci_load(struct kvm_vcpu *vcpu)
> > +{
> > + unsigned long size = kvmppc_get_gpr(vcpu, 4);
> > + unsigned long addr = kvmppc_get_gpr(vcpu, 5);
> > + u64 buf;
> > + int ret;
> > +
> > + if (!is_power_of_2(size) || (size > sizeof(buf)))
> > + return H_TOO_HARD;
> > +
> > + ret = kvm_io_bus_read(vcpu->kvm, KVM_MMIO_BUS, addr, size, &buf);
> > + if (ret != 0)
> > + return H_TOO_HARD;
> > +
> > + switch (size) {
> > + case 1:
> > + kvmppc_set_gpr(vcpu, 4, *(u8 *)&buf);
> > + break;
> > +
> > + case 2:
> > + kvmppc_set_gpr(vcpu, 4, be16_to_cpu(*(u16 *)&buf));
> > + break;
> > +
> > + case 4:
> > + kvmppc_set_gpr(vcpu, 4, be32_to_cpu(*(u32 *)&buf));
> > + break;
> > +
> > + case 8:
> > + kvmppc_set_gpr(vcpu, 4, be64_to_cpu(*(u64 *)&buf));
>
> Shouldn't these casts be __be types?

Ah, yes they should.

> > + break;
> > +
> > + default:
> > + BUG();
> > + }
> > +
> > + return H_SUCCESS;
> > +}
> > +EXPORT_SYMBOL_GPL(kvmppc_h_logical_ci_load); /* For use by the kvm-pr module */
>
> No need for the comment.

Ok.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]