* [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
@ 2014-09-30 16:13 Amit Shah
2014-10-09 11:18 ` Markus Armbruster
0 siblings, 1 reply; 7+ messages in thread
From: Amit Shah @ 2014-09-30 16:13 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Peter Maydell, Amos Kong, Markus Armbruster
Users of virtio-serial may want to know when a port becomes writable. A
port can stop accepting writes if the guest port is open but not being
read from. In this case, data gets queued up in the virtqueue, and
after the vq is full, writes to the port do not succeed.
When the guest reads off a vq element, and adds a new one for the host
to put data in, we can tell users the port is available for more writes,
via the new ->guest_writable() callback.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
v2: check for port != NULL (Peter Maydell)
---
hw/char/virtio-serial-bus.c | 27 +++++++++++++++++++++++++++
include/hw/virtio/virtio-serial.h | 3 +++
2 files changed, 30 insertions(+)
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3931085..1c7acbf 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -465,6 +465,33 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
{
+ /*
+ * Users of virtio-serial would like to know when guest becomes
+ * writable again -- i.e. if a vq had stuff queued up and the
+ * guest wasn't reading at all, the host would not be able to
+ * write to the vq anymore. Once the guest reads off something,
+ * we can start queueing things up again.
+ */
+ VirtIOSerial *vser;
+ VirtIOSerialPort *port;
+ VirtIOSerialPortClass *vsc;
+
+ vser = VIRTIO_SERIAL(vdev);
+ port = find_port_by_vq(vser, vq);
+
+ if (!port) {
+ return;
+ }
+ vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
+ /*
+ * If guest_connected is false, this call is being made by the
+ * early-boot queueing up of descriptors, which is just noise for
+ * the host apps -- don't disturb them in that case.
+ */
+ if (port->guest_connected && port->host_connected && vsc->guest_writable) {
+ vsc->guest_writable(port);
+ }
}
static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index a679e54..b434f78 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -98,6 +98,9 @@ typedef struct VirtIOSerialPortClass {
/* Guest is now ready to accept data (virtqueues set up). */
void (*guest_ready)(VirtIOSerialPort *port);
+ /* Guest vq became writable again */
+ void (*guest_writable)(VirtIOSerialPort *port);
+
/*
* Guest wrote some data to the port. This data is handed over to
* the app via this callback. The app can return a size less than
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-09-30 16:13 [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users Amit Shah
@ 2014-10-09 11:18 ` Markus Armbruster
2014-10-09 12:17 ` Amit Shah
0 siblings, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2014-10-09 11:18 UTC (permalink / raw)
To: Amit Shah; +Cc: Peter Maydell, Amos Kong, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> Users of virtio-serial may want to know when a port becomes writable. A
> port can stop accepting writes if the guest port is open but not being
> read from. In this case, data gets queued up in the virtqueue, and
> after the vq is full, writes to the port do not succeed.
>
> When the guest reads off a vq element, and adds a new one for the host
> to put data in, we can tell users the port is available for more writes,
> via the new ->guest_writable() callback.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>
> ---
> v2: check for port != NULL (Peter Maydell)
> ---
> hw/char/virtio-serial-bus.c | 27 +++++++++++++++++++++++++++
> include/hw/virtio/virtio-serial.h | 3 +++
> 2 files changed, 30 insertions(+)
>
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 3931085..1c7acbf 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -465,6 +465,33 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
>
> static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
> {
> + /*
> + * Users of virtio-serial would like to know when guest becomes
> + * writable again -- i.e. if a vq had stuff queued up and the
> + * guest wasn't reading at all, the host would not be able to
> + * write to the vq anymore. Once the guest reads off something,
> + * we can start queueing things up again.
> + */
> + VirtIOSerial *vser;
> + VirtIOSerialPort *port;
> + VirtIOSerialPortClass *vsc;
> +
> + vser = VIRTIO_SERIAL(vdev);
> + port = find_port_by_vq(vser, vq);
> +
> + if (!port) {
> + return;
> + }
> + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> +
> + /*
> + * If guest_connected is false, this call is being made by the
> + * early-boot queueing up of descriptors, which is just noise for
> + * the host apps -- don't disturb them in that case.
> + */
> + if (port->guest_connected && port->host_connected && vsc->guest_writable) {
> + vsc->guest_writable(port);
> + }
> }
>
> static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
> diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> index a679e54..b434f78 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -98,6 +98,9 @@ typedef struct VirtIOSerialPortClass {
> /* Guest is now ready to accept data (virtqueues set up). */
> void (*guest_ready)(VirtIOSerialPort *port);
>
> + /* Guest vq became writable again */
> + void (*guest_writable)(VirtIOSerialPort *port);
> +
> /*
> * Guest wrote some data to the port. This data is handed over to
> * the app via this callback. The app can return a size less than
The code should work, but whether it makes sense is hard to judge for
virtio noobs like me without a user of guest_writable. The conditional
guarding vsc->guest_writable(port) in particular.
virtio_add_queue()'s callback being undocumented doesn't exactly help,
either. Fun: the parameter is called handle_output, the argument is
handle_input. Clear as mud!
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-10-09 11:18 ` Markus Armbruster
@ 2014-10-09 12:17 ` Amit Shah
2014-10-09 13:04 ` Peter Maydell
0 siblings, 1 reply; 7+ messages in thread
From: Amit Shah @ 2014-10-09 12:17 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Peter Maydell, Amos Kong, qemu list
On (Thu) 09 Oct 2014 [13:18:16], Markus Armbruster wrote:
> Amit Shah <amit.shah@redhat.com> writes:
>
> > Users of virtio-serial may want to know when a port becomes writable. A
> > port can stop accepting writes if the guest port is open but not being
> > read from. In this case, data gets queued up in the virtqueue, and
> > after the vq is full, writes to the port do not succeed.
> >
> > When the guest reads off a vq element, and adds a new one for the host
> > to put data in, we can tell users the port is available for more writes,
> > via the new ->guest_writable() callback.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> >
> > ---
> > v2: check for port != NULL (Peter Maydell)
> > ---
> > hw/char/virtio-serial-bus.c | 27 +++++++++++++++++++++++++++
> > include/hw/virtio/virtio-serial.h | 3 +++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> > index 3931085..1c7acbf 100644
> > --- a/hw/char/virtio-serial-bus.c
> > +++ b/hw/char/virtio-serial-bus.c
> > @@ -465,6 +465,33 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
> >
> > static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
> > {
> > + /*
> > + * Users of virtio-serial would like to know when guest becomes
> > + * writable again -- i.e. if a vq had stuff queued up and the
> > + * guest wasn't reading at all, the host would not be able to
> > + * write to the vq anymore. Once the guest reads off something,
> > + * we can start queueing things up again.
> > + */
> > + VirtIOSerial *vser;
> > + VirtIOSerialPort *port;
> > + VirtIOSerialPortClass *vsc;
> > +
> > + vser = VIRTIO_SERIAL(vdev);
> > + port = find_port_by_vq(vser, vq);
> > +
> > + if (!port) {
> > + return;
> > + }
> > + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> > +
> > + /*
> > + * If guest_connected is false, this call is being made by the
> > + * early-boot queueing up of descriptors, which is just noise for
> > + * the host apps -- don't disturb them in that case.
> > + */
> > + if (port->guest_connected && port->host_connected && vsc->guest_writable) {
> > + vsc->guest_writable(port);
> > + }
> > }
> >
> > static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
> > diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> > index a679e54..b434f78 100644
> > --- a/include/hw/virtio/virtio-serial.h
> > +++ b/include/hw/virtio/virtio-serial.h
> > @@ -98,6 +98,9 @@ typedef struct VirtIOSerialPortClass {
> > /* Guest is now ready to accept data (virtqueues set up). */
> > void (*guest_ready)(VirtIOSerialPort *port);
> >
> > + /* Guest vq became writable again */
> > + void (*guest_writable)(VirtIOSerialPort *port);
> > +
> > /*
> > * Guest wrote some data to the port. This data is handed over to
> > * the app via this callback. The app can return a size less than
>
> The code should work, but whether it makes sense is hard to judge for
> virtio noobs like me without a user of guest_writable. The conditional
> guarding vsc->guest_writable(port) in particular.
Right. This was originally requested by the spice folks, and they
don't yet have a user implemented (waiting for the spice-char
implementation). But Peter came up with a user; so I posted this w/o
the spice part of it. But looks like Peter has lost the code for his
user, so this patch will have to wait ;-)
> virtio_add_queue()'s callback being undocumented doesn't exactly help,
> either. Fun: the parameter is called handle_output, the argument is
> handle_input. Clear as mud!
Yea - some things in virtio are from the guest's POV so it makes these
things really confusing in qemu.
Amit
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-10-09 12:17 ` Amit Shah
@ 2014-10-09 13:04 ` Peter Maydell
2014-10-10 11:19 ` Amit Shah
0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2014-10-09 13:04 UTC (permalink / raw)
To: Amit Shah; +Cc: Amos Kong, Markus Armbruster, qemu list
On 9 October 2014 13:17, Amit Shah <amit.shah@redhat.com> wrote:
> On (Thu) 09 Oct 2014 [13:18:16], Markus Armbruster wrote:
>> The code should work, but whether it makes sense is hard to judge for
>> virtio noobs like me without a user of guest_writable. The conditional
>> guarding vsc->guest_writable(port) in particular.
>
> Right. This was originally requested by the spice folks, and they
> don't yet have a user implemented (waiting for the spice-char
> implementation). But Peter came up with a user; so I posted this w/o
> the spice part of it. But looks like Peter has lost the code for his
> user, so this patch will have to wait ;-)
I have the QEMU code...
https://git.linaro.org/people/peter.maydell/qemu-arm.git/patch/33895359ddee3696bb24eac24cf8ee4cd697c72c
...I just lost the bit of userspace code I was using to test it
It's not very interesting as a use case though since it's
just an echo-back-everything-you-say test backend (but it
does demonstrate that there are basic things you can't do
at all without some variation on this API).
What I would like to see in this patch is a comment giving
much clearer definition of the semantics of the guest_writable
call: for instance, is it always called when the guest is
writable, or is it only guaranteed to be called if the
QEMU backend has previously tried to do virtio_serial_write
and got back a return code indicating an incomplete write?
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-10-09 13:04 ` Peter Maydell
@ 2014-10-10 11:19 ` Amit Shah
2014-10-10 11:23 ` Peter Maydell
0 siblings, 1 reply; 7+ messages in thread
From: Amit Shah @ 2014-10-10 11:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: marcandre.lureau, Amos Kong, Markus Armbruster, qemu list
On (Thu) 09 Oct 2014 [14:04:53], Peter Maydell wrote:
> On 9 October 2014 13:17, Amit Shah <amit.shah@redhat.com> wrote:
> > On (Thu) 09 Oct 2014 [13:18:16], Markus Armbruster wrote:
> >> The code should work, but whether it makes sense is hard to judge for
> >> virtio noobs like me without a user of guest_writable. The conditional
> >> guarding vsc->guest_writable(port) in particular.
> >
> > Right. This was originally requested by the spice folks, and they
> > don't yet have a user implemented (waiting for the spice-char
> > implementation). But Peter came up with a user; so I posted this w/o
> > the spice part of it. But looks like Peter has lost the code for his
> > user, so this patch will have to wait ;-)
>
> I have the QEMU code...
>
> https://git.linaro.org/people/peter.maydell/qemu-arm.git/patch/33895359ddee3696bb24eac24cf8ee4cd697c72c
>
> ...I just lost the bit of userspace code I was using to test it
> It's not very interesting as a use case though since it's
> just an echo-back-everything-you-say test backend (but it
> does demonstrate that there are basic things you can't do
> at all without some variation on this API).
>
> What I would like to see in this patch is a comment giving
> much clearer definition of the semantics of the guest_writable
> call: for instance, is it always called when the guest is
> writable, or is it only guaranteed to be called if the
> QEMU backend has previously tried to do virtio_serial_write
> and got back a return code indicating an incomplete write?
Right now it's called always; if it's necessary to only call after an
incomplete / failed write, we just need to implement that (and also
add that state to the migration data).
I have no preference; just depends on the users of the API.
Amit
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-10-10 11:19 ` Amit Shah
@ 2014-10-10 11:23 ` Peter Maydell
2014-10-10 12:07 ` Amit Shah
0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2014-10-10 11:23 UTC (permalink / raw)
To: Amit Shah; +Cc: marcandre.lureau, Amos Kong, Markus Armbruster, qemu list
On 10 October 2014 12:19, Amit Shah <amit.shah@redhat.com> wrote:
> On (Thu) 09 Oct 2014 [14:04:53], Peter Maydell wrote:
>> What I would like to see in this patch is a comment giving
>> much clearer definition of the semantics of the guest_writable
>> call: for instance, is it always called when the guest is
>> writable, or is it only guaranteed to be called if the
>> QEMU backend has previously tried to do virtio_serial_write
>> and got back a return code indicating an incomplete write?
>
> Right now it's called always; if it's necessary to only call after an
> incomplete / failed write, we just need to implement that (and also
> add that state to the migration data).
>
> I have no preference; just depends on the users of the API.
Called-always makes sense; I'd just like to see the semantics
documented so you don't have to read the whole implementation
of virtio to figure out what they are :-)
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users
2014-10-10 11:23 ` Peter Maydell
@ 2014-10-10 12:07 ` Amit Shah
0 siblings, 0 replies; 7+ messages in thread
From: Amit Shah @ 2014-10-10 12:07 UTC (permalink / raw)
To: Peter Maydell; +Cc: marcandre.lureau, Amos Kong, Markus Armbruster, qemu list
On (Fri) 10 Oct 2014 [12:23:21], Peter Maydell wrote:
> On 10 October 2014 12:19, Amit Shah <amit.shah@redhat.com> wrote:
> > On (Thu) 09 Oct 2014 [14:04:53], Peter Maydell wrote:
> >> What I would like to see in this patch is a comment giving
> >> much clearer definition of the semantics of the guest_writable
> >> call: for instance, is it always called when the guest is
> >> writable, or is it only guaranteed to be called if the
> >> QEMU backend has previously tried to do virtio_serial_write
> >> and got back a return code indicating an incomplete write?
> >
> > Right now it's called always; if it's necessary to only call after an
> > incomplete / failed write, we just need to implement that (and also
> > add that state to the migration data).
> >
> > I have no preference; just depends on the users of the API.
>
> Called-always makes sense; I'd just like to see the semantics
> documented so you don't have to read the whole implementation
> of virtio to figure out what they are :-)
Sure. I'll do that.
Amit
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-10 12:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-30 16:13 [Qemu-devel] [PATCH v2 1/1] virtio: serial: expose a 'guest_writable' callback for users Amit Shah
2014-10-09 11:18 ` Markus Armbruster
2014-10-09 12:17 ` Amit Shah
2014-10-09 13:04 ` Peter Maydell
2014-10-10 11:19 ` Amit Shah
2014-10-10 11:23 ` Peter Maydell
2014-10-10 12:07 ` Amit Shah
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).