diff mbox

[v4,2/2] KVM: s390: use cookies for ioeventfd

Message ID 1372861854-23043-3-git-send-email-cornelia.huck@de.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cornelia Huck July 3, 2013, 2:30 p.m. UTC
Make use of cookies for the virtio ccw notification hypercall to speed up
lookup of devices on the io bus.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/kvm/diag.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini July 3, 2013, 3:30 p.m. UTC | #1
Il 03/07/2013 16:30, Cornelia Huck ha scritto:
> +	/*
> +	 * Return cookie in gpr 2, but don't overwrite the register if the
> +	 * diagnose will be handled by userspace.
> +	 */
> +	if (ret != -EOPNOTSUPP)
> +		vcpu->run->s.regs.gprs[2] = ret;

I think this should now be "if (ret >= 0)".

>  	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */

The comment is now obsolete.

>  	return ret < 0 ? ret : 0;

Otherwise looks good, thanks!

Paolo

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cornelia Huck July 3, 2013, 4:33 p.m. UTC | #2
On Wed, 03 Jul 2013 17:30:40 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 03/07/2013 16:30, Cornelia Huck ha scritto:
> > +	/*
> > +	 * Return cookie in gpr 2, but don't overwrite the register if the
> > +	 * diagnose will be handled by userspace.
> > +	 */
> > +	if (ret != -EOPNOTSUPP)
> > +		vcpu->run->s.regs.gprs[2] = ret;
> 
> I think this should now be "if (ret >= 0)".

Hm, we don't want to kill gpr 2's old contents if userspace will do
something, which means -EOPNOTSUPP.

> 
> >  	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */
> 
> The comment is now obsolete.

s/kvm_io_bus_write/kvm_io_bus_write_cookie/ ? Otherwise, this is still
true.

> 
> >  	return ret < 0 ? ret : 0;
> 
> Otherwise looks good, thanks!
> 
> Paolo
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini July 4, 2013, 6:54 a.m. UTC | #3
Il 03/07/2013 18:33, Cornelia Huck ha scritto:
> On Wed, 03 Jul 2013 17:30:40 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 03/07/2013 16:30, Cornelia Huck ha scritto:
>>> +	/*
>>> +	 * Return cookie in gpr 2, but don't overwrite the register if the
>>> +	 * diagnose will be handled by userspace.
>>> +	 */
>>> +	if (ret != -EOPNOTSUPP)
>>> +		vcpu->run->s.regs.gprs[2] = ret;
>>
>> I think this should now be "if (ret >= 0)".
> 
> Hm, we don't want to kill gpr 2's old contents if userspace will do
> something, which means -EOPNOTSUPP.

In the end kvm_io_bus_write_cookie only returns -EOPNOTSUPP if there is
an error, so it works.  But if this were to change, the code would
break.  That's why I suggested testing "ret >= 0" rather than "ret !=
-EOPNOTSUPP".  But in the end it is the same.

>>
>>>  	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */
>>
>> The comment is now obsolete.
> 
> s/kvm_io_bus_write/kvm_io_bus_write_cookie/ ? Otherwise, this is still
> true.

True but somewhat misplaced, it is basically saying the same thing as
the "Return cookie in gpr 2" comment just above.

Anyhow, these are very small details.  I changed kvm_io_bus_write to
kvm_io_bus_write_cookie in the comment and applied the patches to kvm-queue.

Paolo

>>
>>>  	return ret < 0 ? ret : 0;
>>
>> Otherwise looks good, thanks!
>>
>> Paolo
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov July 14, 2013, 9:25 a.m. UTC | #4
On Thu, Jul 04, 2013 at 08:54:32AM +0200, Paolo Bonzini wrote:
> Il 03/07/2013 18:33, Cornelia Huck ha scritto:
> > On Wed, 03 Jul 2013 17:30:40 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> >> Il 03/07/2013 16:30, Cornelia Huck ha scritto:
> >>> +	/*
> >>> +	 * Return cookie in gpr 2, but don't overwrite the register if the
> >>> +	 * diagnose will be handled by userspace.
> >>> +	 */
> >>> +	if (ret != -EOPNOTSUPP)
> >>> +		vcpu->run->s.regs.gprs[2] = ret;
> >>
> >> I think this should now be "if (ret >= 0)".
> > 
> > Hm, we don't want to kill gpr 2's old contents if userspace will do
> > something, which means -EOPNOTSUPP.
> 
> In the end kvm_io_bus_write_cookie only returns -EOPNOTSUPP if there is
> an error, so it works.  But if this were to change, the code would
> break.  That's why I suggested testing "ret >= 0" rather than "ret !=
> -EOPNOTSUPP".  But in the end it is the same.
> 
> >>
> >>>  	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */
> >>
> >> The comment is now obsolete.
> > 
> > s/kvm_io_bus_write/kvm_io_bus_write_cookie/ ? Otherwise, this is still
> > true.
> 
> True but somewhat misplaced, it is basically saying the same thing as
> the "Return cookie in gpr 2" comment just above.
> 
> Anyhow, these are very small details.  I changed kvm_io_bus_write to
> kvm_io_bus_write_cookie in the comment and applied the patches to kvm-queue.
> 
1/2 broke x86. QEMU prints "Invalid read from memory region kvm-pic" and
guest hangs. Looks like IO is forwarded to userspace instead of in kernel device
for some reason. Un-applying for now.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 3074475..e88c76e 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -119,11 +119,20 @@  static int __diag_virtio_hypercall(struct kvm_vcpu *vcpu)
 	 * The layout is as follows:
 	 * - gpr 2 contains the subchannel id (passed as addr)
 	 * - gpr 3 contains the virtqueue index (passed as datamatch)
+	 * - gpr 4 contains the index on the bus (optionally)
 	 */
-	ret = kvm_io_bus_write(vcpu->kvm, KVM_VIRTIO_CCW_NOTIFY_BUS,
-				vcpu->run->s.regs.gprs[2],
-				8, &vcpu->run->s.regs.gprs[3]);
+	ret = kvm_io_bus_write_cookie(vcpu->kvm, KVM_VIRTIO_CCW_NOTIFY_BUS,
+				      vcpu->run->s.regs.gprs[2],
+				      8, &vcpu->run->s.regs.gprs[3],
+				      vcpu->run->s.regs.gprs[4]);
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
+
+	/*
+	 * Return cookie in gpr 2, but don't overwrite the register if the
+	 * diagnose will be handled by userspace.
+	 */
+	if (ret != -EOPNOTSUPP)
+		vcpu->run->s.regs.gprs[2] = ret;
 	/* kvm_io_bus_write returns -EOPNOTSUPP if it found no match. */
 	return ret < 0 ? ret : 0;
 }