[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v2 5/8] lib/uk9p: Add 9P device implementation



Hi Simon,

Please see my comments inline regarding changes in v3.

Thanks!
Cristi

On Sat, Jun 29, 2019 at 4:36 PM Simon Kuenzer <simon.kuenzer@xxxxxxxxx> wrote:
>
> On 29.06.19 10:56, Cristian Banu wrote:
> > This patch adds the operations that should be supported by transport
> > layers, the API to interact with them and to manage requests.
> >
> > Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
> > ---
> >   lib/uk9p/9pdev.c                  | 326 
> > ++++++++++++++++++++++++++++++++++++++
> >   lib/uk9p/9pdev_trans.c            |   4 +
> >   lib/uk9p/Makefile.uk              |   1 +
> >   lib/uk9p/exportsyms.uk            |   9 ++
> >   lib/uk9p/include/uk/9pdev.h       | 188 ++++++++++++++++++++++
> >   lib/uk9p/include/uk/9pdev_core.h  | 169 ++++++++++++++++++++
> >   lib/uk9p/include/uk/9pdev_trans.h |   3 +
> >   7 files changed, 700 insertions(+)
> >   create mode 100644 lib/uk9p/9pdev.c
> >   create mode 100644 lib/uk9p/include/uk/9pdev.h
> >   create mode 100644 lib/uk9p/include/uk/9pdev_core.h
> >
> > diff --git a/lib/uk9p/9pdev.c b/lib/uk9p/9pdev.c
> > new file mode 100644
> > index 000000000000..05a581eafbdb
> > --- /dev/null
> > +++ b/lib/uk9p/9pdev.c
> > @@ -0,0 +1,326 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause */
> > +/*
> > + * Authors: Cristian Banu <cristb@xxxxxxxxx>
> > + *
> > + * Copyright (c) 2019, University Politehnica of Bucharest. All rights 
> > reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of the copyright holder nor the names of its
> > + *    contributors may be used to endorse or promote products derived from
> > + *    this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> > IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, 
> > THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> > PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS 
> > BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF 
> > THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> > + */
> > +
> > +#include <stdbool.h>
> > +#include <string.h>
> > +#include <uk/config.h>
> > +#include <uk/plat/spinlock.h>
> > +#include <uk/alloc.h>
> > +#include <uk/essentials.h>
> > +#include <uk/errptr.h>
> > +#include <uk/bitmap.h>
> > +#include <uk/refcount.h>
> > +#include <uk/wait.h>
> > +#include <uk/9pdev.h>
> > +#include <uk/9pdev_trans.h>
> > +#include <uk/9preq.h>
> > +#if CONFIG_LIBUKSCHED
> > +#include <uk/sched.h>
> > +#include <uk/wait.h>
> > +#endif
> > +
> > +static int _req_mgmt_init(struct uk_9pdev_req_mgmt *req_mgmt)
> > +{
> > +     ukarch_spin_lock_init(&req_mgmt->spinlock);
> > +     uk_bitmap_zero(req_mgmt->tag_bm, UK_9P_NUMTAGS);
> > +     UK_INIT_LIST_HEAD(&req_mgmt->req_list);
> > +
> > +     return 0;
> > +}
>
> I think this function can be void, right?
Yes, fixed.
>
> > +
> > +static void _req_mgmt_add_req_locked(struct uk_9pdev_req_mgmt *req_mgmt,
> > +                             struct uk_9preq *req)
> > +{
> > +     uk_bitmap_set(req_mgmt->tag_bm, req->tag, 1);
> > +     uk_list_add(&req->_list, &req_mgmt->req_list);
> > +}
> > +
> > +static void _req_mgmt_del_req_locked(struct uk_9pdev_req_mgmt *req_mgmt,
> > +                             struct uk_9preq *req)
> > +{
> > +     uk_bitmap_clear(req_mgmt->tag_bm, req->tag, 1);
> > +     uk_list_del(&req->_list);
> > +}
> > +
> > +static int _req_mgmt_next_tag_locked(struct uk_9pdev_req_mgmt *req_mgmt,
> > +                             uint16_t *tag)
> > +{
> > +     *tag = uk_find_next_zero_bit(req_mgmt->tag_bm, UK_9P_NUMTAGS, 0);
> > +
> > +     return 0;
> > +}
>
> Don't you want to return the tag instead using it as output parameter?
> There are no errors returned. Otherwise this function can be void.
>
> > +
> > +static void _req_mgmt_cleanup(struct uk_9pdev_req_mgmt *req_mgmt __unused)
> > +{
> > +     unsigned long flags;
> > +     uint16_t tag;
> > +     struct uk_9preq *req, *reqn;
> > +
> > +     ukplat_spin_lock_irqsave(&req_mgmt->spinlock, flags);
> > +     uk_list_for_each_entry_safe(req, reqn, &req_mgmt->req_list, _list) {
> > +             tag = req->tag;
> > +             _req_mgmt_del_req_locked(req_mgmt, req);
> > +             if (!uk_9preq_put(req)) {
> > +                     uk_pr_warn("Tag %d still has references on 
> > cleanup.\n",
> > +                             tag);
> > +             }
> > +     }
> > +     ukplat_spin_unlock_irqrestore(&req_mgmt->spinlock, flags);
> > +}
> > +
> > +struct uk_9pdev *uk_9pdev_connect(const struct uk_9pdev_trans *trans,
> > +                             const char *device_identifier,
> > +                             const char *mount_args,
> > +                             struct uk_alloc *a)
> > +{
> > +     struct uk_9pdev *dev;
> > +     int rc = 0;
> > +
> > +     UK_ASSERT(trans);
> > +     UK_ASSERT(device_identifier);
> > +
> > +     if (a == NULL)
> > +             a = trans->a;
> > +
> > +     dev = uk_calloc(a, 1, sizeof(*dev));
> > +     if (dev == NULL) {
> > +             rc = -ENOMEM;
> > +             goto out;
> > +     }
> > +
> > +     dev->ops = trans->ops;
> > +     dev->a = a;
> > +
> > +#if CONFIG_LIBUKSCHED
> > +     uk_waitq_init(&dev->xmit_wq);
> > +#endif
> > +
> > +     rc = _req_mgmt_init(&dev->_req_mgmt);
> > +     if (rc < 0)
> > +             goto free_dev;
> > +
> > +     rc = dev->ops->connect(dev, device_identifier, mount_args);
> > +     if (rc < 0)
> > +             goto free_req;
> > +
> > +     UK_ASSERT(dev->msize != 0);
> > +
> > +     dev->state = UK_9PDEV_CONNECTED;
> > +     goto out;
> > +
>
> Minor thing: Since you do not need any clean-up in the success exit, you
> could return here the dev directly and remove the if statement in the
> out-case (which becomes to an error-only exit).
Done.
>
> > +free_req:
> > +     _req_mgmt_cleanup(&dev->_req_mgmt);
> > +free_dev:
> > +     uk_free(a, dev);
> > +out:
> > +     if (rc == 0)
> > +             return dev;
> > +     return ERR2PTR(rc);
> > +}
> > +
> > +int uk_9pdev_disconnect(struct uk_9pdev *dev)
> > +{
> > +     int rc = 0;
> > +
> > +     UK_ASSERT(dev);
> > +     UK_ASSERT(dev->state == UK_9PDEV_CONNECTED);
> > +
> > +     dev->state = UK_9PDEV_DISCONNECTING;
> > +
> > +     /* Clean up the requests before closing the channel. */
> > +     _req_mgmt_cleanup(&dev->_req_mgmt);
> > +
> > +     rc = dev->ops->disconnect(dev);
>
> It seems in case of driver failure on disconnect you still proceed with
> disconnecting. Is this intended?
Yes, I documented this in the header file as well.
>
> > +
> > +     uk_free(dev->a, dev);
> > +     return rc;
> > +}
> > +
> > +int uk_9pdev_request(struct uk_9pdev *dev, struct uk_9preq *req)
> > +{
> > +     int rc;
> > +
> > +     UK_ASSERT(dev);
> > +     UK_ASSERT(req);
> > +
> > +     if (UK_READ_ONCE(req->state) != UK_9PREQ_READY) {
> > +             rc = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     if (dev->state != UK_9PDEV_CONNECTED) {
> > +             rc = -EIO;
> > +             goto out;
> > +     }
> > +
> > +#if CONFIG_LIBUKSCHED
> > +     uk_waitq_wait_event(&dev->xmit_wq,
> > +             (rc = dev->ops->request(dev, req)) >= 0 || rc != -ENOSPC);
> > +#else
> > +     rc = -ENOSPC;
> > +     while (rc < 0 && rc == -ENOSPC)
>
> Isn't the condition (rc == -ENOSPC) good enough for the while loop?
> It looks also to me that a 'do {} while()' loop would be nicer in this case.
Fixed, modified the wait_event condition as well.
>
> > +             rc = dev->ops->request(dev, req);
> > +#endif
> > +
> > +out:
> > +     return rc;
> > +}
> > +
> > +void uk_9pdev_xmit_ready(struct uk_9pdev *dev)
> > +{
> > +#if CONFIG_LIBUKSCHED
> > +     uk_waitq_wake_up(&dev->xmit_wq);
> > +#endif
> > +}
> > +
> > +struct uk_9preq *uk_9pdev_call(struct uk_9pdev *dev, uint8_t type,
> > +                     uint32_t size, const char *fmt, ...)
> > +{
> > +     struct uk_9preq *req;
> > +     va_list vl;
> > +     int rc;
> > +
> > +     req = uk_9pdev_req_create(dev, type, size);
> > +     if (PTRISERR(req))
> > +             return req;
> > +
> > +     va_start(vl, fmt);
> > +     rc = uk_9preq_vserialize(req, fmt, vl);
> > +     va_end(vl);
> > +
> > +     if (rc < 0)
> > +             goto out;
> > +
> > +     rc = uk_9preq_ready(req, UK_9PREQ_ZCDIR_NONE, NULL, 0, 0);
> > +     if (rc < 0)
> > +             goto out;
> > +
> > +     rc = uk_9pdev_request(dev, req);
> > +     if (rc < 0)
> > +             goto out;
> > +
> > +     rc = uk_9preq_waitreply(req);
> > +     if (rc < 0)
> > +             goto out;
> > +
> > +     return req;
> > +out:
> > +     uk_9pdev_req_remove(dev, req);
> > +     return ERR2PTR(rc);
> > +}
> > +
> > +struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type,
> > +                             uint32_t size)
> > +{
> > +     struct uk_9preq *req;
> > +     int rc = 0;
> > +     uint16_t tag;
> > +     unsigned long flags;
> > +
> > +     UK_ASSERT(dev);
> > +
> > +     size = MIN(size, dev->msize);
> > +
> > +     req = uk_9preq_alloc(dev->a, size);
> > +     if (req == NULL) {
> > +             rc = -ENOMEM;
> > +             goto out;
> > +     }
> > +
> > +     ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
> > +     if (type == UK_9P_TVERSION) {
> > +             tag = UK_9P_NOTAG;
> > +             rc = 0;
> > +     } else
> > +             rc = _req_mgmt_next_tag_locked(&dev->_req_mgmt, &tag);
> > +
> > +     if (rc < 0) {
> > +             ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, 
> > flags);
> > +             goto out_free;
> > +     }
> > +
> > +     req->tag = tag;
> > +     req->xmit.type = type;
> > +
> > +     _req_mgmt_add_req_locked(&dev->_req_mgmt, req);
> > +     ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, flags);
> > +
> > +     req->state = UK_9PREQ_INITIALIZED;
> > +     goto out;
>
> I think you can also here directly return so that you do not need the if
> statement after the out label. But this is really a minor thing.
Fixed.
>
> > +
> > +out_free:
> > +     uk_9preq_put(req);
> > +out:
> > +     if (rc == 0)
> > +             return req;
> > +     return ERR2PTR(rc);
> > +}
> > +
> > +struct uk_9preq *uk_9pdev_req_lookup(struct uk_9pdev *dev, uint16_t tag)
> > +{
> > +     unsigned long flags;
> > +     struct uk_9preq *req;
> > +     int rc = -EINVAL;
> > +
> > +     ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
> > +     uk_list_for_each_entry(req, &dev->_req_mgmt.req_list, _list) {
> > +             if (tag != req->tag)
> > +                     continue;
> > +             rc = 0;
> > +             uk_9preq_get(req);
> > +             break;
> > +     }
> > +     ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, flags);
> > +
> > +     if (rc == 0)
> > +             return req;
> > +
> > +     return ERR2PTR(rc);
> > +}
> > +
> > +int uk_9pdev_req_remove(struct uk_9pdev *dev, struct uk_9preq *req)
> > +{
> > +     unsigned long flags;
> > +
> > +     ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
> > +     _req_mgmt_del_req_locked(&dev->_req_mgmt, req);
> > +     ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, flags);
> > +
> > +     return uk_9preq_put(req);
> > +}
> > +
> > +void uk_9pdev_adjust_msize(struct uk_9pdev *dev, uint32_t msize)
> > +{
> > +     dev->msize = MIN(dev->msize, msize);
> > +}
>
> Hum, with this function you can only decrease the msize, never increase.
> You are also never getting a feedback on which size the device is
> operating now. Is all of this intended? Otherwise I would return an int
> to tell if the wished msize is out of range. I also would save the max
> possible msize separately from the current applied msize on the uk_9pdev
> structure.
It was intended in the sense that when starting a new 9pfs session, the
client sends the maximum message size it supports and the server replies
with the maximum message size both sides support (which is always less
than or equal to the client-side maximum message size).
Nevertheless, I removed this function and added two functions
*_set_size(), *_get_size() which look cleaner and allow more flexibility.
>
> > diff --git a/lib/uk9p/9pdev_trans.c b/lib/uk9p/9pdev_trans.c
> > index 6372876765b2..0da50e51206e 100644
> > --- a/lib/uk9p/9pdev_trans.c
> > +++ b/lib/uk9p/9pdev_trans.c
> > @@ -47,6 +47,10 @@ int uk_9pdev_trans_register(struct uk_9pdev_trans *trans)
> >   {
> >       UK_ASSERT(trans);
> >       UK_ASSERT(trans->name);
> > +     UK_ASSERT(trans->ops);
> > +     UK_ASSERT(trans->ops->connect);
> > +     UK_ASSERT(trans->ops->disconnect);
> > +     UK_ASSERT(trans->ops->request);
> >       UK_ASSERT(trans->a);
> >
> >       uk_list_add_tail(&trans->_list, &uk_9pdev_trans_list);
> > diff --git a/lib/uk9p/Makefile.uk b/lib/uk9p/Makefile.uk
> > index aea722a585b9..34cc987a2f9c 100644
> > --- a/lib/uk9p/Makefile.uk
> > +++ b/lib/uk9p/Makefile.uk
> > @@ -5,3 +5,4 @@ CXXINCLUDES-$(CONFIG_LIBUK9P)         += 
> > -I$(LIBUK9P_BASE)/include
> >
> >   LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pdev_trans.c
> >   LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9preq.c
> > +LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pdev.c
> > diff --git a/lib/uk9p/exportsyms.uk b/lib/uk9p/exportsyms.uk
> > index 850ffa2db233..c373308fb0a2 100644
> > --- a/lib/uk9p/exportsyms.uk
> > +++ b/lib/uk9p/exportsyms.uk
> > @@ -12,3 +12,12 @@ uk_9preq_copy_from
> >   uk_9preq_receive_cb
> >   uk_9preq_waitreply
> >   uk_9preq_error
> > +uk_9pdev_connect
> > +uk_9pdev_disconnect
> > +uk_9pdev_request
> > +uk_9pdev_xmit_ready
> > +uk_9pdev_call
> > +uk_9pdev_req_create
> > +uk_9pdev_req_lookup
> > +uk_9pdev_req_remove
> > +uk_9pdev_adjust_msize
> > diff --git a/lib/uk9p/include/uk/9pdev.h b/lib/uk9p/include/uk/9pdev.h
> > new file mode 100644
> > index 000000000000..f2210bbffc92
> > --- /dev/null
> > +++ b/lib/uk9p/include/uk/9pdev.h
> > @@ -0,0 +1,188 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause */
> > +/*
> > + * Authors: Cristian Banu <cristb@xxxxxxxxx>
> > + *
> > + * Copyright (c) 2019, University Politehnica of Bucharest. All rights 
> > reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of the copyright holder nor the names of its
> > + *    contributors may be used to endorse or promote products derived from
> > + *    this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> > IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, 
> > THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> > PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS 
> > BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF 
> > THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> > + */
> > +
> > +#ifndef __UK_9PDEV__
> > +#define __UK_9PDEV__
> > +
> > +#include <inttypes.h>
> > +#include <uk/config.h>
> > +#include <uk/alloc.h>
> > +#include <uk/assert.h>
> > +#include <uk/essentials.h>
> > +#include <uk/9pdev_core.h>
> > +#include <uk/plat/irq.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +struct uk_9pdev_trans;
> > +
> > +/**
> > + * Connect to an underlying device, obtaining a 9pdev handle to the 
> > connection.
> > + *
> > + * @param trans
> > + *   The underlying transport.
> > + * @param device_identifier
> > + *   The identifier of the device (for virtio and xen, the mount tag).
> > + * @param mount_args
> > + *   Arguments passed down from the mount() call.
> > + * @param a
> > + *   (Optional) Allocator used for any allocations done by this 9pdev.
> > + *   If NULL, the transport-specific allocator will be used.
> > + * @return
> > + *   - Connection handle, if successful.
> > + *   - Error pointer, otherwise.
> > + */
> > +struct uk_9pdev *uk_9pdev_connect(const struct uk_9pdev_trans *trans,
> > +                             const char *device_identifier,
> > +                             const char *mount_args,
> > +                             struct uk_alloc *a);
> > +
> > +/**
> > + * Disconnect from the underlying device.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @return
> > + *   - (0): Successful.
> > + *   - (< 0): Failure to disconnect.
> > + */
> > +int uk_9pdev_disconnect(struct uk_9pdev *dev);
>
> I think you should either document that even in case of failure the
> device is closed or expect that the user is retrying to disconnect.
>
> > +
> > +/**
> > + * Send a 9P request to the given 9P device.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param req
> > + *   The 9P request.
> > + * @return
> > + *   - (0): Successful.
> > + *   - (< 0): Failed.
> > + */
> > +int uk_9pdev_request(struct uk_9pdev *dev, struct uk_9preq *req);
> > +
> > +/**
> > + * Notify the 9P device that the device's transmit queue is not full and
> > + * it may attempt to send requests again.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + */
> > +void uk_9pdev_xmit_ready(struct uk_9pdev *dev);
>
> Maybe call this function then ..._xmit_notify() instead?
Done.
>
> > +
> > +/**
> > + * Creates and sends 9P request to the given 9P device, serializing it with
> > + * the given arguments. This function acts as a shorthand for the explicit
> > + * calls to req_create(), serialize(), ready(), request(), waitreply().
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param type
> > + *   Transmit type of the request, e.g. Tversion, Tread, and so on.
> > + * @param size
> > + *   The maximum size for the receive and send buffers.
> > + * @param fmt
> > + *   The format of the data to be serialized, in the way 
> > uk_9preq_serialize()
> > + *   expects it.
> > + * @param ...
> > + *   The arguments to be serialized.
> > + * @return
> > + *   - (!PTRISERR): The 9p request in the UK_9PREQ_RECEIVED state.
> > + *   - PTRISERR: The error code with which any of the steps failed.
> > + */
> > +struct uk_9preq *uk_9pdev_call(struct uk_9pdev *dev, uint8_t type,
> > +                     uint32_t size, const char *fmt, ...);
> > +
> > +/**
> > + * Create a new request, automatically allocating its tag, based on its 
> > type.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param type
> > + *   Transmit type of the request, e.g. Tversion, Tread, and so on.
> > + * @param size
> > + *   The maximum size for the receive and send buffers.
> > + * @return
> > + *   If not an error pointer, the created request.
> > + *   Otherwise, the error in creating the request:
> > + *   - ENOMEM: No memory for the request or no available tags.
> > + */
> > +struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type,
> > +                             uint32_t size);
> > +
> > +/**
> > + * Looks up a request based on the given tag. This is generally used by
> > + * transport layers on receiving a 9P message.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param tag
> > + *   The tag to look up.
> > + * @return
> > + *   - NULL: No request with the given tag was found.
> > + *   - (!=NULL): The request.
> > + */
> > +struct uk_9preq *uk_9pdev_req_lookup(struct uk_9pdev *dev, uint16_t tag);
> > +
> > +/**
> > + * Remove a request from the given 9p device. If the request is in-flight,
> > + * it will be freed when all the references to it are gone.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param req
> > + *   The request to be removed.
> > + * @return
> > + *   - 0: There are more active references.
> > + *   - 1: This was the last reference to the request.
> > + */
> > +int uk_9pdev_req_remove(struct uk_9pdev *dev, struct uk_9preq *req);
> > +
> > +/**
> > + * Adjusts the message size to the given maximum size.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param msize
> > + *   Allowed maximum message size.
> > + */
> > +void uk_9pdev_adjust_msize(struct uk_9pdev *dev, uint32_t msize);
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* __UK_9PDEV__ */
> > diff --git a/lib/uk9p/include/uk/9pdev_core.h 
> > b/lib/uk9p/include/uk/9pdev_core.h
> > new file mode 100644
> > index 000000000000..d7ef341e12d6
> > --- /dev/null
> > +++ b/lib/uk9p/include/uk/9pdev_core.h
> > @@ -0,0 +1,169 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause */
> > +/*
> > + * Authors: Cristian Banu <cristb@xxxxxxxxx>
> > + *
> > + * Copyright (c) 2019, University Politehnica of Bucharest. All rights 
> > reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of the copyright holder nor the names of its
> > + *    contributors may be used to endorse or promote products derived from
> > + *    this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> > IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, 
> > THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> > PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS 
> > BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF 
> > THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> > + */
> > +
> > +#ifndef __UK_9PDEV_CORE__
> > +#define __UK_9PDEV_CORE__
> > +
> > +#include <string.h>
> > +#include <inttypes.h>
> > +#include <uk/config.h>
> > +#include <uk/arch/spinlock.h>
> > +#include <uk/bitmap.h>
> > +#include <uk/list.h>
> > +#include <uk/9p_core.h>
> > +#if CONFIG_LIBUKSCHED
> > +#include <uk/wait_types.h>
> > +#endif
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +struct uk_9pdev;
> > +struct uk_9preq;
> > +
> > +/**
> > + * Function type used for connecting to a device on a certain transport.
> > + * The implementation should also set the msize field in the 9P device
> > + * struct to the maximum allowed message size.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @param device_identifier
> > + *   The identifier of the underlying device (mount_tag for virtio, etc.)
> > + * @param mount_args
> > + *   Arguments received by the mount() call, for transport-specific 
> > options.
> > + * @return
> > + *   - (-EBUSY): Device is already in-use.
> > + *   - (-ENOENT): Device does not exist.
> > + *   - (0): Successful.
> > + *   - (< 0): Failed with a transport layer dependent error.
> > + */
> > +typedef int (*uk_9pdev_connect_t)(struct uk_9pdev *dev,
> > +                             const char *device_identifier,
> > +                             const char *mount_args);
> > +
> > +/**
> > + * Function type used for disconnecting from the device.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P Device.
> > + * @return
> > + *   - (0): Successful.
> > + *   - (< 0): Failed with a transport layer dependent error.
> > + */
> > +typedef int (*uk_9pdev_disconnect_t)(struct uk_9pdev *dev);
> > +
> > +/**
> > + * Function type used for sending a request to the 9P device.
> > + *
> > + * @param dev
> > + *   The Unikraft 9P device.
> > + * @param req
> > + *   Reference to the request to be sent.
> > + * @return
> > + *   - (0): Successful.
> > + *   - (< 0): Failed. If -ENOSPC, then the transport layer does not have 
> > enough
> > + *   space to send this request and retries are required.
> > + */
> > +typedef int (*uk_9pdev_request_t)(struct uk_9pdev *dev,
> > +                             struct uk_9preq *req);
> > +
> > +/**
> > + * A structure used to store the operations supported by a certain 
> > transport.
> > + */
> > +struct uk_9pdev_trans_ops {
> > +     uk_9pdev_connect_t                      connect;
> > +     uk_9pdev_disconnect_t                   disconnect;
> > +     uk_9pdev_request_t                      request;
> > +};
> > +
> > +/**
> > + * @internal
> > + * A structure used for 9p requests' management.
> > + */
> > +struct uk_9pdev_req_mgmt {
> > +     /* Spinlock protecting this data. */
> > +     spinlock_t                      spinlock;
> > +     /* Bitmap of available tags. */
> > +     unsigned long                   
> > tag_bm[UK_BITS_TO_LONGS(UK_9P_NUMTAGS)];
> > +     /* List of requests allocated and not yet removed. */
> > +     struct uk_list_head             req_list;
> > +};
> > +
> > +/**
> > + * @internal
> > + * 9PDEV transport state
> > + *
> > + * - CONNECTED: Default state after initialization and during normal 
> > operation.
> > + * - DISCONNECTING: After a uk_9pdev_disconnect() call.
> > + *   No requests are allowed anymore. When all live resources have been
> > + *   destroyed, the 9pdev will free itself.
> > + */
> > +enum uk_9pdev_trans_state {
> > +     UK_9PDEV_CONNECTED,
> > +     UK_9PDEV_DISCONNECTING
> > +};
> > +
> > +/**
> > + * 9PDEV
> > + * A structure used to interact with a 9P device.
> > + */
> > +struct uk_9pdev {
> > +     /* Underlying transport operations. */
> > +     const struct uk_9pdev_trans_ops *ops;
> > +     /* Allocator used by this device. */
> > +     struct uk_alloc                 *a; /* See uk_9pdev_connect(). */
> > +     /* Transport state. */
> > +     enum uk_9pdev_trans_state       state;
> > +     /* Maximum size of a message. */
> > +     uint32_t                        msize;
> > +     /* Transport-allocated data. */
> > +     void                            *priv;
> > +     /* @internal Request management. */
> > +     struct uk_9pdev_req_mgmt        _req_mgmt;
> > +#if CONFIG_LIBUKSCHED
> > +     /*
> > +      * Slept on by threads waiting for their turn for enough space to send
> > +      * the request.
> > +      */
> > +     struct uk_waitq                 xmit_wq;
> > +#endif
> > +};
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* __UK_9PDEV_CORE__ */
> > diff --git a/lib/uk9p/include/uk/9pdev_trans.h 
> > b/lib/uk9p/include/uk/9pdev_trans.h
> > index 7b6645fb5e84..d73af3e3beac 100644
> > --- a/lib/uk9p/include/uk/9pdev_trans.h
> > +++ b/lib/uk9p/include/uk/9pdev_trans.h
> > @@ -37,6 +37,7 @@
> >
> >   #include <stdbool.h>
> >   #include <uk/config.h>
> > +#include <uk/9pdev_core.h>
> >
> >   #ifdef __cplusplus
> >   extern "C" {
> > @@ -51,6 +52,8 @@ struct uk_9pdev_trans {
> >        * specific transport.
> >        */
> >       const char                              *name;
> > +     /* Supported operations. */
> > +     const struct uk_9pdev_trans_ops         *ops;
> >       /* Can the transport be used as a default one? */
> >       bool                                    is_default;
> >       /* Allocator used for devices which use this transport layer. */
> >

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.