Re: [Minios-devel] [UNIKRAFT PATCH v3 10/14] plat/drivers: Start virtio block device

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hi Justin,

we introduced these operations on the ukblkdev API design on purpose: A 
reason was that we wanted that there is no predone decision on which 
devices are initialized. It should happen by libraries using the devices 
or even at Unikernel application level. This way we make sure that 
unused devices are not introducing any boot delay or any other unwanted 
overhead because of their initialization. It may also be easier to 
implement parallelized device initialization when doing this at a later 
stage during the Unikernel lifetime.
Thanks,

Simon

Reviewed-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>

On 11.03.20 04:49, Justin He wrote:
> Hi Roxana
> Although the function of this patch looks good to me, I still prefer let
> uk_blkdev_start/configure be automatically invoked by virtio_blk device instead
> of the application itself.
> But as you once mentioned before, Simon agreed this patch.
> If that, @Simon Kuenzer Simon, would you please add your reviewed-by to this
> single patch, thanks.
>
> --
> Cheers,
> Justin (Jia He)
>
>
>
>
> > -----Original Message-----
> > From: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> > Sent: Friday, March 6, 2020 4:30 AM
> > To: minios-devel@xxxxxxxxxxxxx
> > Cc: Justin He <Justin.He@xxxxxxx>; Roxana Nicolescu
> > <nicolescu.roxana1996@xxxxxxxxx>
> > Subject: [UNIKRAFT PATCH v3 10/14] plat/drivers: Start virtio block device
> >
> > This patch introduces start/stop operations to virtio device.
> > This implies connection to backend by setting the DRIVER_OK status
> > bit.
> >
> > Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> > ---
> >   plat/drivers/virtio/virtio_blk.c | 48 ++++++++++++++++++++++++++++++++
> >   1 file changed, 48 insertions(+)
> >
> > diff --git a/plat/drivers/virtio/virtio_blk.c b/plat/drivers/virtio/virtio_blk.c
> > index cb732135..ecd8e691 100644
> > --- a/plat/drivers/virtio/virtio_blk.c
> > +++ b/plat/drivers/virtio/virtio_blk.c
> > @@ -354,6 +354,52 @@ exit:
> >   return rc;
> >   }
> >
> > +static int virtio_blkdev_start(struct uk_blkdev *dev)
> > +{
> > +struct virtio_blk_device *d;
> > +
> > +UK_ASSERT(dev != NULL);
> > +
> > +d = to_virtioblkdev(dev);
> > +virtio_dev_drv_up(d->vdev);
> > +
> > +uk_pr_info(DRIVER_NAME": %"__PRIu16" started\n", d->uid);
> > +
> > +return 0;
> > +}
> > +
> > +/* If one queue has unconsumed responses it returns -EBUSY
> > + * TODO restart doesn't work
> > + **/
> > +static int virtio_blkdev_stop(struct uk_blkdev *dev)
> > +{
> > +struct virtio_blk_device *d;
> > +uint16_t q_id;
> > +int rc = 0;
> > +
> > +UK_ASSERT(dev != NULL);
> > +
> > +d = to_virtioblkdev(dev);
> > +for (q_id = 0; q_id < d->nb_queues; ++q_id) {
> > +if (virtqueue_hasdata(d->qs[q_id].vq)) {
> > +uk_pr_err("Queue:%"__PRIu16" has unconsumed
> > responses\n",
> > +q_id);
> > +return -EBUSY;
> > +}
> > +}
> > +
> > +rc = virtio_dev_reset(d->vdev);
> > +if (rc) {
> > +uk_pr_info(DRIVER_NAME":%"__PRIu16" stopped", d->uid);
> > +goto out;
> > +}
> > +
> > +uk_pr_warn(DRIVER_NAME":%"__PRIu16" Start is not allowed!!!", d-
> > > uid);
> > +
> > +out:
> > +return rc;
> > +}
> > +
> >   static int virtio_blkdev_unconfigure(struct uk_blkdev *dev)
> >   {
> >   struct virtio_blk_device *d;
> > @@ -507,6 +553,8 @@ static const struct uk_blkdev_ops virtio_blkdev_ops =
> > {
> >   .queue_get_info = virtio_blkdev_queue_info_get,
> >   .queue_setup = virtio_blkdev_queue_setup,
> >   .queue_intr_enable = virtio_blkdev_queue_intr_enable,
> > +.dev_start = virtio_blkdev_start,
> > +.dev_stop = virtio_blkdev_stop,
> >   .queue_intr_disable = virtio_blkdev_queue_intr_disable,
> >   .queue_release = virtio_blkdev_queue_release,
> >   .dev_unconfigure = virtio_blkdev_unconfigure,
> > --
> > 2.17.1
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended recipient, 
> please notify the sender immediately and do not disclose the contents to any 
> other person, use it for any purpose, or store or copy the information in any 
> medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel