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

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

[David Gibson]

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>
---
 hw/ppc/spapr.c       |  5 +++++
 target-ppc/kvm.c     | 27 +++++++++++++++++++++++++++
 target-ppc/kvm_ppc.h |  5 +++++
 3 files changed, 37 insertions(+)

The kernel support this qemu patch enables has been posted but not
merged as of this post.  See https://lkml.org/lkml/2015/2/3/17

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b560459..40fe1dd 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1459,6 +1459,11 @@ static void ppc_spapr_init(MachineState *machine)
         qemu_register_reset(spapr_cpu_reset, cpu);
     }
 
+    if (kvm_enabled()) {
+        /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
+        kvmppc_enable_logical_ci_hcalls();
+    }
+
     /* allocate RAM */
     spapr->ram_limit = ram_size;
     memory_region_allocate_system_memory(ram, NULL, "ppc_spapr.ram",
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 1edf2b5..c9d04e8 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1882,6 +1882,33 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len)
     return 0;
 }
 
+static inline int kvmppc_enable_hcall(KVMState *s, target_ulong hcall)
+{
+    return kvm_vm_enable_cap(s, KVM_CAP_PPC_ENABLE_HCALL, 0, hcall, 1);
+}
+
+void kvmppc_enable_logical_ci_hcalls(void)
+{
+    int ret1, ret2;
+
+    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");
+    }
+}
+
 void kvmppc_set_papr(PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 2e0224c..4d30e27 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -24,6 +24,7 @@ bool kvmppc_get_host_serial(char **buf);
 int kvmppc_get_hasidle(CPUPPCState *env);
 int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
 int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
+void kvmppc_enable_logical_ci_hcalls(void);
 void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
@@ -107,6 +108,10 @@ static inline int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level)
     return -1;
 }
 
+static inline void kvmppc_enable_logical_ci_hcalls(void)
+{
+}
+
 static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
-- 
2.1.0

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

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

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

Reviewed-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

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

[Paul Mackerras]

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

Paul.

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

[David Gibson]

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

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.

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

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

[Alexander Graf]

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


Alex

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

[David Gibson]

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

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.

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

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

[Alexander Graf]

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.


Alex

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

[David Gibson]

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

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.

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

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

[Alexander Graf]

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

Alex

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

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

[David Gibson]

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

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

[Alexander Graf]

On 05.02.15 12:30, David Gibson wrote:
> 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.

So at least with vhost-net the in-kernel trapping is optional. If we
happen to get MMIO into QEMU, we'll just handle it there.

Enlighten me why the iothread stuff can't handle it that way too.


Alex

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

[David Gibson]

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

