From: Jason Wang <jasowang@redhat.com>
To: "Michael S. Tsirkin" <mst@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 21:13:02 +0800 [thread overview]
Message-ID: <572ffc34-3081-8503-d3cc-192edc9b5311@redhat.com> (raw)
In-Reply-To: <20190910055744-mutt-send-email-mst@kernel.org>
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?
>
>> +
>> +#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"?
>
>> +
>> +/* 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:13 UTC|newest]
Thread overview: 18+ 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 ` [RFC PATCH 2/4] mdev: introduce helper to set per device dma ops Jason Wang
2019-09-17 19:00 ` Alex Williamson
2019-09-18 5:58 ` Jason Wang
2019-09-10 8:19 ` [RFC PATCH 3/4] virtio: introudce a mdev based transport Jason Wang
2019-09-10 10:01 ` Michael S. Tsirkin
2019-09-10 13:13 ` Jason Wang [this message]
2019-09-10 13:52 ` Michael S. Tsirkin
2019-09-11 2:38 ` Jason Wang
2019-09-11 9:36 ` Michael S. Tsirkin
2019-09-11 9:53 ` Jason Wang
2019-09-11 1:47 ` Tiwei Bie
2019-09-11 2:52 ` Jason Wang
2019-09-11 3:08 ` Tiwei Bie
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 10:15 ` Michael S. Tsirkin
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=572ffc34-3081-8503-d3cc-192edc9b5311@redhat.com \
--to=jasowang@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=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.coquelin@redhat.com \
--cc=mst@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 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).