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

[Justin He <Justin.He@xxxxxxx>]

Hi Simon, thanks for the explanation
Hence,
Reviewed-by: Jia He <justin.he@xxxxxxx>

--
Cheers,
Justin (Jia He)



> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: Wednesday, March 11, 2020 9:03 PM
> To: Justin He <Justin.He@xxxxxxx>; Roxana Nicolescu
> <nicolescu.roxana1996@xxxxxxxxx>; minios-devel@xxxxxxxxxxxxx
> Subject: Re: [UNIKRAFT PATCH v3 10/14] plat/drivers: Start virtio block device
>
> 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.
> >
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