* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node
From: Guenter Roeck @ 2018-05-29 22:19 UTC (permalink / raw)
To: Kevin Easton
Cc: kvm, Michael S. Tsirkin, netdev, syzkaller-bugs, linux-kernel,
virtualization
In-Reply-To: <20180427154502.GA22544@la.guarana.org>
On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton wrote:
> The struct vhost_msg within struct vhost_msg_node is copied to userspace,
> so it should be allocated with kzalloc() to ensure all structure padding
> is zeroed.
>
> Signed-off-by: Kevin Easton <kevin@guarana.org>
> Reported-by: syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com
Is this patch going anywhere ?
The patch fixes CVE-2018-1118. It would be useful to understand if and when
this problem is going to be fixed.
Thanks,
Guenter
> ---
> drivers/vhost/vhost.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index f3bd8e9..1b84dcff 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2339,7 +2339,7 @@ EXPORT_SYMBOL_GPL(vhost_disable_notify);
> /* Create a new message. */
> struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> {
> - struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> + struct vhost_msg_node *node = kzalloc(sizeof *node, GFP_KERNEL);
> if (!node)
> return NULL;
> node->vq = vq;
^ permalink raw reply
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node
From: Michael S. Tsirkin @ 2018-05-30 3:01 UTC (permalink / raw)
To: Guenter Roeck
Cc: Kevin Easton, kvm, netdev, syzkaller-bugs, linux-kernel,
virtualization
In-Reply-To: <20180529221908.GA22742@roeck-us.net>
On Tue, May 29, 2018 at 03:19:08PM -0700, Guenter Roeck wrote:
> On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton wrote:
> > The struct vhost_msg within struct vhost_msg_node is copied to userspace,
> > so it should be allocated with kzalloc() to ensure all structure padding
> > is zeroed.
> >
> > Signed-off-by: Kevin Easton <kevin@guarana.org>
> > Reported-by: syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com
>
> Is this patch going anywhere ?
>
> The patch fixes CVE-2018-1118. It would be useful to understand if and when
> this problem is going to be fixed.
>
> Thanks,
> Guenter
> > ---
> > drivers/vhost/vhost.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index f3bd8e9..1b84dcff 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2339,7 +2339,7 @@ EXPORT_SYMBOL_GPL(vhost_disable_notify);
> > /* Create a new message. */
> > struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> > {
> > - struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> > + struct vhost_msg_node *node = kzalloc(sizeof *node, GFP_KERNEL);
> > if (!node)
> > return NULL;
> > node->vq = vq;
As I pointed out, we don't need to init the whole structure. The proper
fix is thus (I think) below.
Could you use your testing infrastructure to confirm this fixes the issue?
Thanks!
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index f3bd8e941224..58d9aec90afb 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
if (!node)
return NULL;
+
+ /* Make sure all padding within the structure is initialized. */
+ memset(&node->msg, 0, sizeof node->msg);
node->vq = vq;
node->msg.type = type;
return node;
^ permalink raw reply related
* Re: [net] vhost: Use kzalloc() to allocate vhost_msg_node
From: Guenter Roeck @ 2018-05-30 3:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Easton, kvm, netdev, syzkaller-bugs, linux-kernel,
virtualization
In-Reply-To: <20180530055704-mutt-send-email-mst@kernel.org>
On 05/29/2018 08:01 PM, Michael S. Tsirkin wrote:
> On Tue, May 29, 2018 at 03:19:08PM -0700, Guenter Roeck wrote:
>> On Fri, Apr 27, 2018 at 11:45:02AM -0400, Kevin Easton wrote:
>>> The struct vhost_msg within struct vhost_msg_node is copied to userspace,
>>> so it should be allocated with kzalloc() to ensure all structure padding
>>> is zeroed.
>>>
>>> Signed-off-by: Kevin Easton <kevin@guarana.org>
>>> Reported-by: syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com
>>
>> Is this patch going anywhere ?
>>
>> The patch fixes CVE-2018-1118. It would be useful to understand if and when
>> this problem is going to be fixed.
>>
>> Thanks,
>> Guenter
>>> ---
>>> drivers/vhost/vhost.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>> index f3bd8e9..1b84dcff 100644
>>> --- a/drivers/vhost/vhost.c
>>> +++ b/drivers/vhost/vhost.c
>>> @@ -2339,7 +2339,7 @@ EXPORT_SYMBOL_GPL(vhost_disable_notify);
>>> /* Create a new message. */
>>> struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
>>> {
>>> - struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
>>> + struct vhost_msg_node *node = kzalloc(sizeof *node, GFP_KERNEL);
>>> if (!node)
>>> return NULL;
>>> node->vq = vq;
>
> As I pointed out, we don't need to init the whole structure. The proper
> fix is thus (I think) below.
>
> Could you use your testing infrastructure to confirm this fixes the issue?
>
Sorry, I don't have the means to test the fix.
Guenter
> Thanks!
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index f3bd8e941224..58d9aec90afb 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2342,6 +2342,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> if (!node)
> return NULL;
> +
> + /* Make sure all padding within the structure is initialized. */
> + memset(&node->msg, 0, sizeof node->msg);
> node->vq = vq;
> node->msg.type = type;
> return node;
>
^ permalink raw reply
* [PATCH] virtio_pci: support enabling VFs
From: Tiwei Bie @ 2018-05-30 8:55 UTC (permalink / raw)
To: mst, bhelgaas, virtualization, linux-kernel, virtio-dev,
linux-pci
Cc: alexander.h.duyck, mark.d.rustad, zhihong.wang
There is a new feature bit allocated in virtio spec to
support SR-IOV (Single Root I/O Virtualization):
https://github.com/oasis-tcs/virtio-spec/issues/11
This patch enables the support for this feature bit in
virtio driver.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
This patch depends on below proposal:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
This patch depends on below patch:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
include/uapi/linux/virtio_config.h | 7 ++++++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..023da80a7a3e 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
put_device(dev);
}
+static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
+{
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_device *vdev = &vp_dev->vdev;
+ int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
+
+ if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ return -EBUSY;
+
+ if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
+ return -EINVAL;
+
+ sriov_configure = pci_sriov_configure_simple;
+ if (sriov_configure == NULL)
+ return -ENOENT;
+
+ return sriov_configure(pci_dev, num_vfs);
+}
+
static struct pci_driver virtio_pci_driver = {
.name = "virtio-pci",
.id_table = virtio_pci_id_table,
@@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
#ifdef CONFIG_PM_SLEEP
.driver.pm = &virtio_pci_pm_ops,
#endif
+ .sriov_configure = virtio_pci_sriov_configure,
};
module_pci_driver(virtio_pci_driver);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 2555d80f6eec..07571daccfec 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
return features;
}
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Give virtio_pci a chance to accept features. */
+ vp_transport_features(vdev, features);
+
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
dev_err(&vdev->dev, "virtio: device uses modern interface "
"but does not have VIRTIO_F_VERSION_1\n");
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..b7c1f4e7d59e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 34
+#define VIRTIO_TRANSPORT_F_END 38
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,9 @@
* this is for compatibility with legacy systems.
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+
+/*
+ * Does the device support Single Root I/O Virtualization?
+ */
+#define VIRTIO_F_SR_IOV 37
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
--
2.17.0
^ permalink raw reply related
* Re: [RFC V5 PATCH 8/8] vhost: event suppression for packed ring
From: Wei Xu @ 2018-05-30 11:42 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, mst, netdev, linux-kernel, virtualization
In-Reply-To: <1527559830-8133-9-git-send-email-jasowang@redhat.com>
On Tue, May 29, 2018 at 10:10:30AM +0800, Jason Wang wrote:
> This patch introduces basic support for event suppression aka driver
> and device area.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/vhost.c | 191 ++++++++++++++++++++++++++++++++++++---
> drivers/vhost/vhost.h | 10 +-
> include/uapi/linux/virtio_ring.h | 19 ++++
> 3 files changed, 204 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a36e5ad2..112f680 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1112,10 +1112,15 @@ static int vq_access_ok_packed(struct vhost_virtqueue *vq, unsigned int num,
> struct vring_used __user *used)
> {
> struct vring_desc_packed *packed = (struct vring_desc_packed *)desc;
> + struct vring_packed_desc_event *driver_event =
> + (struct vring_packed_desc_event *)avail;
> + struct vring_packed_desc_event *device_event =
> + (struct vring_packed_desc_event *)used;
>
> - /* FIXME: check device area and driver area */
> return access_ok(VERIFY_READ, packed, num * sizeof(*packed)) &&
> - access_ok(VERIFY_WRITE, packed, num * sizeof(*packed));
> + access_ok(VERIFY_WRITE, packed, num * sizeof(*packed)) &&
> + access_ok(VERIFY_READ, driver_event, sizeof(*driver_event)) &&
> + access_ok(VERIFY_WRITE, device_event, sizeof(*device_event));
> }
>
> static int vq_access_ok_split(struct vhost_virtqueue *vq, unsigned int num,
> @@ -1190,14 +1195,27 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq,
> return true;
> }
>
> -int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
> +int vq_iotlb_prefetch_packed(struct vhost_virtqueue *vq)
> +{
> + int num = vq->num;
> +
> + return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
> + num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
> + iotlb_access_ok(vq, VHOST_ACCESS_WO, (u64)(uintptr_t)vq->desc,
> + num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
> + iotlb_access_ok(vq, VHOST_ACCESS_RO,
> + (u64)(uintptr_t)vq->driver_event,
> + sizeof(*vq->driver_event), VHOST_ADDR_AVAIL) &&
> + iotlb_access_ok(vq, VHOST_ACCESS_WO,
> + (u64)(uintptr_t)vq->device_event,
> + sizeof(*vq->device_event), VHOST_ADDR_USED);
> +}
> +
> +int vq_iotlb_prefetch_split(struct vhost_virtqueue *vq)
> {
> size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> unsigned int num = vq->num;
>
> - if (!vq->iotlb)
> - return 1;
> -
> return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
> num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
> iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->avail,
> @@ -1209,6 +1227,17 @@ int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
> num * sizeof(*vq->used->ring) + s,
> VHOST_ADDR_USED);
> }
> +
> +int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
> +{
> + if (!vq->iotlb)
> + return 1;
> +
> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
> + return vq_iotlb_prefetch_packed(vq);
> + else
> + return vq_iotlb_prefetch_split(vq);
> +}
> EXPORT_SYMBOL_GPL(vq_iotlb_prefetch);
>
> /* Can we log writes? */
> @@ -1730,6 +1759,50 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq)
> return 0;
> }
>
> +static int vhost_update_device_flags(struct vhost_virtqueue *vq,
> + __virtio16 device_flags)
> +{
> + void __user *flags;
> +
> + if (vhost_put_user(vq, device_flags, &vq->device_event->flags,
> + VHOST_ADDR_USED) < 0)
> + return -EFAULT;
> + if (unlikely(vq->log_used)) {
> + /* Make sure the flag is seen before log. */
> + smp_wmb();
> + /* Log used flag write. */
> + flags = &vq->device_event->flags;
> + log_write(vq->log_base, vq->log_addr +
> + (flags - (void __user *)vq->device_event),
> + sizeof(vq->device_event->flags));
> + if (vq->log_ctx)
> + eventfd_signal(vq->log_ctx, 1);
> + }
> + return 0;
> +}
> +
> +static int vhost_update_device_off_wrap(struct vhost_virtqueue *vq,
> + __virtio16 device_off_wrap)
> +{
> + void __user *off_wrap;
> +
> + if (vhost_put_user(vq, device_off_wrap, &vq->device_event->off_wrap,
> + VHOST_ADDR_USED) < 0)
> + return -EFAULT;
> + if (unlikely(vq->log_used)) {
> + /* Make sure the flag is seen before log. */
> + smp_wmb();
> + /* Log used flag write. */
> + off_wrap = &vq->device_event->off_wrap;
> + log_write(vq->log_base, vq->log_addr +
> + (off_wrap - (void __user *)vq->device_event),
> + sizeof(vq->device_event->off_wrap));
> + if (vq->log_ctx)
> + eventfd_signal(vq->log_ctx, 1);
> + }
> + return 0;
> +}
> +
> static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event)
> {
> if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
> @@ -2683,16 +2756,13 @@ int vhost_add_used_n(struct vhost_virtqueue *vq,
> }
> EXPORT_SYMBOL_GPL(vhost_add_used_n);
>
> -static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> +static bool vhost_notify_split(struct vhost_dev *dev,
> + struct vhost_virtqueue *vq)
> {
> __u16 old, new;
> __virtio16 event;
> bool v;
>
> - /* FIXME: check driver area */
> - if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
> - return true;
> -
> /* Flush out used index updates. This is paired
> * with the barrier that the Guest executes when enabling
> * interrupts. */
> @@ -2725,6 +2795,64 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> return vring_need_event(vhost16_to_cpu(vq, event), new, old);
> }
>
> +static bool vhost_notify_packed(struct vhost_dev *dev,
> + struct vhost_virtqueue *vq)
> +{
> + __virtio16 event_off_wrap, event_flags;
> + __u16 old, new, off_wrap;
> + bool v;
> +
> + /* Flush out used descriptors updates. This is paired
> + * with the barrier that the Guest executes when enabling
> + * interrupts.
> + */
> + smp_mb();
> +
> + if (vhost_get_avail(vq, event_flags,
> + &vq->driver_event->flags) < 0) {
> + vq_err(vq, "Failed to get driver desc_event_flags");
> + return true;
> + }
> +
> + if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX))
> + return event_flags !=
> + cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
> +
> + old = vq->signalled_used;
> + v = vq->signalled_used_valid;
> + new = vq->signalled_used = vq->last_used_idx;
> + vq->signalled_used_valid = true;
> +
> + if (event_flags != cpu_to_vhost16(vq, RING_EVENT_FLAGS_DESC))
> + return event_flags !=
> + cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
> +
> + /* Read desc event flags before event_off and event_wrap */
> + smp_rmb();
> +
> + if (vhost_get_avail(vq, event_off_wrap,
> + &vq->driver_event->off_wrap) < 0) {
> + vq_err(vq, "Failed to get driver desc_event_off/wrap");
> + return true;
> + }
> +
> + off_wrap = vhost16_to_cpu(vq, event_off_wrap);
> +
> + if (unlikely(!v))
> + return true;
> +
> + return vhost_vring_packed_need_event(vq, vq->used_wrap_counter,
> + off_wrap, new, old);
> +}
> +
> +static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> +{
> + if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
> + return vhost_notify_packed(dev, vq);
> + else
> + return vhost_notify_split(dev, vq);
> +}
> +
> /* This actually signals the guest, using eventfd. */
> void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> {
> @@ -2802,10 +2930,34 @@ static bool vhost_enable_notify_packed(struct vhost_dev *dev,
> struct vhost_virtqueue *vq)
> {
> struct vring_desc_packed *d = vq->desc_packed + vq->avail_idx;
> - __virtio16 flags;
> + __virtio16 flags = RING_EVENT_FLAGS_ENABLE;
> int ret;
>
> - /* FIXME: disable notification through device area */
> + if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
> + return false;
> + vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
'used_flags' was originally designed for 1.0, why should we pay attetion to it here?
Wei
> +
> + if (vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
> + __virtio16 off_wrap = cpu_to_vhost16(vq, vq->avail_idx |
> + vq->avail_wrap_counter << 15);
> +
> + ret = vhost_update_device_off_wrap(vq, off_wrap);
> + if (ret) {
> + vq_err(vq, "Failed to write to off warp at %p: %d\n",
> + &vq->device_event->off_wrap, ret);
> + return false;
> + }
> + /* Make sure off_wrap is wrote before flags */
> + smp_wmb();
> + flags = RING_EVENT_FLAGS_DESC;
> + }
> +
> + ret = vhost_update_device_flags(vq, flags);
> + if (ret) {
> + vq_err(vq, "Failed to enable notification at %p: %d\n",
> + &vq->device_event->flags, ret);
> + return false;
> + }
>
> /* They could have slipped one in as we were doing that: make
> * sure it's written, then check again. */
> @@ -2871,7 +3023,18 @@ EXPORT_SYMBOL_GPL(vhost_enable_notify);
> static void vhost_disable_notify_packed(struct vhost_dev *dev,
> struct vhost_virtqueue *vq)
> {
> - /* FIXME: disable notification through device area */
> + __virtio16 flags;
> + int r;
> +
> + if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
> + return;
> + vq->used_flags |= VRING_USED_F_NO_NOTIFY;
> +
> + flags = cpu_to_vhost16(vq, RING_EVENT_FLAGS_DISABLE);
> + r = vhost_update_device_flags(vq, flags);
> + if (r)
> + vq_err(vq, "Failed to enable notification at %p: %d\n",
> + &vq->device_event->flags, r);
> }
>
> static void vhost_disable_notify_split(struct vhost_dev *dev,
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 7543a46..b920582 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -96,8 +96,14 @@ struct vhost_virtqueue {
> struct vring_desc __user *desc;
> struct vring_desc_packed __user *desc_packed;
> };
> - struct vring_avail __user *avail;
> - struct vring_used __user *used;
> + union {
> + struct vring_avail __user *avail;
> + struct vring_packed_desc_event __user *driver_event;
> + };
> + union {
> + struct vring_used __user *used;
> + struct vring_packed_desc_event __user *device_event;
> + };
> const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
> struct file *kick;
> struct eventfd_ctx *call_ctx;
> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
> index e297580..71c7a46 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -75,6 +75,25 @@ struct vring_desc_packed {
> __virtio16 flags;
> };
>
> +/* Enable events */
> +#define RING_EVENT_FLAGS_ENABLE 0x0
> +/* Disable events */
> +#define RING_EVENT_FLAGS_DISABLE 0x1
> +/*
> + * Enable events for a specific descriptor
> + * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
> + * Only valid if VIRTIO_F_RING_EVENT_IDX has been negotiated.
> + */
> +#define RING_EVENT_FLAGS_DESC 0x2
> +/* The value 0x3 is reserved */
> +
> +struct vring_packed_desc_event {
> + /* Descriptor Ring Change Event Offset and Wrap Counter */
> + __virtio16 off_wrap;
> + /* Descriptor Ring Change Event Flags */
> + __virtio16 flags;
> +};
> +
> /* Virtio ring descriptors: 16 bytes. These can chain together via "next". */
> struct vring_desc {
> /* Address (guest-physical). */
> --
> 2.7.4
>
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Stefan Hajnoczi @ 2018-05-30 12:29 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, mst, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
In-Reply-To: <20180530085521.26583-1-tiwei.bie@intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 1073 bytes --]
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
Looks good and matches the virtio spec change, but I'm not familiar with
PCI SR-IOV details, so not a full Reviewed-by.
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio_pci: support enabling VFs
From: Michael S. Tsirkin @ 2018-05-30 12:41 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
In-Reply-To: <20180530085521.26583-1-tiwei.bie@intel.com>
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply
* Re: [PATCH] virtio_ring: switch to dma_XX barriers for rpmsg
From: Michael S. Tsirkin @ 2018-05-30 14:10 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Ohad Ben-Cohen, linux-remoteproc, linux-kernel, virtualization,
Bjorn Andersson
In-Reply-To: <524fb400-2325-f60b-7e03-15be01888afc@redhat.com>
On Thu, Apr 19, 2018 at 07:48:24PM +0200, Paolo Bonzini wrote:
> On 19/04/2018 19:46, Michael S. Tsirkin wrote:
> >> This should be okay, but I wonder if there should be a virtio_wmb(...)
> >> or an "if (weak_barriers) wmb()" before the "writel" in vm_notify
> >> (drivers/virtio/virtio_mmio.c).
> >>
> >> Thanks,
> >>
> >> Paolo
> > That one uses weak barriers AFAIK.
> >
> > IIUC you mean rproc_virtio_notify.
> >
> > I suspect it works because specific kick callbacks have a barrier internally.
>
> Yes, that one. At least keystone_rproc_kick doesn't seem to have a barrier.
>
> Paolo
Any feedback from rproc maintainers?
--
MST
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Rustad, Mark D @ 2018-05-30 16:03 UTC (permalink / raw)
To: Bie, Tiwei
Cc: Duyck, Alexander H, virtio-dev@lists.oasis-open.org,
Michael S. Tsirkin, linux-pci@vger.kernel.org, LKML,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
Bjorn Helgaas
In-Reply-To: <20180530085521.26583-1-tiwei.bie@intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 1671 bytes --]
On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
<snip>
> diff --git a/include/uapi/linux/virtio_config.h
> b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
There is a value in the comment directly before this that should
be updated as well to be consistent with the new value for
VIRTIO_TRANSPORT_F_END below.
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
<snip>
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Michael S. Tsirkin @ 2018-05-30 16:09 UTC (permalink / raw)
To: Rustad, Mark D
Cc: Duyck, Alexander H, virtio-dev@lists.oasis-open.org,
linux-pci@vger.kernel.org, LKML,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
Bjorn Helgaas
In-Reply-To: <D7004E41-C23D-48B7-94A4-ACBBA06B021D@intel.com>
On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
>
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
>
> <snip>
>
> > diff --git a/include/uapi/linux/virtio_config.h
> > b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
>
> There is a value in the comment directly before this that should
> be updated as well to be consistent with the new value for
> VIRTIO_TRANSPORT_F_END below.
It hasn't been updated to 34 yet.
I suggest a separate patch to replace the numbers with
VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
in the comment.
Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
virtio_pci etc." as well.
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
>
> <snip>
>
> --
> Mark Rustad, Networking Division, Intel Corporation
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Alexander Duyck @ 2018-05-30 16:10 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, virtio-dev, Michael S. Tsirkin, linux-pci,
LKML, virtualization, zhihong.wang, Bjorn Helgaas, Rustad, Mark D
In-Reply-To: <20180530085521.26583-1-tiwei.bie@intel.com>
On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
So from a quick glance it looks like we are leaving SR-IOV enabled if
the driver is removed. Do we want to have that behavior or should we
be adding the code to disable SR-IOV and free the VFs on driver
removal?
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
^ permalink raw reply
* Re: [PATCH] virtio_pci: support enabling VFs
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
In-Reply-To: <20180530085521.26583-1-tiwei.bie@intel.com>
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
BTW what is all this trickery in aid of?
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
zhihong.wang, Bjorn Helgaas, Rustad, Mark D
In-Reply-To: <CAKgT0UcRZd5ajpTyC3KMvNFACVAb6D8A9Tss3m0r+UdamvVeRA@mail.gmail.com>
On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> So from a quick glance it looks like we are leaving SR-IOV enabled if
> the driver is removed. Do we want to have that behavior or should we
> be adding the code to disable SR-IOV and free the VFs on driver
> removal?
Could pci core handle it for us somehow?
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 48d4d1cf1cb6..023da80a7a3e 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> > put_device(dev);
> > }
> >
> > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> > +{
> > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > + struct virtio_device *vdev = &vp_dev->vdev;
> > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > +
> > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > + return -EBUSY;
> > +
> > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > + return -EINVAL;
> > +
> > + sriov_configure = pci_sriov_configure_simple;
> > + if (sriov_configure == NULL)
> > + return -ENOENT;
> > +
> > + return sriov_configure(pci_dev, num_vfs);
> > +}
> > +
> > static struct pci_driver virtio_pci_driver = {
> > .name = "virtio-pci",
> > .id_table = virtio_pci_id_table,
> > @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> > #ifdef CONFIG_PM_SLEEP
> > .driver.pm = &virtio_pci_pm_ops,
> > #endif
> > + .sriov_configure = virtio_pci_sriov_configure,
> > };
> >
> > module_pci_driver(virtio_pci_driver);
> > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > index 2555d80f6eec..07571daccfec 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> > return features;
> > }
> >
> > +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct pci_dev *pci_dev = vp_dev->pci_dev;
> > +
> > + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > +}
> > +
> > /* virtio config->finalize_features() implementation */
> > static int vp_finalize_features(struct virtio_device *vdev)
> > {
> > struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + u64 features = vdev->features;
> >
> > /* Give virtio_ring a chance to accept features. */
> > vring_transport_features(vdev);
> >
> > + /* Give virtio_pci a chance to accept features. */
> > + vp_transport_features(vdev, features);
> > +
> > if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> > dev_err(&vdev->dev, "virtio: device uses modern interface "
> > "but does not have VIRTIO_F_VERSION_1\n");
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,9 @@
> > * this is for compatibility with legacy systems.
> > */
> > #define VIRTIO_F_IOMMU_PLATFORM 33
> > +
> > +/*
> > + * Does the device support Single Root I/O Virtualization?
> > + */
> > +#define VIRTIO_F_SR_IOV 37
> > #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > --
> > 2.17.0
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> >
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Duyck, Alexander H @ 2018-05-30 16:26 UTC (permalink / raw)
To: mst@redhat.com, alexander.duyck@gmail.com
Cc: virtio-dev@lists.oasis-open.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
bhelgaas@google.com, Rustad, Mark D
In-Reply-To: <20180530192215-mutt-send-email-mst@kernel.org>
On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> >
> > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > the driver is removed. Do we want to have that behavior or should we
> > be adding the code to disable SR-IOV and free the VFs on driver
> > removal?
>
> Could pci core handle it for us somehow?
Maybe, but it would require changes to the pci core to do it.
The problem is some drivers want to leave the VFs there since the PF
doesn't really do anything, or they have the option of essentially
putting the VFs into a standby state when the PF is gone.
My main concern is do we care if VFs are allocated and then somebody
removes the driver and binds a different driver to the interface? If
not then this code and be left as is, but I just wanted to be certain
since I know this isn't just enabling SR-IOV we are having to do a
number of other checks against the virtio device.
^ permalink raw reply
* Re: [PATCH] virtio_pci: support enabling VFs
From: Rustad, Mark D @ 2018-05-30 16:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev@lists.oasis-open.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
Bjorn Helgaas
In-Reply-To: <20180530192010-mutt-send-email-mst@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 996 bytes --]
On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> num_vfs)
>> +{
>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> + struct virtio_device *vdev = &vp_dev->vdev;
>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> +
>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> + return -EBUSY;
>> +
>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> + return -EINVAL;
>> +
>> + sriov_configure = pci_sriov_configure_simple;
>> + if (sriov_configure == NULL)
>> + return -ENOENT;
>
> BTW what is all this trickery in aid of?
When SR-IOV support is not compiled into the kernel,
pci_sriov_configure_simple is #defined as NULL. This allows it to compile
in that case, even though there is utterly no way for it to be called in
that case. It is an alternative to #ifs in the code.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio_pci: support enabling VFs
From: Duyck, Alexander H @ 2018-05-30 16:54 UTC (permalink / raw)
To: Rustad, Mark D, mst@redhat.com
Cc: virtio-dev@lists.oasis-open.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
bhelgaas@google.com
In-Reply-To: <414C18B1-30FA-4AC0-B47D-F0FBF9832737@intel.com>
On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > num_vfs)
> > > +{
> > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > +
> > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > + return -EBUSY;
> > > +
> > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > + return -EINVAL;
> > > +
> > > + sriov_configure = pci_sriov_configure_simple;
> > > + if (sriov_configure == NULL)
> > > + return -ENOENT;
> >
> > BTW what is all this trickery in aid of?
>
> When SR-IOV support is not compiled into the kernel,
> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> in that case, even though there is utterly no way for it to be called in
> that case. It is an alternative to #ifs in the code.
Why even have the call though? I would wrap all of this in an #ifdef
and strip it out since you couldn't support SR-IOV if it isn't present
in the kernel anyway.
^ permalink raw reply
* Re: [PATCH] virtio_pci: support enabling VFs
From: Rustad, Mark D @ 2018-05-30 17:11 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: virtio-dev@lists.oasis-open.org, mst@redhat.com,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
bhelgaas@google.com
In-Reply-To: <1527699273.29907.2.camel@intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 1571 bytes --]
On May 30, 2018, at 9:54 AM, Duyck, Alexander H
<alexander.h.duyck@intel.com> wrote:
> On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>>>> num_vfs)
>>>> +{
>>>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>>>> + struct virtio_device *vdev = &vp_dev->vdev;
>>>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>>>> +
>>>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>>>> + return -EBUSY;
>>>> +
>>>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>>>> + return -EINVAL;
>>>> +
>>>> + sriov_configure = pci_sriov_configure_simple;
>>>> + if (sriov_configure == NULL)
>>>> + return -ENOENT;
>>>
>>> BTW what is all this trickery in aid of?
>>
>> When SR-IOV support is not compiled into the kernel,
>> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> in that case, even though there is utterly no way for it to be called in
>> that case. It is an alternative to #ifs in the code.
>
> Why even have the call though? I would wrap all of this in an #ifdef
> and strip it out since you couldn't support SR-IOV if it isn't present
> in the kernel anyway.
I am inclined to agree. In this case, the presence of #ifdefs I think would
be clearer. As written, someone will want to get rid of the pointer only to
create a build problem when SR-IOV is not configured.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Michael S. Tsirkin @ 2018-05-30 17:15 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: virtio-dev@lists.oasis-open.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, alexander.duyck@gmail.com,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
bhelgaas@google.com, Rustad, Mark D
In-Reply-To: <1527697588.16245.136.camel@intel.com>
On Wed, May 30, 2018 at 04:26:30PM +0000, Duyck, Alexander H wrote:
> On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > >
> > > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > > the driver is removed. Do we want to have that behavior or should we
> > > be adding the code to disable SR-IOV and free the VFs on driver
> > > removal?
> >
> > Could pci core handle it for us somehow?
>
> Maybe, but it would require changes to the pci core to do it.
>
> The problem is some drivers want to leave the VFs there since the PF
> doesn't really do anything, or they have the option of essentially
> putting the VFs into a standby state when the PF is gone.
>
> My main concern is do we care if VFs are allocated and then somebody
> removes the driver and binds a different driver to the interface? If
> not then this code and be left as is, but I just wanted to be certain
> since I know this isn't just enabling SR-IOV we are having to do a
> number of other checks against the virtio device.
Well the spec says features have to be negotiated, and since we reset
the device when we unbind from it I think it's a given we should keep a
driver bound to the PF.
IOW until we are sure we need the capability to keep it enabled, let's
disable it to be safe.
--
MST
^ permalink raw reply
* Re: [PATCH net] vhost_net: flush batched heads before trying to busy polling
From: David Miller @ 2018-05-30 17:31 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <1527574699-13047-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 29 May 2018 14:18:19 +0800
> After commit e2b3b35eb989 ("vhost_net: batch used ring update in rx"),
> we tend to batch updating used heads. But it doesn't flush batched
> heads before trying to do busy polling, this will cause vhost to wait
> for guest TX which waits for the used RX. Fixing by flush batched
> heads before busy loop.
>
> 1 byte TCP_RR performance recovers from 13107.83 to 50402.65.
>
> Fixes: e2b3b35eb989 ("vhost_net: batch used ring update in rx")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v2 00/13] drm: Eliminate plane->fb/crtc usage for atomic drivers
From: Sinclair Yeh @ 2018-05-30 17:41 UTC (permalink / raw)
To: Ville Syrjala
Cc: David Airlie, Daniel Vetter, dri-devel, virtualization,
Eric Anholt, David (ChunMing) Zhou, Thomas Hellstrom,
Joonyoung Shim, Kyungmin Park, amd-gfx, VMware Graphics,
Harry Wentland, linux-arm-msm, intel-gfx, Inki Dae, Deepak Rawat,
Seung-Woo Kim, Rob Clark, Alex Deucher, freedreno,
Christian König
In-Reply-To: <20180525185045.29689-1-ville.syrjala@linux.intel.com>
Thanks Ville.
This series: Reviewed-by: Sinclair Yeh <syeh@vmware.com>
On Fri, May 25, 2018 at 09:50:32PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Here are again the last (?) bits of eliminating the plane->fb/crtc
> usage for atomic drivers. I've pushed everything else (thanks to
> everyone who reviewed them).
>
> Deepak said he'd tested the vmwgfx stuff, so I think it should be
> safe to land. Just missing a bit of review...
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Airlie <airlied@linux.ie>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Deepak Rawat <drawat@vmware.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: freedreno@lists.freedesktop.org
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>
> Ville Syrjälä (13):
> drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
> drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
> drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
> drm/vmwgfx: Stop updating plane->fb
> drm/vmwgfx: Stop using plane->fb in atomic_enable()
> drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc
> drm/amdgpu/dc: Stop updating plane->fb
> drm/i915: Stop updating plane->fb/crtc
> drm/exynos: Stop updating plane->crtc
> drm/msm: Stop updating plane->fb/crtc
> drm/virtio: Stop updating plane->crtc
> drm/vc4: Stop updating plane->fb/crtc
> drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
>
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 -
> drivers/gpu/drm/drm_atomic.c | 55 +++--------------------
> drivers/gpu/drm/drm_atomic_helper.c | 15 +------
> drivers/gpu/drm/drm_crtc.c | 8 +++-
> drivers/gpu/drm/drm_fb_helper.c | 7 ---
> drivers/gpu/drm/drm_framebuffer.c | 5 ---
> drivers/gpu/drm/drm_plane.c | 14 +++---
> drivers/gpu/drm/drm_plane_helper.c | 4 +-
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 -
> drivers/gpu/drm/i915/intel_atomic_plane.c | 12 -----
> drivers/gpu/drm/i915/intel_display.c | 7 ++-
> drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 -
> drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 -
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 -
> drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 -
> drivers/gpu/drm/vc4/vc4_crtc.c | 3 --
> drivers/gpu/drm/virtio/virtgpu_display.c | 2 -
> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 24 ----------
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 24 +++++++---
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 -
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 5 +--
> include/drm/drm_atomic.h | 3 --
> 22 files changed, 46 insertions(+), 154 deletions(-)
>
> --
> 2.16.1
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: fix error return code in virtnet_probe()
From: Jason Wang @ 2018-05-31 2:05 UTC (permalink / raw)
To: Wei Yongjun, Michael S. Tsirkin, Sridhar Samudrala
Cc: netdev, kernel-janitors, virtualization
In-Reply-To: <1527732307-145609-1-git-send-email-weiyongjun1@huawei.com>
On 2018年05月31日 10:05, Wei Yongjun wrote:
> Fix to return a negative error code from the failover create fail error
> handling case instead of 0, as done elsewhere in this function.
>
> Fixes: ba5e4426e80e ("virtio_net: Extend virtio to use VF datapath when available")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/net/virtio_net.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8f08a3e..2d55e2a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2935,8 +2935,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
> vi->failover = net_failover_create(vi->dev);
> - if (IS_ERR(vi->failover))
> + if (IS_ERR(vi->failover)) {
> + err = PTR_ERR(vi->failover);
> goto free_vqs;
> + }
> }
>
> err = register_netdev(dev);
>
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
From: Stephen Hemminger @ 2018-05-31 2:06 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, anjali.singhai, aaron.f.brown, davem
In-Reply-To: <1527180917-39737-3-git-send-email-sridhar.samudrala@intel.com>
On Thu, 24 May 2018 09:55:14 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> Use the registration/notification framework supported by the generic
> failover infrastructure.
>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Why was this merged? It was never signed off by any of the netvsc maintainers,
and there were still issues unresolved.
There are also namespaces issues I am fixing and this breaks them.
Will start my patch set with a revert for this. Sorry
^ permalink raw reply
* Re: [PATCH net-next v12 1/5] net: Introduce generic failover module
From: Stephen Hemminger @ 2018-05-31 2:52 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, anjali.singhai, aaron.f.brown, davem
In-Reply-To: <00d34f67-f26f-0b20-af3f-2add24ae8a5c@intel.com>
On Fri, 25 May 2018 16:06:58 -0700
"Samudrala, Sridhar" <sridhar.samudrala@intel.com> wrote:
> On 5/25/2018 3:38 PM, Stephen Hemminger wrote:
> > On Thu, 24 May 2018 09:55:13 -0700
> > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> >
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 03ed492c4e14..0f4ba52b641d 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -1421,6 +1421,8 @@ struct net_device_ops {
> >> * entity (i.e. the master device for bridged veth)
> >> * @IFF_MACSEC: device is a MACsec device
> >> * @IFF_NO_RX_HANDLER: device doesn't support the rx_handler hook
> >> + * @IFF_FAILOVER: device is a failover master device
> >> + * @IFF_FAILOVER_SLAVE: device is lower dev of a failover master device
> >> */
> >> enum netdev_priv_flags {
> >> IFF_802_1Q_VLAN = 1<<0,
> >> @@ -1450,6 +1452,8 @@ enum netdev_priv_flags {
> >> IFF_PHONY_HEADROOM = 1<<24,
> >> IFF_MACSEC = 1<<25,
> >> IFF_NO_RX_HANDLER = 1<<26,
> >> + IFF_FAILOVER = 1<<27,
> >> + IFF_FAILOVER_SLAVE = 1<<28,
> >> };
> > Why is FAILOVER any different than other master/slave relationships.
> > I don't think you need to take up precious netdev flag bits for this.
>
> These are netdev priv flags.
> Jiri says that IFF_MASTER/IFF_SLAVE are bonding specific flags and cannot be used
> with other failover mechanisms. Team also doesn't use this flags and it has its own
> priv_flags.
>
This change breaks userspace.
We already have worked with partners to ignore devices marked as IFF_SLAVE,
and IFF_SLAVE is visible to user space API's.
NAK
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
From: Tiwei Bie @ 2018-05-31 2:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev@lists.oasis-open.org,
linux-pci@vger.kernel.org, LKML,
virtualization@lists.linux-foundation.org, Wang, Zhihong,
Bjorn Helgaas, Rustad, Mark D
In-Reply-To: <20180530190819-mutt-send-email-mst@kernel.org>
On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> >
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > ---
> > > This patch depends on below proposal:
> > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > >
> > > This patch depends on below patch:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > >
> > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > >
> >
> > <snip>
> >
> > > diff --git a/include/uapi/linux/virtio_config.h
> > > b/include/uapi/linux/virtio_config.h
> > > index 308e2096291f..b7c1f4e7d59e 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -49,7 +49,7 @@
> >
> > There is a value in the comment directly before this that should
> > be updated as well to be consistent with the new value for
> > VIRTIO_TRANSPORT_F_END below.
>
> It hasn't been updated to 34 yet.
> I suggest a separate patch to replace the numbers with
> VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> in the comment.
> Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> virtio_pci etc." as well.
Good idea! Thanks for the suggestion! I'll do it.
Best regards,
Tiwei Bie
>
> > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > * bits. */
> > > #define VIRTIO_TRANSPORT_F_START 28
> > > -#define VIRTIO_TRANSPORT_F_END 34
> > > +#define VIRTIO_TRANSPORT_F_END 38
> > >
> > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > /* Do we get callbacks when the ring is completely used, even if we've
> >
> > <snip>
> >
> > --
> > Mark Rustad, Networking Division, Intel Corporation
>
>
^ permalink raw reply
* Re: [PATCH net-next v12 1/5] net: Introduce generic failover module
From: Samudrala, Sridhar @ 2018-05-31 2:58 UTC (permalink / raw)
To: Stephen Hemminger
Cc: alexander.h.duyck, virtio-dev, jiri, mst, kubakici, netdev,
virtualization, loseweigh, anjali.singhai, aaron.f.brown, davem
In-Reply-To: <20180530225259.2cf3f7be@shemminger-XPS-13-9360>
On 5/30/2018 7:52 PM, Stephen Hemminger wrote:
> On Fri, 25 May 2018 16:06:58 -0700
> "Samudrala, Sridhar" <sridhar.samudrala@intel.com> wrote:
>
>> On 5/25/2018 3:38 PM, Stephen Hemminger wrote:
>>> On Thu, 24 May 2018 09:55:13 -0700
>>> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>>>
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 03ed492c4e14..0f4ba52b641d 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -1421,6 +1421,8 @@ struct net_device_ops {
>>>> * entity (i.e. the master device for bridged veth)
>>>> * @IFF_MACSEC: device is a MACsec device
>>>> * @IFF_NO_RX_HANDLER: device doesn't support the rx_handler hook
>>>> + * @IFF_FAILOVER: device is a failover master device
>>>> + * @IFF_FAILOVER_SLAVE: device is lower dev of a failover master device
>>>> */
>>>> enum netdev_priv_flags {
>>>> IFF_802_1Q_VLAN = 1<<0,
>>>> @@ -1450,6 +1452,8 @@ enum netdev_priv_flags {
>>>> IFF_PHONY_HEADROOM = 1<<24,
>>>> IFF_MACSEC = 1<<25,
>>>> IFF_NO_RX_HANDLER = 1<<26,
>>>> + IFF_FAILOVER = 1<<27,
>>>> + IFF_FAILOVER_SLAVE = 1<<28,
>>>> };
>>> Why is FAILOVER any different than other master/slave relationships.
>>> I don't think you need to take up precious netdev flag bits for this.
>> These are netdev priv flags.
>> Jiri says that IFF_MASTER/IFF_SLAVE are bonding specific flags and cannot be used
>> with other failover mechanisms. Team also doesn't use this flags and it has its own
>> priv_flags.
>>
> This change breaks userspace.
> We already have worked with partners to ignore devices marked as IFF_SLAVE,
> and IFF_SLAVE is visible to user space API's.
I specifically made sure not to remove IFF_SLAVE in the netvsc patch.
>
> NAK
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox