All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.