qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Alexander Graf <agraf@suse.de>
Cc: "aik@ozlabs.ru" <aik@ozlabs.ru>,
	"qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>,
	Paul Mackerras <paulus@samba.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"mdroth@us.ibm.com" <mdroth@us.ibm.com>
Subject: Re: [Qemu-devel] [RFC] pseries: Enable in-kernel H_LOGICAL_CI_{LOAD, STORE} implementations
Date: Thu, 5 Feb 2015 22:30:07 +1100	[thread overview]
Message-ID: <20150205113007.GT25675@voom.fritz.box> (raw)
In-Reply-To: <6EFB0F0E-BB1D-4DAE-8BA4-367B05E88553@suse.de>

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

On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
> 
> 
> 
> > Am 05.02.2015 um 03:55 schrieb David Gibson <david@gibson.dropbear.id.au>:
> > 
> >> 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 <david@gibson.dropbear.id.au>
> >>>>>> 
> >>>>>> [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.
> 
> But in that case the VM stays functional and will merely see a
> performance hit when using virtio in SLOF, no? I don't think that's
> a problem worth worrying users about.

Alas, no.  The iothread stuff *relies* on the in-kernel notification,
so it will not work if the IO gets punted to qemu.  This is the whole
reason for the in-kernel hcall implementation.

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

  reply	other threads:[~2015-02-05 11:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-03  6:10 [Qemu-devel] [RFC] pseries: Enable in-kernel H_LOGICAL_CI_{LOAD, STORE} implementations David Gibson
2015-02-03  8:56 ` [Qemu-devel] [Qemu-ppc] " Nikunj A Dadhania
2015-02-03 21:19 ` [Qemu-devel] " Paul Mackerras
2015-02-04  1:32   ` David Gibson
2015-02-04 15:19     ` Alexander Graf
2015-02-05  0:48       ` David Gibson
2015-02-05  0:54         ` Alexander Graf
2015-02-05  2:55           ` David Gibson
2015-02-05 10:22             ` Alexander Graf
2015-02-05 11:30               ` David Gibson [this message]
2015-02-05 11:55                 ` Alexander Graf
2015-02-06  2:54                   ` David Gibson
2015-02-06  7:56                     ` Alexander Graf
2015-02-09  0:37                       ` David Gibson
2015-02-09  1:41                         ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150205113007.GT25675@voom.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=mdroth@us.ibm.com \
    --cc=paulus@samba.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).