Re: [Qemu-devel] [RFC] pseries: Enable in-kernel H_LOGICAL_CI_{LOAD, STORE} implementations

[David Gibson]

On Thu, Feb 05, 2015 at 01:54:39AM +0100, Alexander Graf wrote:
> 
> 
> On 05.02.15 01:48, David Gibson wrote:
> > On Wed, Feb 04, 2015 at 04:19:14PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 04.02.15 02:32, David Gibson wrote:
> >>> On Wed, Feb 04, 2015 at 08:19:06AM +1100, Paul Mackerras wrote:
> >>>> On Tue, Feb 03, 2015 at 05:10:51PM +1100, David Gibson wrote:
> >>>>> qemu currently implements the hypercalls H_LOGICAL_CI_LOAD and
> >>>>> H_LOGICAL_CI_STORE as PAPR extensions.  These are used by the SLOF 
> >>>>> firmware
> >>>>> for IO, because performing cache inhibited MMIO accesses with the MMU 
> >>>>> off
> >>>>> (real mode) is very awkward on POWER.
> >>>>>
> >>>>> This approach breaks when SLOF needs to access IO devices implemented
> >>>>> within KVM instead of in qemu.  The simplest example would be virtio-blk
> >>>>> using an iothread, because the iothread / dataplane mechanism relies on
> >>>>> an in-kernel implementation of the virtio queue notification MMIO.
> >>>>>
> >>>>> To fix this, an in-kernel implementation of these hypercalls has been 
> >>>>> made,
> >>>>> however, the hypercalls still need to be enabled from qemu.  This 
> >>>>> performs
> >>>>> the necessary calls to do so.
> >>>>>
> >>>>> Signed-off-by: David Gibson <address@hidden>
> >>>>
> >>>> [snip]
> >>>>
> >>>>> +    ret1 = kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_LOAD);
> >>>>> +    if (ret1 != 0) {
> >>>>> +        fprintf(stderr, "Warning: error enabling H_LOGICAL_CI_LOAD in 
> >>>>> KVM:"
> >>>>> +                " %s\n", strerror(errno));
> >>>>> +    }
> >>>>> +
> >>>>> +    ret2 = kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
> >>>>> +    if (ret2 != 0) {
> >>>>> +        fprintf(stderr, "Warning: error enabling H_LOGICAL_CI_STORE in 
> >>>>> KVM:"
> >>>>> +                " %s\n", strerror(errno));
> >>>>> +     }
> >>>>> +
> >>>>> +    if ((ret1 != 0) || (ret2 != 0)) {
> >>>>> +        fprintf(stderr, "Warning: Couldn't enable H_LOGICAL_CI_* in 
> >>>>> KVM, SLOF"
> >>>>> +                " may be unable to operate devices with in-kernel 
> >>>>> emulation\n");
> >>>>> +    }
> >>>>
> >>>> You'll always get these warnings if you're running on an old (meaning
> >>>> current upstream) kernel, which could be annoying.
> >>>
> >>> True.
> >>>
> >>>> Is there any way
> >>>> to tell whether you have configured any devices which need the
> >>>> in-kernel MMIO emulation and only warn if you have?
> >>>
> >>> In theory, I guess so.  In practice I can't see how you'd enumerate
> >>> all devices that might require kernel intervention without something
> >>> horribly invasive.
> >>
> >> We could WARN_ONCE in QEMU if we emulate such a hypercall, but its
> >> handler is io_mem_unassigned (or we add another minimum priority huge
> >> memory region on all 64bits of address space that reports the breakage).
> > 
> > Would that work for the virtio+iothread case?  I had the impression
> > the kernel handled notification region was layered over the qemu
> > emulated region in that case.
> 
> IIRC we don't have a way to call back into kvm saying "please write to
> this in-kernel device". But we could at least defer the warning to a
> point where we know that we actually hit it.

Right, but I'm saying we might miss the warning in cases where we want
it, because the KVM device is shadowed by a qemu device, so qemu won't
see the IO as unassigned or unhandled.

In particular, I think that will happen in the case of virtio-blk with
iothread, which is the simplest case in which to observe the problem.
The virtio-blk device exists in qemu and is functional, but we rely on
KVM catching the queue notification MMIO before it reaches the qemu
implementation of the rest of the device's IO space.

-- 
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

pgpC_pK1qc8Op.pgp
Description: PGP signature