On Thu, Feb 05, 2015 at 12:55:45PM +0100, Alexander Graf wrote:
> 
> 
> On 05.02.15 12:30, David Gibson wrote:
> > On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
[snip]
> >>>>>>>> [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.
> 
> So at least with vhost-net the in-kernel trapping is optional. If we
> happen to get MMIO into QEMU, we'll just handle it there.
> 
> Enlighten me why the iothread stuff can't handle it that way too.

So, as I understand it, it could, but it doesn't.  Working out how to
fix it properly requires better understanding of the dataplane code
than I currently possess,

So, using virtio-blk as the example case.  Normally the queue notify
mmio will get routed by the general virtio code to
virtio_blk_handle_output().

In the case of dataplane, that just calls
virtio_blk_data_plane_start().  So the first time we get a vq notify,
the dataplane is started.  That sets up the host notifier
(VirtioBusClass::set_host_notifier -> virtio_pci_set_host_notifier ->
virtio_pci_set_host_notifier_internal -> memory_region_add_eventfd()
-> memory_region_transaction_commit() ->
address_space_update_ioeventfds - >address_space_add_del_ioeventfds ->
kvm_mem_ioeventfd_add -> kvm_set_ioeventfd_mmio -> KVM_IOEVENTFD
ioctl)

From this point on further calls to virtio_blk_handle_output() are
IIUC a "can't happen", because vq notifies should go to the eventfd
instead, where they will kick the iothread.

So, with SLOF, the first request is ok - it hits
virtio_blk_handle_output() which starts the iothread which goes on to
process the request.

On the second request, however, we get back into
virtio_blk_data_plane_start() which sees the iothread is already
running and aborts.  I think it is assuming that this must be the
result of a race with another vcpu starting the dataplane, and so
assumes the racing thread will have woken the dataplane which will
then handle this vcpu's request as well.

In our case, however, the IO hcalls go through to
virtio_blk_handle_output() when the dataplane already going, and
become no-ops without waking it up again to handle the new request.

Enlightened enough yet?

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

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

[Alexander Graf]

On 06.02.15 03:54, David Gibson wrote:
> On Thu, Feb 05, 2015 at 12:55:45PM +0100, Alexander Graf wrote:
>>
>>
>> On 05.02.15 12:30, David Gibson wrote:
>>> On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
> [snip]
>>>>>>>>>> [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.
>>
>> So at least with vhost-net the in-kernel trapping is optional. If we
>> happen to get MMIO into QEMU, we'll just handle it there.
>>
>> Enlighten me why the iothread stuff can't handle it that way too.
> 
> So, as I understand it, it could, but it doesn't.  Working out how to
> fix it properly requires better understanding of the dataplane code
> than I currently possess,
> 
> So, using virtio-blk as the example case.  Normally the queue notify
> mmio will get routed by the general virtio code to
> virtio_blk_handle_output().
> 
> In the case of dataplane, that just calls
> virtio_blk_data_plane_start().  So the first time we get a vq notify,
> the dataplane is started.  That sets up the host notifier
> (VirtioBusClass::set_host_notifier -> virtio_pci_set_host_notifier ->
> virtio_pci_set_host_notifier_internal -> memory_region_add_eventfd()
> -> memory_region_transaction_commit() ->
> address_space_update_ioeventfds - >address_space_add_del_ioeventfds ->
> kvm_mem_ioeventfd_add -> kvm_set_ioeventfd_mmio -> KVM_IOEVENTFD
> ioctl)
> 
> From this point on further calls to virtio_blk_handle_output() are
> IIUC a "can't happen", because vq notifies should go to the eventfd
> instead, where they will kick the iothread.
> 
> So, with SLOF, the first request is ok - it hits
> virtio_blk_handle_output() which starts the iothread which goes on to
> process the request.
> 
> On the second request, however, we get back into
> virtio_blk_data_plane_start() which sees the iothread is already
> running and aborts.  I think it is assuming that this must be the
> result of a race with another vcpu starting the dataplane, and so
> assumes the racing thread will have woken the dataplane which will
> then handle this vcpu's request as well.
> 
> In our case, however, the IO hcalls go through to
> virtio_blk_handle_output() when the dataplane already going, and
> become no-ops without waking it up again to handle the new request.
> 
> Enlightened enough yet?

So reading this, it sounds like we could just add logic in the virtio
dataplane code that allows for a graceful fallback to QEMU based MMIO by
triggering the eventfd itself in the MMIO handler. When going via this
slow path, we should of course emit a warning (once) to the user ;).

Stefan, what do you think?


Alex

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

[David Gibson]

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

On Fri, 06 Feb 2015 08:56:32 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> 
> On 06.02.15 03:54, David Gibson wrote:
> > On Thu, Feb 05, 2015 at 12:55:45PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 05.02.15 12:30, David Gibson wrote:
> >>> On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
> > [snip]
> >>>>>>>>>> [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.
> >>
> >> So at least with vhost-net the in-kernel trapping is optional. If we
> >> happen to get MMIO into QEMU, we'll just handle it there.
> >>
> >> Enlighten me why the iothread stuff can't handle it that way too.
> > 
> > So, as I understand it, it could, but it doesn't.  Working out how to
> > fix it properly requires better understanding of the dataplane code
> > than I currently possess,
> > 
> > So, using virtio-blk as the example case.  Normally the queue notify
> > mmio will get routed by the general virtio code to
> > virtio_blk_handle_output().
> > 
> > In the case of dataplane, that just calls
> > virtio_blk_data_plane_start().  So the first time we get a vq notify,
> > the dataplane is started.  That sets up the host notifier
> > (VirtioBusClass::set_host_notifier -> virtio_pci_set_host_notifier ->
> > virtio_pci_set_host_notifier_internal -> memory_region_add_eventfd()
> > -> memory_region_transaction_commit() ->
> > address_space_update_ioeventfds - >address_space_add_del_ioeventfds ->
> > kvm_mem_ioeventfd_add -> kvm_set_ioeventfd_mmio -> KVM_IOEVENTFD
> > ioctl)
> > 
> > From this point on further calls to virtio_blk_handle_output() are
> > IIUC a "can't happen", because vq notifies should go to the eventfd
> > instead, where they will kick the iothread.
> > 
> > So, with SLOF, the first request is ok - it hits
> > virtio_blk_handle_output() which starts the iothread which goes on to
> > process the request.
> > 
> > On the second request, however, we get back into
> > virtio_blk_data_plane_start() which sees the iothread is already
> > running and aborts.  I think it is assuming that this must be the
> > result of a race with another vcpu starting the dataplane, and so
> > assumes the racing thread will have woken the dataplane which will
> > then handle this vcpu's request as well.
> > 
> > In our case, however, the IO hcalls go through to
> > virtio_blk_handle_output() when the dataplane already going, and
> > become no-ops without waking it up again to handle the new request.
> > 
> > Enlightened enough yet?
> 
> So reading this, it sounds like we could just add logic in the virtio
> dataplane code that allows for a graceful fallback to QEMU based MMIO by
> triggering the eventfd itself in the MMIO handler. When going via this
> slow path, we should of course emit a warning (once) to the user ;).
> 
> Stefan, what do you think?

So, as I understand it this should be possible.  I did even have a
draft which did this.  However, I don't know the dataplane well enough
to know what gotchas there might be in terms of races, and therefore
how to do this quite right.

Note that this doesn't remove the need for the in-kernel H_LOGICAL_CI_*
hcalls, because those will still be necessary if we get real in-kernel
emulated devices in future.

-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

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

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

[Alexander Graf]

On 09.02.15 01:37, David Gibson wrote:
> On Fri, 06 Feb 2015 08:56:32 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>>
>>
>> On 06.02.15 03:54, David Gibson wrote:
>>> On Thu, Feb 05, 2015 at 12:55:45PM +0100, Alexander Graf wrote:
>>>>
>>>>
>>>> On 05.02.15 12:30, David Gibson wrote:
>>>>> On Thu, Feb 05, 2015 at 11:22:13AM +0100, Alexander Graf wrote:
>>> [snip]
>>>>>>>>>>>> [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.
>>>>
>>>> So at least with vhost-net the in-kernel trapping is optional. If we
>>>> happen to get MMIO into QEMU, we'll just handle it there.
>>>>
>>>> Enlighten me why the iothread stuff can't handle it that way too.
>>>
>>> So, as I understand it, it could, but it doesn't.  Working out how to
>>> fix it properly requires better understanding of the dataplane code
>>> than I currently possess,
>>>
>>> So, using virtio-blk as the example case.  Normally the queue notify
>>> mmio will get routed by the general virtio code to
>>> virtio_blk_handle_output().
>>>
>>> In the case of dataplane, that just calls
>>> virtio_blk_data_plane_start().  So the first time we get a vq notify,
>>> the dataplane is started.  That sets up the host notifier
>>> (VirtioBusClass::set_host_notifier -> virtio_pci_set_host_notifier ->
>>> virtio_pci_set_host_notifier_internal -> memory_region_add_eventfd()
>>> -> memory_region_transaction_commit() ->
>>> address_space_update_ioeventfds - >address_space_add_del_ioeventfds ->
>>> kvm_mem_ioeventfd_add -> kvm_set_ioeventfd_mmio -> KVM_IOEVENTFD
>>> ioctl)
>>>
>>> From this point on further calls to virtio_blk_handle_output() are
>>> IIUC a "can't happen", because vq notifies should go to the eventfd
>>> instead, where they will kick the iothread.
>>>
>>> So, with SLOF, the first request is ok - it hits
>>> virtio_blk_handle_output() which starts the iothread which goes on to
>>> process the request.
>>>
>>> On the second request, however, we get back into
>>> virtio_blk_data_plane_start() which sees the iothread is already
>>> running and aborts.  I think it is assuming that this must be the
>>> result of a race with another vcpu starting the dataplane, and so
>>> assumes the racing thread will have woken the dataplane which will
>>> then handle this vcpu's request as well.
>>>
>>> In our case, however, the IO hcalls go through to
>>> virtio_blk_handle_output() when the dataplane already going, and
>>> become no-ops without waking it up again to handle the new request.
>>>
>>> Enlightened enough yet?
>>
>> So reading this, it sounds like we could just add logic in the virtio
>> dataplane code that allows for a graceful fallback to QEMU based MMIO by
>> triggering the eventfd itself in the MMIO handler. When going via this
>> slow path, we should of course emit a warning (once) to the user ;).
>>
>> Stefan, what do you think?
> 
> So, as I understand it this should be possible.  I did even have a
> draft which did this.  However, I don't know the dataplane well enough
> to know what gotchas there might be in terms of races, and therefore
> how to do this quite right.

I'm sure Stefan knows :).

> Note that this doesn't remove the need for the in-kernel H_LOGICAL_CI_*
> hcalls, because those will still be necessary if we get real in-kernel
> emulated devices in future.

Oh, I definitely agree on that part. I just want to make sure we
gracefully run on older host kernels with newer QEMU versions.


Alex