From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kwankhede@nvidia.com, alex.williamson@redhat.com,
cohuck@redhat.com, tiwei.bie@intel.com,
maxime.coquelin@redhat.com, cunming.liang@intel.com,
zhihong.wang@intel.com, rob.miller@broadcom.com,
idos@mellanox.com, xiao.w.wang@intel.com,
haotian.wang@sifive.com
Subject: Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
Date: Tue, 10 Sep 2019 09:52:46 -0400 [thread overview]
Message-ID: <20190910094807-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <572ffc34-3081-8503-d3cc-192edc9b5311@redhat.com>
On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
>
> On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
> > > +#ifndef _LINUX_VIRTIO_MDEV_H
> > > +#define _LINUX_VIRTIO_MDEV_H
> > > +
> > > +#include <linux/interrupt.h>
> > > +#include <linux/vringh.h>
> > > +#include <uapi/linux/virtio_net.h>
> > > +
> > > +/*
> > > + * Ioctls
> > > + */
> > Pls add a bit more content here. It's redundant to state these
> > are ioctls. Much better to document what does each one do.
>
>
> Ok.
>
>
> >
> > > +
> > > +struct virtio_mdev_callback {
> > > + irqreturn_t (*callback)(void *);
> > > + void *private;
> > > +};
> > > +
> > > +#define VIRTIO_MDEV 0xAF
> > > +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> > > + struct virtio_mdev_callback)
> > > +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> > > + struct virtio_mdev_callback)
> > Function pointer in an ioctl parameter? How does this ever make sense?
>
>
> I admit this is hacky (casting).
>
>
> > And can't we use a couple of registers for this, and avoid ioctls?
>
>
> Yes, how about something like interrupt numbers for each virtqueue and
> config?
Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
>
> >
> > > +
> > > +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
> > > +
> > > +/*
> > > + * Control registers
> > > + */
> > > +
> > > +/* Magic value ("virt" string) - Read Only */
> > > +#define VIRTIO_MDEV_MAGIC_VALUE 0x000
> > > +
> > > +/* Virtio device version - Read Only */
> > > +#define VIRTIO_MDEV_VERSION 0x004
> > > +
> > > +/* Virtio device ID - Read Only */
> > > +#define VIRTIO_MDEV_DEVICE_ID 0x008
> > > +
> > > +/* Virtio vendor ID - Read Only */
> > > +#define VIRTIO_MDEV_VENDOR_ID 0x00c
> > > +
> > > +/* Bitmask of the features supported by the device (host)
> > > + * (32 bits per set) - Read Only */
> > > +#define VIRTIO_MDEV_DEVICE_FEATURES 0x010
> > > +
> > > +/* Device (host) features set selector - Write Only */
> > > +#define VIRTIO_MDEV_DEVICE_FEATURES_SEL 0x014
> > > +
> > > +/* Bitmask of features activated by the driver (guest)
> > > + * (32 bits per set) - Write Only */
> > > +#define VIRTIO_MDEV_DRIVER_FEATURES 0x020
> > > +
> > > +/* Activated features set selector - Write Only */
> > > +#define VIRTIO_MDEV_DRIVER_FEATURES_SEL 0x024
> > > +
> > > +/* Queue selector - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_SEL 0x030
> > > +
> > > +/* Maximum size of the currently selected queue - Read Only */
> > > +#define VIRTIO_MDEV_QUEUE_NUM_MAX 0x034
> > > +
> > > +/* Queue size for the currently selected queue - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_NUM 0x038
> > > +
> > > +/* Ready bit for the currently selected queue - Read Write */
> > > +#define VIRTIO_MDEV_QUEUE_READY 0x044
> > Is this same as started?
>
>
> Do you mean "status"?
I really meant "enabled", didn't remember the correct name.
As in: VIRTIO_PCI_COMMON_Q_ENABLE
>
> >
> > > +
> > > +/* Alignment of virtqueue - Read Only */
> > > +#define VIRTIO_MDEV_QUEUE_ALIGN 0x048
> > > +
> > > +/* Queue notifier - Write Only */
> > > +#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
> > > +
> > > +/* Device status register - Read Write */
> > > +#define VIRTIO_MDEV_STATUS 0x060
> > > +
> > > +/* Selected queue's Descriptor Table address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
> > > +#define VIRTIO_MDEV_QUEUE_DESC_HIGH 0x084
> > > +
> > > +/* Selected queue's Available Ring address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_AVAIL_LOW 0x090
> > > +#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
> > > +
> > > +/* Selected queue's Used Ring address, 64 bits in two halves */
> > > +#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
> > > +#define VIRTIO_MDEV_QUEUE_USED_HIGH 0x0a4
> > > +
> > > +/* Configuration atomicity value */
> > > +#define VIRTIO_MDEV_CONFIG_GENERATION 0x0fc
> > > +
> > > +/* The config space is defined by each driver as
> > > + * the per-driver configuration space - Read Write */
> > > +#define VIRTIO_MDEV_CONFIG 0x100
> > Mixing device and generic config space is what virtio pci did,
> > caused lots of problems with extensions.
> > It would be better to reserve much more space.
>
>
> I see, will do this.
>
> Thanks
>
>
> >
> >
> > > +
> > > +#endif
> > > +
> > > +
> > > +/* Ready bit for the currently selected queue - Read Write */
> > > --
> > > 2.19.1
next prev parent reply other threads:[~2019-09-10 13:52 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-10 8:19 [RFC PATCH 0/4] mdev based hardware virtio offloading support Jason Wang
2019-09-10 8:19 ` [RFC PATCH 1/4] vringh: fix copy direction of vringh_iov_push_kern() Jason Wang
2019-09-10 8:19 ` Jason Wang
2019-09-10 8:19 ` [RFC PATCH 2/4] mdev: introduce helper to set per device dma ops Jason Wang
2019-09-10 8:19 ` Jason Wang
2019-09-17 19:00 ` Alex Williamson
2019-09-18 5:58 ` Jason Wang
2019-09-18 5:58 ` Jason Wang
2019-09-17 19:00 ` Alex Williamson
2019-09-10 8:19 ` [RFC PATCH 3/4] virtio: introudce a mdev based transport Jason Wang
2019-09-10 8:19 ` Jason Wang
2019-09-10 10:01 ` Michael S. Tsirkin
2019-09-10 10:01 ` Michael S. Tsirkin
2019-09-10 13:13 ` Jason Wang
2019-09-10 13:13 ` Jason Wang
2019-09-10 13:52 ` Michael S. Tsirkin [this message]
2019-09-11 2:38 ` Jason Wang
2019-09-11 2:38 ` Jason Wang
2019-09-11 9:36 ` Michael S. Tsirkin
2019-09-11 9:36 ` Michael S. Tsirkin
2019-09-11 9:53 ` Jason Wang
2019-09-11 9:53 ` Jason Wang
2019-09-10 13:52 ` Michael S. Tsirkin
2019-09-11 1:47 ` Tiwei Bie
2019-09-11 1:47 ` Tiwei Bie
2019-09-11 2:52 ` Jason Wang
2019-09-11 3:08 ` Tiwei Bie
2019-09-11 3:08 ` Tiwei Bie
2019-09-11 2:52 ` Jason Wang
2019-09-10 8:19 ` [RFC PATCH 4/4] docs: Sample driver to demonstrate how to implement virtio-mdev framework Jason Wang
2019-09-10 8:19 ` Jason Wang
2019-09-10 10:15 ` Michael S. Tsirkin
2019-09-10 10:15 ` Michael S. Tsirkin
2019-09-10 13:22 ` Jason Wang
2019-09-10 13:22 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190910094807-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=cohuck@redhat.com \
--cc=cunming.liang@intel.com \
--cc=haotian.wang@sifive.com \
--cc=idos@mellanox.com \
--cc=jasowang@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.coquelin@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=rob.miller@broadcom.com \
--cc=tiwei.bie@intel.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xiao.w.wang@intel.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.