date:20210209

Re: [RFC 09/10] vhost: Route guest->host notification through shadow virtqueue

2021-02-09 Thread Jason Wang



On 2021/2/9 下午11:02, Eugenio Perez Martin wrote:

On Thu, Feb 4, 2021 at 4:27 AM Jason Wang  wrote:


On 2021/2/2 下午6:08, Eugenio Perez Martin wrote:

On Mon, Feb 1, 2021 at 7:29 AM Jason Wang  wrote:

On 2021/1/30 上午4:54, Eugenio Pérez wrote:

Shadow virtqueue notifications forwarding is disabled when vhost_dev
stops.

Signed-off-by: Eugenio Pérez 
---
hw/virtio/vhost-shadow-virtqueue.h |   5 ++
include/hw/virtio/vhost.h  |   4 +
hw/virtio/vhost-shadow-virtqueue.c | 123 +-
hw/virtio/vhost.c  | 135 -
4 files changed, 264 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
b/hw/virtio/vhost-shadow-virtqueue.h
index 6cc18d6acb..466f8ae595 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -17,6 +17,11 @@

typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;

+bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
+   VhostShadowVirtqueue *svq);
+void vhost_shadow_vq_stop_rcu(struct vhost_dev *dev,
+  VhostShadowVirtqueue *svq);
+
VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx);

void vhost_shadow_vq_free(VhostShadowVirtqueue *vq);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 2be782cefd..732a4b2a2b 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -55,6 +55,8 @@ struct vhost_iommu {
QLIST_ENTRY(vhost_iommu) iommu_next;
};

+typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+
typedef struct VhostDevConfigOps {
/* Vhost device config space changed callback
 */
@@ -83,7 +85,9 @@ struct vhost_dev {
uint64_t backend_cap;
bool started;
bool log_enabled;
+bool sw_lm_enabled;
uint64_t log_size;
+VhostShadowVirtqueue **shadow_vqs;
Error *migration_blocker;
const VhostOps *vhost_ops;
void *opaque;
diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
b/hw/virtio/vhost-shadow-virtqueue.c
index c0c967a7c5..908c36c66d 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -8,15 +8,129 @@
 */

#include "hw/virtio/vhost-shadow-virtqueue.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/virtio-access.h"
+
+#include "standard-headers/linux/vhost_types.h"
+#include "standard-headers/linux/virtio_ring.h"

#include "qemu/error-report.h"
-#include "qemu/event_notifier.h"
+#include "qemu/main-loop.h"

typedef struct VhostShadowVirtqueue {
EventNotifier kick_notifier;
EventNotifier call_notifier;
+const struct vhost_virtqueue *hvq;
+VirtIODevice *vdev;
+VirtQueue *vq;
} VhostShadowVirtqueue;

So instead of doing things at virtio level, how about do the shadow
stuffs at vhost level?

It works like:

virtio -> [shadow vhost backend] -> vhost backend

Then the QMP is used to plug the shadow vhost backend in the middle or not.

It looks kind of easier since we don't need to deal with virtqueue
handlers etc.. Instead, we just need to deal with eventfd stuffs:

When shadow vhost mode is enabled, we just intercept the host_notifiers
and guest_notifiers. When it was disabled, we just pass the host/guest
notifiers to the real vhost backends?


Hi Jason.

Sure we can try that model, but it seems to me that it comes with a
different set of problems.

For example, there are code in vhost.c that checks if implementations
are available in vhost_ops, like:

if (dev->vhost_ops->vhost_vq_get_addr) {
  r = dev->vhost_ops->vhost_vq_get_addr(dev, , vq);
  ...
}

I can count 14 of these, checking:

dev->vhost_ops->vhost_backend_can_merge
dev->vhost_ops->vhost_backend_mem_section_filter
dev->vhost_ops->vhost_force_iommu
dev->vhost_ops->vhost_requires_shm_log
dev->vhost_ops->vhost_set_backend_cap
dev->vhost_ops->vhost_set_vring_busyloop_timeout
dev->vhost_ops->vhost_vq_get_addr
hdev->vhost_ops->vhost_dev_start
hdev->vhost_ops->vhost_get_config
hdev->vhost_ops->vhost_get_inflight_fd
hdev->vhost_ops->vhost_net_set_backend
hdev->vhost_ops->vhost_set_config
hdev->vhost_ops->vhost_set_inflight_fd
hdev->vhost_ops->vhost_set_iotlb_callback

So we should Implement all of the vhost_ops callbacks, forwarding them
to actual vhost_backed, and delete conditionally these ones? In other
words, dynamically generate the new shadow vq vhost_ops? If a new
callback is added to any vhost backend in the future, do we have to
force the adding / checking for NULL in shadow backend vhost_ops?
Would this be a good moment to check if all backends implement these
and delete the checks?


I think it won't be easy if we want to support all kinds of vhost
backends from the start. So we can go with vhost-vdpa one first.

Actually how it work might be something like (no need to switch
vhost_ops, we can do everything silently in the ops)

1) when device to switch to shadow vq

Re: [RFC 05/10] vhost: Add vhost_dev_from_virtio

2021-02-09 Thread Jason Wang



On 2021/2/9 下午11:35, Eugenio Perez Martin wrote:

On Fri, Feb 5, 2021 at 4:52 AM Jason Wang  wrote:


On 2021/2/4 下午5:25, Eugenio Perez Martin wrote:

On Thu, Feb 4, 2021 at 4:14 AM Jason Wang  wrote:

On 2021/2/2 下午6:17, Eugenio Perez Martin wrote:

On Tue, Feb 2, 2021 at 4:31 AM Jason Wang  wrote:

On 2021/2/1 下午4:28, Eugenio Perez Martin wrote:

On Mon, Feb 1, 2021 at 7:13 AM Jason Wang  wrote:

On 2021/1/30 上午4:54, Eugenio Pérez wrote:

Signed-off-by: Eugenio Pérez 
---
  include/hw/virtio/vhost.h |  1 +
  hw/virtio/vhost.c | 17 +
  2 files changed, 18 insertions(+)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 4a8bc75415..fca076e3f0 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -123,6 +123,7 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const 
int *feature_bits,
  void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
  uint64_t features);
  bool vhost_has_free_slot(void);
+struct vhost_dev *vhost_dev_from_virtio(const VirtIODevice *vdev);

  int vhost_net_set_backend(struct vhost_dev *hdev,
struct vhost_vring_file *file);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 28c7d78172..8683d507f5 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -61,6 +61,23 @@ bool vhost_has_free_slot(void)
  return slots_limit > used_memslots;
  }

+/*
+ * Get the vhost device associated to a VirtIO device.
+ */
+struct vhost_dev *vhost_dev_from_virtio(const VirtIODevice *vdev)
+{
+struct vhost_dev *hdev;
+
+QLIST_FOREACH(hdev, _devices, entry) {
+if (hdev->vdev == vdev) {
+return hdev;
+}
+}
+
+assert(hdev);
+return NULL;
+}

I'm not sure this can work in the case of multiqueue. E.g vhost-net
multiqueue is a N:1 mapping between vhost devics and virtio devices.

Thanks


Right. We could add an "vdev vq index" parameter to the function in
this case, but I guess the most reliable way to do this is to add a
vhost_opaque value to VirtQueue, as Stefan proposed in previous RFC.

So the question still, it looks like it's easier to hide the shadow
virtqueue stuffs at vhost layer instead of expose them to virtio layer:

1) vhost protocol is stable ABI
2) no need to deal with virtio stuffs which is more complex than vhost

Or are there any advantages if we do it at virtio layer?


As far as I can tell, we will need the virtio layer the moment we
start copying/translating buffers.

In this series, the virtio dependency can be reduced if qemu does not
check the used ring _F_NO_NOTIFY flag before writing to irqfd. It
would enable packed queues and IOMMU immediately, and I think the cost
should not be so high. In the previous RFC this check was deleted
later anyway, so I think it was a bad idea to include it from the start.

I am not sure I understand here. For vhost, we can still do anything we
want, e.g accessing guest memory etc. Any blocker that prevent us from
copying/translating buffers? (Note that qemu will propagate memory
mappings to vhost).


There is nothing that forbids us to access directly, but if we don't
reuse the virtio layer functionality we would have to duplicate every
access function. "Need" was a too strong word maybe :).

In other words: for the shadow vq vring exposed for the device, qemu
treats it as a driver, and this functionality needs to be added to
qemu. But for accessing the guest's one do not reuse virtio.c would be
a bad idea in my opinion.


The problem is, virtio.c is not a library and it has a lot of dependency
with other qemu modules basically makes it impossible to be reused at
vhost level.


While virtio.c as a whole has dependencies, I think that the functions
needed in the original RFC do not have these dependencies.

However I see how to split vring dataplane from virtio device
management can benefit.



If you can split them out, that would be fine.





We can solve this by:

1) split the core functions out as a library or
2) switch to use contrib/lib-vhostuser but needs to decouple UNIX socket
transport

None of the above looks trivial and they are only device codes. For
shadow virtqueue, we need driver codes as well where no code can be reused.

As we discussed, we probably need IOVA allocated when forwarding
descriptors between the two virtqueues. So my feeling is we can have our
own codes to start then we can consider whether we can reuse some from
the existing virtio.c or lib-vhostuser.


As I see it, if we develop our own code a lot of it will be copied
from current virtio.c, which itself duplicates a lot of contrib/ lib
functionality.

Maybe it's better to combine your proposals and decouple the vring
functions, the vhost transport, and the qemu virtio device management,
so other projects can reuse them directly?



I think this can work.




I still think this can be left for a later series with buffer
forwarding on

RE: [RFC v3 2/3] virtio: Introduce Vdmabuf driver

2021-02-09 Thread Kasireddy, Vivek

Hi Gerd,

> -Original Message-
> From: Gerd Hoffmann 
> Sent: Tuesday, February 09, 2021 12:45 AM
> To: Kasireddy, Vivek 
> Cc: Daniel Vetter ; 
> virtualization@lists.linux-foundation.org; dri-
> de...@lists.freedesktop.org; Vetter, Daniel ;
> daniel.vet...@ffwll.ch; Kim, Dongwon ;
> sumit.sem...@linaro.org; christian.koe...@amd.com; linux-me...@vger.kernel.org
> Subject: Re: [RFC v3 2/3] virtio: Introduce Vdmabuf driver
> 
>   Hi,
> 
> > > > > Nack, this doesn't work on dma-buf. And it'll blow up at runtime
> > > > > when you enable the very recently merged CONFIG_DMABUF_DEBUG (would
> > > > > be good to test with that, just to make sure).
> > [Kasireddy, Vivek] Although, I have not tested it yet but it looks like 
> > this will
> > throw a wrench in our solution as we use sg_next to iterate over all the 
> > struct page *
> > and get their PFNs. I wonder if there is any other clean way to get the 
> > PFNs of all
> > the pages associated with a dmabuf.
> 
> Well, there is no guarantee that dma-buf backing storage actually has
> struct page ...
[Kasireddy, Vivek] What if I do mmap() on the fd followed by mlock() or mmap()
followed by get_user_pages()? If it still fails, would ioremapping the device 
memory
and poking at the backing storage be an option? Or, if I bind the passthrough'd 
GPU device
to vfio-pci and tap into the memory region associated with the device memory, 
can it be
made to work? 

And, I noticed that for PFNs that do not have valid struct page associated with 
it, KVM
does a memremap() to access/map them. Is this an option?

> 
> > [Kasireddy, Vivek] To exclude such cases, would it not be OK to limit the 
> > scope
> > of this solution (Vdmabuf) to make it clear that the dma-buf has to live in 
> > Guest RAM?
> > Or, are there any ways to pin the dma-buf pages in Guest RAM to make this
> > solution work?
> 
> At that point it becomes (i915) driver-specific.  If you go that route
> it doesn't look that useful to use dma-bufs in the first place ...
[Kasireddy, Vivek] I prefer not to make this driver specific if possible.

> 
> > IIUC, Virtio GPU is used to present a virtual GPU to the Guest and all the 
> > rendering
> > commands are captured and forwarded to the Host GPU via Virtio.
> 
> You don't have to use the rendering pipeline.  You can let the i915 gpu
> render into a dma-buf shared with virtio-gpu, then use virtio-gpu only for
> buffer sharing with the host.
[Kasireddy, Vivek] Is this the most viable path forward? I am not sure how 
complex or 
feasible it would be but I'll look into it.
Also, not using the rendering capabilities of virtio-gpu and turning it into a 
sharing only
device means there would be a giant mode switch with a lot of if() conditions 
sprinkled
across. Are you OK with that?

Thanks,
Vivek
> 
> take care,
>   Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 16/19] virtio-pci: introduce modern device module

2021-02-09 Thread Jason Wang



On 2021/2/9 下午10:20, Michael S. Tsirkin wrote:

On Mon, Jan 04, 2021 at 02:55:00PM +0800, Jason Wang wrote:

Signed-off-by: Jason Wang 
---
  drivers/virtio/Kconfig |  10 +-
  drivers/virtio/Makefile|   1 +
  drivers/virtio/virtio_pci_common.h |  27 +-
  drivers/virtio/virtio_pci_modern.c | 617 -
  drivers/virtio/virtio_pci_modern_dev.c | 599 
  include/linux/virtio_pci_modern.h  | 111 +
  6 files changed, 721 insertions(+), 644 deletions(-)
  create mode 100644 drivers/virtio/virtio_pci_modern_dev.c
  create mode 100644 include/linux/virtio_pci_modern.h

diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 7b41130d3f35..6b9b81f4b8c2 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -12,6 +12,14 @@ config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
  This option is selected if the architecture may need to enforce
  VIRTIO_F_ACCESS_PLATFORM
  
+config VIRTIO_PCI_MODERN

+   tristate "Modern Virtio PCI Device"
+   depends on PCI
+   help
+ Modern PCI device implementation. This module implements the
+ basic probe and control for devices which are based on modern
+ PCI device with possible vendor specific extensions.
+
  menuconfig VIRTIO_MENU
bool "Virtio drivers"
default y
@@ -20,7 +28,7 @@ if VIRTIO_MENU
  
  config VIRTIO_PCI

tristate "PCI driver for virtio devices"
-   depends on PCI
+   depends on VIRTIO_PCI_MODERN
select VIRTIO
help
  This driver provides support for virtio based paravirtual device

Looks like VIRTIO_PCI_MODERN is actually just a library that
virtio pci uses. Is that right?



Right.



In that case just select it
automatically, let's not make users enable it manually.



I've considered to do this but the problem is that the module depends on 
PCI so it can't be selected I think.


Thanks






___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 16/19] virtio-pci: introduce modern device module

2021-02-09 Thread Jason Wang



On 2021/2/9 下午6:15, Naresh Kamboju wrote:

Hi Jason,

On Mon, 4 Jan 2021 at 12:28, Jason Wang  wrote:

Signed-off-by: Jason Wang 
---
  drivers/virtio/Kconfig |  10 +-
  drivers/virtio/Makefile|   1 +
  drivers/virtio/virtio_pci_common.h |  27 +-
  drivers/virtio/virtio_pci_modern.c | 617 -
  drivers/virtio/virtio_pci_modern_dev.c | 599 
  include/linux/virtio_pci_modern.h  | 111 +
  6 files changed, 721 insertions(+), 644 deletions(-)
  create mode 100644 drivers/virtio/virtio_pci_modern_dev.c
  create mode 100644 include/linux/virtio_pci_modern.h

diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 7b41130d3f35..6b9b81f4b8c2 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -12,6 +12,14 @@ config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
   This option is selected if the architecture may need to enforce
   VIRTIO_F_ACCESS_PLATFORM

+config VIRTIO_PCI_MODERN
+   tristate "Modern Virtio PCI Device"
+   depends on PCI
+   help
+ Modern PCI device implementation. This module implements the
+ basic probe and control for devices which are based on modern
+ PCI device with possible vendor specific extensions.
+
  menuconfig VIRTIO_MENU
 bool "Virtio drivers"
 default y
@@ -20,7 +28,7 @@ if VIRTIO_MENU

  config VIRTIO_PCI
 tristate "PCI driver for virtio devices"
-   depends on PCI
+   depends on VIRTIO_PCI_MODERN

While booting Linux next tag 20210208 kernel on qemu_arm64 and qemu_arm
mount rootfs failed.  The root cause seems to be due to missing configs
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_PCI_LEGACY=y

Reported-by: Naresh Kamboju 

Then I have to force to enable this MODERN config
CONFIG_VIRTIO_PCI_MODERN=y
and which enabled
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_PCI_LEGACY=y

and the qemu_arm64 and qemu_arm boot pass.


New build link,
https://builds.tuxbuild.com/1oEse4EFsoQr1FkKBfiLmhMCe7j/



Thanks for the reporting.

I will post a patch to fix the def config to enable VIRTIO_PCI_MODERN.

Thanks







___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: fix param validation in mlx5_vdpa_get_config()

2021-02-09 Thread Jason Wang



On 2021/2/9 下午5:00, Stefano Garzarella wrote:

On Tue, Feb 09, 2021 at 07:43:02AM +0200, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 05:17:41PM +0100, Stefano Garzarella wrote:

It's legal to have 'offset + len' equal to
sizeof(struct virtio_net_config), since 'ndev->config' is a
'struct virtio_net_config', so we can safely copy its content under
this condition.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 
devices")

Cc: sta...@vger.kernel.org
Signed-off-by: Stefano Garzarella 


Acked-by: Eli Cohen 

BTW, same error in vdpa_sim you may want to fix.



Commit 65b709586e22 ("vdpa_sim: add get_config callback in 
vdpasim_dev_attr") unintentionally solved it.


Since it's a simulator, maybe we can avoid solving it in the stable 
branches. Or does it matter?



I think not, since the module depends on RUNTIME_TESTING_MENU.

Thanks






Thanks,
Stefano 


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 3/4] virtio-net: support transmit timestamp

2021-02-09 Thread Jason Wang



On 2021/2/10 上午10:36, Willem de Bruijn wrote:

On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin  wrote:

On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:

On 2021/2/9 上午2:55, Willem de Bruijn wrote:

From: Willem de Bruijn

Add optional PTP hardware tx timestamp offload for virtio-net.

Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.

The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.

The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.

Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.

RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.

Tested: modified txtimestamp.c to with h/w timestamping:
-   sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+   sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);

Signed-off-by: Willem de Bruijn
---
   drivers/net/virtio_net.c| 61 -
   drivers/virtio/virtio_ring.c|  3 +-
   include/linux/virtio.h  |  1 +
   include/uapi/linux/virtio_net.h |  1 +
   4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ac44c5efa0bc..fc8ecd3a333a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -210,6 +210,12 @@ struct virtnet_info {
 /* Device will pass rx timestamp. Requires has_rx_tstamp */
 bool enable_rx_tstamp;
+   /* Device can pass CLOCK_TAI transmit time to the driver */
+   bool has_tx_tstamp;
+
+   /* Device will pass tx timestamp. Requires has_tx_tstamp */
+   bool enable_tx_tstamp;
+
 /* Has control virtqueue */
 bool has_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int 
budget,
 return stats.packets;
   }
+static void virtnet_record_tx_tstamp(const struct send_queue *sq,
+struct sk_buff *skb)
+{
+   const struct virtio_net_hdr_hash_ts *h = skb_vnet_hdr_ht(skb);
+   const struct virtnet_info *vi = sq->vq->vdev->priv;
+   struct skb_shared_hwtstamps ts;
+
+   if (h->hdr.flags & VIRTIO_NET_HDR_F_TSTAMP &&
+   vi->enable_tx_tstamp) {
+   ts.hwtstamp = ns_to_ktime(le64_to_cpu(h->tstamp));
+   skb_tstamp_tx(skb, );

This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).

Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.

Both Driver Requirements: Message Framing and Driver Requirements: 
Scatter-Gather Support
have this statement:

 The driver MUST place any device-writable descriptor elements after 
any device-readable descriptor ele-
 ments.


similarly

Device Requirements: The Virtqueue Descriptor Table
 A device MUST NOT write to a device-readable buffer, and a device 
SHOULD NOT read a device-writable
 buffer.

Thanks. That's clear. So the clean solution would be to add a
device-writable descriptor after the existing device-readable ones.



I think so, but a question is the format for this tailer. I think it 
might be better to post a spec patch to discuss.


Thanks




And the device must be aware that this is to return the tstamp only.
In the example implementation of vhost, it has to exclude this last
descriptor from the msg->msg_iter iovec array with packet data
initialized at get_tx_bufs/init_iov_iter.



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 3/4] virtio-net: support transmit timestamp

2021-02-09 Thread Jason Wang



On 2021/2/10 上午12:38, Michael S. Tsirkin wrote:

On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:

On 2021/2/9 上午2:55, Willem de Bruijn wrote:

From: Willem de Bruijn

Add optional PTP hardware tx timestamp offload for virtio-net.

Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
equivalent to VIRTIO_NET_F_RX_TSTAMP.

The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
returned on completion. If the feature is negotiated, the device
either places the timestamp or clears the feature bit.

The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.

Modify can_push to ensure that on tx completion the header, and thus
timestamp, is in a predicatable location at skb_vnet_hdr.

RFC: this implementation relies on the device writing to the buffer.
That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
The virtio changes should be a separate patch at the least.

Tested: modified txtimestamp.c to with h/w timestamping:
-   sock_opt = SOF_TIMESTAMPING_SOFTWARE |
+   sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);

Signed-off-by: Willem de Bruijn
---
   drivers/net/virtio_net.c| 61 -
   drivers/virtio/virtio_ring.c|  3 +-
   include/linux/virtio.h  |  1 +
   include/uapi/linux/virtio_net.h |  1 +
   4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ac44c5efa0bc..fc8ecd3a333a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -210,6 +210,12 @@ struct virtnet_info {
/* Device will pass rx timestamp. Requires has_rx_tstamp */
bool enable_rx_tstamp;
+   /* Device can pass CLOCK_TAI transmit time to the driver */
+   bool has_tx_tstamp;
+
+   /* Device will pass tx timestamp. Requires has_tx_tstamp */
+   bool enable_tx_tstamp;
+
/* Has control virtqueue */
bool has_cvq;
@@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, int 
budget,
return stats.packets;
   }
+static void virtnet_record_tx_tstamp(const struct send_queue *sq,
+struct sk_buff *skb)
+{
+   const struct virtio_net_hdr_hash_ts *h = skb_vnet_hdr_ht(skb);
+   const struct virtnet_info *vi = sq->vq->vdev->priv;
+   struct skb_shared_hwtstamps ts;
+
+   if (h->hdr.flags & VIRTIO_NET_HDR_F_TSTAMP &&
+   vi->enable_tx_tstamp) {
+   ts.hwtstamp = ns_to_ktime(le64_to_cpu(h->tstamp));
+   skb_tstamp_tx(skb, );

This probably won't work since the buffer is read-only from the device. (See
virtqueue_add_outbuf()).

Another issue that I vaguely remember that the virtio spec forbids out
buffer after in buffer.

Both Driver Requirements: Message Framing and Driver Requirements: 
Scatter-Gather Support
have this statement:

The driver MUST place any device-writable descriptor elements after any 
device-readable descriptor ele-
ments.


similarly

Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device 
SHOULD NOT read a device-writable
buffer.




Exactly. But I wonder what's the rationale behinds those requirements?

Thanks


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 2/4] virtio-net: support receive timestamp

2021-02-09 Thread Jason Wang



On 2021/2/9 下午9:53, Willem de Bruijn wrote:

On Mon, Feb 8, 2021 at 11:13 PM Jason Wang  wrote:


On 2021/2/9 上午2:55, Willem de Bruijn wrote:

From: Willem de Bruijn 

Add optional PTP hardware rx timestamp offload for virtio-net.

Accurate RTT measurement requires timestamps close to the wire.
Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
virtio-net header is expanded with room for a timestamp.

A device may pass receive timestamps for all or some packets. Flag
VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.

A driver that supports hardware timestamping must also support
ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).

The timestamp straddles (virtual) hardware domains. Like PTP, use
international atomic time (CLOCK_TAI) as global clock base. The driver
must sync with the device, e.g., through kvm-clock.

Tested:
guest: ./timestamping eth0 \
SOF_TIMESTAMPING_RAW_HARDWARE \
SOF_TIMESTAMPING_RX_HARDWARE
host: nc -4 -u 192.168.1.1 319

Changes RFC -> RFCv2
- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
- add ethtool .get_ts_info to query capabilities
- add ioctl SIOC[GS]HWTSTAMP to configure feature
- add vi->enable_rx_tstamp to store configuration
- convert virtioXX_to_cpu to leXX_to_cpu
- convert reserved to __u32

Signed-off-by: Willem de Bruijn 
   static const struct net_device_ops virtnet_netdev = {
   .ndo_open= virtnet_open,
   .ndo_stop= virtnet_close,
@@ -2573,6 +2676,7 @@ static const struct net_device_ops virtnet_netdev = {
   .ndo_features_check = passthru_features_check,
   .ndo_get_phys_port_name = virtnet_get_phys_port_name,
   .ndo_set_features   = virtnet_set_features,
+ .ndo_do_ioctl   = virtnet_ioctl,
   };

   static void virtnet_config_changed_work(struct work_struct *work)
@@ -3069,6 +3173,11 @@ static int virtnet_probe(struct virtio_device *vdev)
   vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
   }

+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RX_TSTAMP)) {
+ vi->has_rx_tstamp = true;
+ vi->hdr_len = sizeof(struct virtio_net_hdr_hash_ts);


Does this mean even if the device doesn't pass timestamp, the header
still contains the timestamp fields.

Yes. As implemented, the size of the header is constant across
packets. If both sides negotiate the feature, then all headers reserve
space, whether or not the specific packet has a timestamp.

So far headers are fixed size. I suppose we could investigate variable
size headers. This goes back to our discussion in the previous
patchset, that we can always add a packed-header feature later, if the
number of optional features reaches a size that makes the complexity
worthwhile.



Right, so for timstamp it's probably OK but we probably need to do as 
you said here if we want to add more in the header. Let's see how 
Michael think about this.






+ }
+
   if (virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT) ||
   virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
   vi->any_header_sg = true;
@@ -3260,7 +3369,7 @@ static struct virtio_device_id id_table[] = {
   VIRTIO_NET_F_CTRL_MAC_ADDR, \
   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
   VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
- VIRTIO_NET_F_TX_HASH
+ VIRTIO_NET_F_TX_HASH, VIRTIO_NET_F_RX_TSTAMP

   static unsigned int features[] = {
   VIRTNET_FEATURES,
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 273d43c35f59..a5c84410cf92 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -57,6 +57,7 @@
* Steering */
   #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */

+#define VIRTIO_NET_F_RX_TSTAMP 55/* Device sends TAI receive time 
*/
   #define VIRTIO_NET_F_TX_HASH  56/* Driver sends hash report */
   #define VIRTIO_NET_F_HASH_REPORT  57/* Supports hash report */
   #define VIRTIO_NET_F_RSS  60/* Supports RSS RX steering */
@@ -126,6 +127,7 @@ struct virtio_net_hdr_v1 {
   #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, csum_offset */
   #define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
   #define VIRTIO_NET_HDR_F_RSC_INFO   4   /* rsc info in csum_ fields */
+#define VIRTIO_NET_HDR_F_TSTAMP  8   /* timestamp is recorded 
*/
   __u8 flags;
   #define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
   #define VIRTIO_NET_HDR_GSO_TCPV41   /* GSO frame, IPv4 TCP (TSO) */
@@ -181,6 +183,17 @@ struct virtio_net_hdr_v1_hash {
   };
   };

+struct virtio_net_hdr_hash_ts {
+ struct virtio_net_hdr_v1 hdr;
+ struct {
+ __le32 value;
+ __le16 report;
+

Re: [PATCH 3/3] mlx5_vdpa: defer clear_virtqueues to until DRIVER_OK

2021-02-09 Thread Jason Wang



On 2021/2/10 上午8:26, Si-Wei Liu wrote:



On 2/8/2021 7:37 PM, Jason Wang wrote:


On 2021/2/6 下午8:29, Si-Wei Liu wrote:

While virtq is stopped,  get_vq_state() is supposed to
be  called to  get  sync'ed  with  the latest internal
avail_index from device. The saved avail_index is used
to restate  the virtq  once device is started.  Commit
b35ccebe3ef7 introduced the clear_virtqueues() routine
to  reset  the saved  avail_index,  however, the index
gets cleared a bit earlier before get_vq_state() tries
to read it. This would cause consistency problems when
virtq is restarted, e.g. through a series of link down
and link up events. We  could  defer  the  clearing of
avail_index  to  until  the  device  is to be started,
i.e. until  VIRTIO_CONFIG_S_DRIVER_OK  is set again in
set_status().

Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index 
after change map")

Signed-off-by: Si-Wei Liu 
---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c

index aa6f8cd..444ab58 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1785,7 +1785,6 @@ static void mlx5_vdpa_set_status(struct 
vdpa_device *vdev, u8 status)

  if (!status) {
  mlx5_vdpa_info(mvdev, "performing device reset\n");
  teardown_driver(ndev);
-    clear_virtqueues(ndev);
  mlx5_vdpa_destroy_mr(>mvdev);
  ndev->mvdev.status = 0;
  ++mvdev->generation;
@@ -1794,6 +1793,7 @@ static void mlx5_vdpa_set_status(struct 
vdpa_device *vdev, u8 status)
    if ((status ^ ndev->mvdev.status) & 
VIRTIO_CONFIG_S_DRIVER_OK) {

  if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+    clear_virtqueues(ndev);



Rethink about this. As mentioned in another thread, this in fact 
breaks set_vq_state().  (See vhost_virtqueue_start() -> 
vhost_vdpa_set_vring_base() in qemu codes).

I assume that the clearing for vhost-vdpa would be done via (qemu code),

vhost_dev_start()->vhost_vdpa_dev_start()->vhost_vdpa_call(status | 
VIRTIO_CONFIG_S_DRIVER_OK)


which is _after_ vhost_virtqueue_start() gets called to restore the 
avail_idx to h/w in vhost_dev_start(). What am I missing here?


-Siwei



I think not. I thought clear_virtqueues() will clear hardware index but 
looks not. (I guess we need a better name other than clear_virtqueues(), 
e.g from the name it looks like the it will clear the hardware states)


Thanks







The issue is that the avail idx is forgot, we need keep it.

Thanks



  err = setup_driver(ndev);
  if (err) {
  mlx5_vdpa_warn(mvdev, "failed to setup driver\n");






___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1] vdpa/mlx5: Restore the hardware used index after change map

2021-02-09 Thread Jason Wang



On 2021/2/10 上午10:30, Si-Wei Liu wrote:



On 2/8/2021 10:37 PM, Jason Wang wrote:


On 2021/2/9 下午2:12, Eli Cohen wrote:

On Tue, Feb 09, 2021 at 11:20:14AM +0800, Jason Wang wrote:

On 2021/2/8 下午6:04, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 05:04:27PM +0800, Jason Wang wrote:

On 2021/2/8 下午2:37, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 12:27:18PM +0800, Jason Wang wrote:

On 2021/2/6 上午7:07, Si-Wei Liu wrote:

On 2/3/2021 11:36 PM, Eli Cohen wrote:
When a change of memory map occurs, the hardware resources 
are destroyed
and then re-created again with the new memory map. In such 
case, we need
to restore the hardware available and used indices. The 
driver failed to

restore the used index which is added here.

Also, since the driver also fails to reset the available and 
used
indices upon device reset, fix this here to avoid regression 
caused by

the fact that used index may not be zero upon device reset.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for 
supported mlx5

devices")
Signed-off-by: Eli Cohen
---
v0 -> v1:
Clear indices upon device reset

     drivers/vdpa/mlx5/net/mlx5_vnet.c | 18 ++
     1 file changed, 18 insertions(+)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 88dde3455bfd..b5fe6d2ad22f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -87,6 +87,7 @@ struct mlx5_vq_restore_info {
     u64 device_addr;
     u64 driver_addr;
     u16 avail_index;
+    u16 used_index;
     bool ready;
     struct vdpa_callback cb;
     bool restore;
@@ -121,6 +122,7 @@ struct mlx5_vdpa_virtqueue {
     u32 virtq_id;
     struct mlx5_vdpa_net *ndev;
     u16 avail_idx;
+    u16 used_idx;
     int fw_state;
       /* keep last in the struct */
@@ -804,6 +806,7 @@ static int create_virtqueue(struct 
mlx5_vdpa_net

*ndev, struct mlx5_vdpa_virtque
       obj_context = MLX5_ADDR_OF(create_virtio_net_q_in, 
in,

obj_context);
     MLX5_SET(virtio_net_q_object, obj_context, 
hw_available_index,

mvq->avail_idx);
+    MLX5_SET(virtio_net_q_object, obj_context, hw_used_index,
mvq->used_idx);
     MLX5_SET(virtio_net_q_object, obj_context,
queue_feature_bit_mask_12_3,
get_features_12_3(ndev->mvdev.actual_features));
     vq_ctx = MLX5_ADDR_OF(virtio_net_q_object, obj_context,
virtio_q_context);
@@ -1022,6 +1025,7 @@ static int connect_qps(struct 
mlx5_vdpa_net

*ndev, struct mlx5_vdpa_virtqueue *m
     struct mlx5_virtq_attr {
     u8 state;
     u16 available_index;
+    u16 used_index;
     };
       static int query_virtqueue(struct mlx5_vdpa_net *ndev, 
struct

mlx5_vdpa_virtqueue *mvq,
@@ -1052,6 +1056,7 @@ static int query_virtqueue(struct
mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueu
     memset(attr, 0, sizeof(*attr));
     attr->state = MLX5_GET(virtio_net_q_object, 
obj_context, state);

     attr->available_index = MLX5_GET(virtio_net_q_object,
obj_context, hw_available_index);
+    attr->used_index = MLX5_GET(virtio_net_q_object, 
obj_context,

hw_used_index);
     kfree(out);
     return 0;
     @@ -1535,6 +1540,16 @@ static void 
teardown_virtqueues(struct

mlx5_vdpa_net *ndev)
     }
     }
     +static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
+{
+    int i;
+
+    for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
+    ndev->vqs[i].avail_idx = 0;
+    ndev->vqs[i].used_idx = 0;
+    }
+}
+
     /* TODO: cross-endian support */
     static inline bool mlx5_vdpa_is_little_endian(struct 
mlx5_vdpa_dev

*mvdev)
     {
@@ -1610,6 +1625,7 @@ static int save_channel_info(struct
mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqu
     return err;
       ri->avail_index = attr.available_index;
+    ri->used_index = attr.used_index;
     ri->ready = mvq->ready;
     ri->num_ent = mvq->num_ent;
     ri->desc_addr = mvq->desc_addr;
@@ -1654,6 +1670,7 @@ static void restore_channels_info(struct
mlx5_vdpa_net *ndev)
     continue;
       mvq->avail_idx = ri->avail_index;
+    mvq->used_idx = ri->used_index;
     mvq->ready = ri->ready;
     mvq->num_ent = ri->num_ent;
     mvq->desc_addr = ri->desc_addr;
@@ -1768,6 +1785,7 @@ static void mlx5_vdpa_set_status(struct
vdpa_device *vdev, u8 status)
     if (!status) {
     mlx5_vdpa_info(mvdev, "performing device reset\n");
     teardown_driver(ndev);
+    clear_virtqueues(ndev);
The clearing looks fine at the first glance, as it aligns with 
the other
state cleanups floating around at the same place. However, the 
thing is
get_vq_state() is supposed to be called right after to get 
sync'ed with
the latest internal avail_index from device while vq is 
stopped. The
index was saved in the driver software at vq suspension, but 
before the
virtq object is destroyed. We shouldn't clear the avail_index 
too early.

Good point.

There's a limitation on the virtio spec

Re: [PATCH RFC v2 3/4] virtio-net: support transmit timestamp

2021-02-09 Thread Willem de Bruijn

On Tue, Feb 9, 2021 at 11:39 AM Michael S. Tsirkin  wrote:
>
> On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
> >
> > On 2021/2/9 上午2:55, Willem de Bruijn wrote:
> > > From: Willem de Bruijn 
> > >
> > > Add optional PTP hardware tx timestamp offload for virtio-net.
> > >
> > > Accurate RTT measurement requires timestamps close to the wire.
> > > Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
> > > equivalent to VIRTIO_NET_F_RX_TSTAMP.
> > >
> > > The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
> > > returned on completion. If the feature is negotiated, the device
> > > either places the timestamp or clears the feature bit.
> > >
> > > The timestamp straddles (virtual) hardware domains. Like PTP, use
> > > international atomic time (CLOCK_TAI) as global clock base. The driver
> > > must sync with the device, e.g., through kvm-clock.
> > >
> > > Modify can_push to ensure that on tx completion the header, and thus
> > > timestamp, is in a predicatable location at skb_vnet_hdr.
> > >
> > > RFC: this implementation relies on the device writing to the buffer.
> > > That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
> > > The virtio changes should be a separate patch at the least.
> > >
> > > Tested: modified txtimestamp.c to with h/w timestamping:
> > >-   sock_opt = SOF_TIMESTAMPING_SOFTWARE |
> > >+   sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
> > >+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
> > >
> > > Signed-off-by: Willem de Bruijn 
> > > ---
> > >   drivers/net/virtio_net.c| 61 -
> > >   drivers/virtio/virtio_ring.c|  3 +-
> > >   include/linux/virtio.h  |  1 +
> > >   include/uapi/linux/virtio_net.h |  1 +
> > >   4 files changed, 56 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index ac44c5efa0bc..fc8ecd3a333a 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -210,6 +210,12 @@ struct virtnet_info {
> > > /* Device will pass rx timestamp. Requires has_rx_tstamp */
> > > bool enable_rx_tstamp;
> > > +   /* Device can pass CLOCK_TAI transmit time to the driver */
> > > +   bool has_tx_tstamp;
> > > +
> > > +   /* Device will pass tx timestamp. Requires has_tx_tstamp */
> > > +   bool enable_tx_tstamp;
> > > +
> > > /* Has control virtqueue */
> > > bool has_cvq;
> > > @@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue 
> > > *rq, int budget,
> > > return stats.packets;
> > >   }
> > > +static void virtnet_record_tx_tstamp(const struct send_queue *sq,
> > > +struct sk_buff *skb)
> > > +{
> > > +   const struct virtio_net_hdr_hash_ts *h = skb_vnet_hdr_ht(skb);
> > > +   const struct virtnet_info *vi = sq->vq->vdev->priv;
> > > +   struct skb_shared_hwtstamps ts;
> > > +
> > > +   if (h->hdr.flags & VIRTIO_NET_HDR_F_TSTAMP &&
> > > +   vi->enable_tx_tstamp) {
> > > +   ts.hwtstamp = ns_to_ktime(le64_to_cpu(h->tstamp));
> > > +   skb_tstamp_tx(skb, );
> >
> >
> > This probably won't work since the buffer is read-only from the device. (See
> > virtqueue_add_outbuf()).
> >
> > Another issue that I vaguely remember that the virtio spec forbids out
> > buffer after in buffer.
>
> Both Driver Requirements: Message Framing and Driver Requirements: 
> Scatter-Gather Support
> have this statement:
>
> The driver MUST place any device-writable descriptor elements after 
> any device-readable descriptor ele-
> ments.
>
>
> similarly
>
> Device Requirements: The Virtqueue Descriptor Table
> A device MUST NOT write to a device-readable buffer, and a device 
> SHOULD NOT read a device-writable
> buffer.

Thanks. That's clear. So the clean solution would be to add a
device-writable descriptor after the existing device-readable ones.

And the device must be aware that this is to return the tstamp only.
In the example implementation of vhost, it has to exclude this last
descriptor from the msg->msg_iter iovec array with packet data
initialized at get_tx_bufs/init_iov_iter.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1] vdpa/mlx5: Restore the hardware used index after change map

2021-02-09 Thread Si-Wei Liu




On 2/8/2021 10:37 PM, Jason Wang wrote:


On 2021/2/9 下午2:12, Eli Cohen wrote:

On Tue, Feb 09, 2021 at 11:20:14AM +0800, Jason Wang wrote:

On 2021/2/8 下午6:04, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 05:04:27PM +0800, Jason Wang wrote:

On 2021/2/8 下午2:37, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 12:27:18PM +0800, Jason Wang wrote:

On 2021/2/6 上午7:07, Si-Wei Liu wrote:

On 2/3/2021 11:36 PM, Eli Cohen wrote:
When a change of memory map occurs, the hardware resources are 
destroyed
and then re-created again with the new memory map. In such 
case, we need
to restore the hardware available and used indices. The driver 
failed to

restore the used index which is added here.

Also, since the driver also fails to reset the available and used
indices upon device reset, fix this here to avoid regression 
caused by

the fact that used index may not be zero upon device reset.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported 
mlx5

devices")
Signed-off-by: Eli Cohen
---
v0 -> v1:
Clear indices upon device reset

     drivers/vdpa/mlx5/net/mlx5_vnet.c | 18 ++
     1 file changed, 18 insertions(+)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 88dde3455bfd..b5fe6d2ad22f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -87,6 +87,7 @@ struct mlx5_vq_restore_info {
     u64 device_addr;
     u64 driver_addr;
     u16 avail_index;
+    u16 used_index;
     bool ready;
     struct vdpa_callback cb;
     bool restore;
@@ -121,6 +122,7 @@ struct mlx5_vdpa_virtqueue {
     u32 virtq_id;
     struct mlx5_vdpa_net *ndev;
     u16 avail_idx;
+    u16 used_idx;
     int fw_state;
       /* keep last in the struct */
@@ -804,6 +806,7 @@ static int create_virtqueue(struct 
mlx5_vdpa_net

*ndev, struct mlx5_vdpa_virtque
       obj_context = MLX5_ADDR_OF(create_virtio_net_q_in, in,
obj_context);
     MLX5_SET(virtio_net_q_object, obj_context, 
hw_available_index,

mvq->avail_idx);
+    MLX5_SET(virtio_net_q_object, obj_context, hw_used_index,
mvq->used_idx);
     MLX5_SET(virtio_net_q_object, obj_context,
queue_feature_bit_mask_12_3,
get_features_12_3(ndev->mvdev.actual_features));
     vq_ctx = MLX5_ADDR_OF(virtio_net_q_object, obj_context,
virtio_q_context);
@@ -1022,6 +1025,7 @@ static int connect_qps(struct mlx5_vdpa_net
*ndev, struct mlx5_vdpa_virtqueue *m
     struct mlx5_virtq_attr {
     u8 state;
     u16 available_index;
+    u16 used_index;
     };
       static int query_virtqueue(struct mlx5_vdpa_net *ndev, 
struct

mlx5_vdpa_virtqueue *mvq,
@@ -1052,6 +1056,7 @@ static int query_virtqueue(struct
mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueu
     memset(attr, 0, sizeof(*attr));
     attr->state = MLX5_GET(virtio_net_q_object, 
obj_context, state);

     attr->available_index = MLX5_GET(virtio_net_q_object,
obj_context, hw_available_index);
+    attr->used_index = MLX5_GET(virtio_net_q_object, 
obj_context,

hw_used_index);
     kfree(out);
     return 0;
     @@ -1535,6 +1540,16 @@ static void 
teardown_virtqueues(struct

mlx5_vdpa_net *ndev)
     }
     }
     +static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
+{
+    int i;
+
+    for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
+    ndev->vqs[i].avail_idx = 0;
+    ndev->vqs[i].used_idx = 0;
+    }
+}
+
     /* TODO: cross-endian support */
     static inline bool mlx5_vdpa_is_little_endian(struct 
mlx5_vdpa_dev

*mvdev)
     {
@@ -1610,6 +1625,7 @@ static int save_channel_info(struct
mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqu
     return err;
       ri->avail_index = attr.available_index;
+    ri->used_index = attr.used_index;
     ri->ready = mvq->ready;
     ri->num_ent = mvq->num_ent;
     ri->desc_addr = mvq->desc_addr;
@@ -1654,6 +1670,7 @@ static void restore_channels_info(struct
mlx5_vdpa_net *ndev)
     continue;
       mvq->avail_idx = ri->avail_index;
+    mvq->used_idx = ri->used_index;
     mvq->ready = ri->ready;
     mvq->num_ent = ri->num_ent;
     mvq->desc_addr = ri->desc_addr;
@@ -1768,6 +1785,7 @@ static void mlx5_vdpa_set_status(struct
vdpa_device *vdev, u8 status)
     if (!status) {
     mlx5_vdpa_info(mvdev, "performing device reset\n");
     teardown_driver(ndev);
+    clear_virtqueues(ndev);
The clearing looks fine at the first glance, as it aligns with 
the other
state cleanups floating around at the same place. However, the 
thing is
get_vq_state() is supposed to be called right after to get 
sync'ed with
the latest internal avail_index from device while vq is 
stopped. The
index was saved in the driver software at vq suspension, but 
before the
virtq object is destroyed. We shouldn't clear the avail_index 
too early.

Good point.

There's a limitation on the virtio spec and vDPA framework that 
we can not

simply

Re: [PATCH 3/3] mlx5_vdpa: defer clear_virtqueues to until DRIVER_OK

2021-02-09 Thread Si-Wei Liu




On 2/8/2021 7:37 PM, Jason Wang wrote:


On 2021/2/6 下午8:29, Si-Wei Liu wrote:

While virtq is stopped,  get_vq_state() is supposed to
be  called to  get  sync'ed  with  the latest internal
avail_index from device. The saved avail_index is used
to restate  the virtq  once device is started.  Commit
b35ccebe3ef7 introduced the clear_virtqueues() routine
to  reset  the saved  avail_index,  however, the index
gets cleared a bit earlier before get_vq_state() tries
to read it. This would cause consistency problems when
virtq is restarted, e.g. through a series of link down
and link up events. We  could  defer  the  clearing of
avail_index  to  until  the  device  is to be started,
i.e. until  VIRTIO_CONFIG_S_DRIVER_OK  is set again in
set_status().

Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index 
after change map")

Signed-off-by: Si-Wei Liu 
---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c

index aa6f8cd..444ab58 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1785,7 +1785,6 @@ static void mlx5_vdpa_set_status(struct 
vdpa_device *vdev, u8 status)

  if (!status) {
  mlx5_vdpa_info(mvdev, "performing device reset\n");
  teardown_driver(ndev);
-    clear_virtqueues(ndev);
  mlx5_vdpa_destroy_mr(>mvdev);
  ndev->mvdev.status = 0;
  ++mvdev->generation;
@@ -1794,6 +1793,7 @@ static void mlx5_vdpa_set_status(struct 
vdpa_device *vdev, u8 status)

    if ((status ^ ndev->mvdev.status) & VIRTIO_CONFIG_S_DRIVER_OK) {
  if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+    clear_virtqueues(ndev);



Rethink about this. As mentioned in another thread, this in fact 
breaks set_vq_state().  (See vhost_virtqueue_start() -> 
vhost_vdpa_set_vring_base() in qemu codes).

I assume that the clearing for vhost-vdpa would be done via (qemu code),

vhost_dev_start()->vhost_vdpa_dev_start()->vhost_vdpa_call(status | 
VIRTIO_CONFIG_S_DRIVER_OK)


which is _after_ vhost_virtqueue_start() gets called to restore the 
avail_idx to h/w in vhost_dev_start(). What am I missing here?


-Siwei




The issue is that the avail idx is forgot, we need keep it.

Thanks



  err = setup_driver(ndev);
  if (err) {
  mlx5_vdpa_warn(mvdev, "failed to setup driver\n");




___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

CISTI'2021 - 16th Iberian Conference on IST | Deadline: 21th of February

2021-02-09 Thread Lemos

* Published by IEEE
* Google Scholar H-Index = 17
* SCIMago H-Index = 13
* Indexed in Scopus, WoS, Ei-Compendex, Inspec, etc


-   
   --
CISTI'2021 - 16th Iberian Conference on Information Systems and Technologies
23 - 26 June 2021 | Chaves, Portugal
http://cisti.eu/ 
--- -   
  ---


We are pleased to invite the academic and business community to submit their 
papers to CISTI'2021 - 16th Iberian Conference on Information Systems and 
Technologies, to be held in Chaves, Portugal, between the 23th and 26th of June 
2021. Authors are encouraged to submit original scientific contributions such 
as state-of-art reviews and new research perspectives, groundbreaking ideas 
and/or architectures, solutions and/or applications for real problems, 
empirical and/or evaluation works, case studies, etc., in conformity with the 
themes of this Conference.

Four types of papers can be submitted:

Full paper: Finished or consolidated R works, to be included in one of the 
Conference themes. These papers are assigned  a 6-page limit.

Short paper: Ongoing works with relevant preliminary results, opened to 
discussion. These papers are assigned a 4-page limit.

Poster paper: Initial work with relevant ideas, opened to discussion. These 
papers are assigned a 2-page limit.

Company paper: Companies' papers  that show practical experience, R & D, tools, 
etc., focused in some topics of the conference. These articles are abstracts 
with a maximum of 2 pages.

Papers submitted for the Scientific Committee’s evaluation must not include any 
information leading to the authors’ identification. Therefore, the authors’ 
names, affiliations and bibliographic references should not be included in the 
early version. This information should only be included in the final version.

Submitted papers must not have been published and must not be under review for 
any other conference and national or international publication.

Papers must comply with the format standard 
 and be written in Portuguese, 
Spanish or English.

All papers will be subjected to a “blind review” by at least two members of the 
Scientific Committee.

Full papers can be accepted as short papers or poster papers only. Similarly, 
short papers can be accepted as poster papers only. In these two cases, the 
authors will be allowed to maintain the original number of pages in the 
proceedings publication.

The authors of accepted poster papers must also build and print a poster to be 
exhibited during the Conference. This poster must follow an A1 or A2 vertical 
format. The Conference includes Work Sessions where these posters are presented 
and orally discussed, with a 5-minute limit per poster.

The authors of accepted full papers will dispose of a 15-minute presentation in 
the Conference Work Session, and approximately 5 minutes of discussion will 
follow each presentation. The authors of accepted short papers and company 
papers will dispose of an 11-minute presentation in the Conference Work 
Session, and approximately 4 minutes of discussion will follow each 
presentation.



Themes

Submitted papers must follow the main themes proposed for the Conference (the 
topics proposed in each theme constitute a mere framework reference; they are 
not intended as restrictive):

A) OMIS - Organizational Models and Information Systems

B) KMDSS - Knowledge Management and Decision Support Systems

C) SSAAT - Software Systems, Architectures, Applications and Tools

D) CNMPS - Computer Networks, Mobility and Pervasive Systems

E) HCC - Human Centered Computing

F) HIS - Health Informatics

G) ITE - Information Technologies in Education


H) AEC – Architecture and Engineering of Construction



Publication and Indexing

To ensure that the contribution (full paper, short paper, symposium doctoral 
paper) is published in the Proceedings, at least one of the authors must be 
fully registered by the 11th of April, and the paper must comply with the 
suggested layout and page-limit. Additionally, all recommended modifications 
must be addressed by the authors before they submit the final version.

No more than one paper per registration will be published in the Conference 
Proceedings. An extra fee must be paid for publication of additional papers, 
with a maximum of one additional paper per registration.

Full and short papers, including symposium doctoral papers, will be submitted 
for inclusion/indexing into IEEE XPlore, ISI, SCOPUS, EI-Compendex, INSPEC and 
Google Scholar.

The best articles will be selected for publication in the following journals 
and books:

- Journal of Information Systems Engineering & Management (JISEM 
)

-

[PATCH v5 4/8] x86/mm/tlb: Flush remote and local TLBs concurrently

2021-02-09 Thread Nadav Amit

From: Nadav Amit 

To improve TLB shootdown performance, flush the remote and local TLBs
concurrently. Introduce flush_tlb_multi() that does so. Introduce
paravirtual versions of flush_tlb_multi() for KVM, Xen and hyper-v (Xen
and hyper-v are only compile-tested).

While the updated smp infrastructure is capable of running a function on
a single local core, it is not optimized for this case. The multiple
function calls and the indirect branch introduce some overhead, and
might make local TLB flushes slower than they were before the recent
changes.

Before calling the SMP infrastructure, check if only a local TLB flush
is needed to restore the lost performance in this common case. This
requires to check mm_cpumask() one more time, but unless this mask is
updated very frequently, this should impact performance negatively.

Cc: "K. Y. Srinivasan" 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: Sasha Levin 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: x...@kernel.org
Cc: Juergen Gross 
Cc: Paolo Bonzini 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Boris Ostrovsky 
Cc: linux-hyp...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: k...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Reviewed-by: Michael Kelley  # Hyper-v parts
Reviewed-by: Juergen Gross  # Xen and paravirt parts
Reviewed-by: Dave Hansen 
Signed-off-by: Nadav Amit 
---
 arch/x86/hyperv/mmu.c | 10 +++---
 arch/x86/include/asm/paravirt.h   |  6 ++--
 arch/x86/include/asm/paravirt_types.h |  4 +--
 arch/x86/include/asm/tlbflush.h   |  4 +--
 arch/x86/include/asm/trace/hyperv.h   |  2 +-
 arch/x86/kernel/kvm.c | 11 --
 arch/x86/mm/tlb.c | 49 +--
 arch/x86/xen/mmu_pv.c | 11 +++---
 include/trace/events/xen.h|  2 +-
 9 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index 2c87350c1fb0..681dba8de4f2 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -52,8 +52,8 @@ static inline int fill_gva_list(u64 gva_list[], int offset,
return gva_n - offset;
 }
 
-static void hyperv_flush_tlb_others(const struct cpumask *cpus,
-   const struct flush_tlb_info *info)
+static void hyperv_flush_tlb_multi(const struct cpumask *cpus,
+  const struct flush_tlb_info *info)
 {
int cpu, vcpu, gva_n, max_gvas;
struct hv_tlb_flush **flush_pcpu;
@@ -61,7 +61,7 @@ static void hyperv_flush_tlb_others(const struct cpumask 
*cpus,
u64 status = U64_MAX;
unsigned long flags;
 
-   trace_hyperv_mmu_flush_tlb_others(cpus, info);
+   trace_hyperv_mmu_flush_tlb_multi(cpus, info);
 
if (!hv_hypercall_pg)
goto do_native;
@@ -164,7 +164,7 @@ static void hyperv_flush_tlb_others(const struct cpumask 
*cpus,
if (!(status & HV_HYPERCALL_RESULT_MASK))
return;
 do_native:
-   native_flush_tlb_others(cpus, info);
+   native_flush_tlb_multi(cpus, info);
 }
 
 static u64 hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
@@ -239,6 +239,6 @@ void hyperv_setup_mmu_ops(void)
return;
 
pr_info("Using hypercall for remote TLB flush\n");
-   pv_ops.mmu.flush_tlb_others = hyperv_flush_tlb_others;
+   pv_ops.mmu.flush_tlb_multi = hyperv_flush_tlb_multi;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
 }
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f8dce11d2bc1..515e49204c8b 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -50,7 +50,7 @@ static inline void slow_down_io(void)
 void native_flush_tlb_local(void);
 void native_flush_tlb_global(void);
 void native_flush_tlb_one_user(unsigned long addr);
-void native_flush_tlb_others(const struct cpumask *cpumask,
+void native_flush_tlb_multi(const struct cpumask *cpumask,
 const struct flush_tlb_info *info);
 
 static inline void __flush_tlb_local(void)
@@ -68,10 +68,10 @@ static inline void __flush_tlb_one_user(unsigned long addr)
PVOP_VCALL1(mmu.flush_tlb_one_user, addr);
 }
 
-static inline void __flush_tlb_others(const struct cpumask *cpumask,
+static inline void __flush_tlb_multi(const struct cpumask *cpumask,
  const struct flush_tlb_info *info)
 {
-   PVOP_VCALL2(mmu.flush_tlb_others, cpumask, info);
+   PVOP_VCALL2(mmu.flush_tlb_multi, cpumask, info);
 }
 
 static inline void paravirt_tlb_remove_table(struct mmu_gather *tlb, void 
*table)
diff --git a/arch/x86/include/asm/paravirt_types.h 
b/arch/x86/include/asm/paravirt_types.h
index b6b02b7c19cc..541fe7193526 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -201,8 +201,8 @@ struct pv_mmu_ops {
void (*flush_tlb_user)(void);

[PATCH v5 0/8] x86/tlb: Concurrent TLB flushes

2021-02-09 Thread Nadav Amit

From: Nadav Amit 

This is a respin of a rebased version of an old series, which I did not
follow, as I was preoccupied with personal issues (sorry).

The series improve TLB shootdown by flushing the local TLB concurrently
with remote TLBs, overlapping the IPI delivery time with the local
flush. Performance numbers can be found in the previous version [1].

The patches are essentially the same, but rebasing on the last version
required some changes. I left the reviewed-by tags - if anyone considers
it inappropriate, please let me know (and you have my apology).

[1] https://lore.kernel.org/lkml/20190823224153.15223-1-na...@vmware.com/

v4 -> v5:
* Rebase on 5.11
* Move concurrent smp logic to smp_call_function_many_cond() 
* Remove SGI-UV patch which is not needed anymore

v3 -> v4:
* Merge flush_tlb_func_local and flush_tlb_func_remote() [Peter]
* Prevent preemption on_each_cpu(). It is not needed, but it prevents
  concerns. [Peter/tglx]
* Adding acked-, review-by tags

v2 -> v3:
* Open-code the remote/local-flush decision code [Andy]
* Fix hyper-v, Xen implementations [Andrew]
* Fix redundant TLB flushes.

v1 -> v2:
* Removing the patches that Thomas took [tglx]
* Adding hyper-v, Xen compile-tested implementations [Dave]
* Removing UV [Andy]
* Adding lazy optimization, removing inline keyword [Dave]
* Restructuring patch-set

RFCv2 -> v1:
* Fix comment on flush_tlb_multi [Juergen]
* Removing async invalidation optimizations [Andy]
* Adding KVM support [Paolo]

Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Boris Ostrovsky 
Cc: Dave Hansen 
Cc: Haiyang Zhang 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Juergen Gross 
Cc: "K. Y. Srinivasan" 
Cc: Paolo Bonzini 
Cc: Peter Zijlstra 
Cc: Rik van Riel 
Cc: Sasha Levin 
Cc: Stephen Hemminger 
Cc: Thomas Gleixner 
Cc: k...@vger.kernel.org
Cc: linux-hyp...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org

Nadav Amit (8):
  smp: Run functions concurrently in smp_call_function_many_cond()
  x86/mm/tlb: Unify flush_tlb_func_local() and flush_tlb_func_remote()
  x86/mm/tlb: Open-code on_each_cpu_cond_mask() for tlb_is_not_lazy()
  x86/mm/tlb: Flush remote and local TLBs concurrently
  x86/mm/tlb: Privatize cpu_tlbstate
  x86/mm/tlb: Do not make is_lazy dirty for no reason
  cpumask: Mark functions as pure
  x86/mm/tlb: Remove unnecessary uses of the inline keyword

 arch/x86/hyperv/mmu.c |  10 +-
 arch/x86/include/asm/paravirt.h   |   6 +-
 arch/x86/include/asm/paravirt_types.h |   4 +-
 arch/x86/include/asm/tlbflush.h   |  48 +++
 arch/x86/include/asm/trace/hyperv.h   |   2 +-
 arch/x86/kernel/alternative.c |   2 +-
 arch/x86/kernel/kvm.c |  11 +-
 arch/x86/mm/init.c|   2 +-
 arch/x86/mm/tlb.c | 177 +++---
 arch/x86/xen/mmu_pv.c |  11 +-
 include/linux/cpumask.h   |   6 +-
 include/trace/events/xen.h|   2 +-
 kernel/smp.c  | 148 +++--
 13 files changed, 242 insertions(+), 187 deletions(-)

-- 
2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 3/4] virtio-net: support transmit timestamp

2021-02-09 Thread Michael S. Tsirkin

On Tue, Feb 09, 2021 at 01:45:11PM +0800, Jason Wang wrote:
> 
> On 2021/2/9 上午2:55, Willem de Bruijn wrote:
> > From: Willem de Bruijn 
> > 
> > Add optional PTP hardware tx timestamp offload for virtio-net.
> > 
> > Accurate RTT measurement requires timestamps close to the wire.
> > Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
> > equivalent to VIRTIO_NET_F_RX_TSTAMP.
> > 
> > The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
> > returned on completion. If the feature is negotiated, the device
> > either places the timestamp or clears the feature bit.
> > 
> > The timestamp straddles (virtual) hardware domains. Like PTP, use
> > international atomic time (CLOCK_TAI) as global clock base. The driver
> > must sync with the device, e.g., through kvm-clock.
> > 
> > Modify can_push to ensure that on tx completion the header, and thus
> > timestamp, is in a predicatable location at skb_vnet_hdr.
> > 
> > RFC: this implementation relies on the device writing to the buffer.
> > That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
> > The virtio changes should be a separate patch at the least.
> > 
> > Tested: modified txtimestamp.c to with h/w timestamping:
> >-   sock_opt = SOF_TIMESTAMPING_SOFTWARE |
> >+   sock_opt = SOF_TIMESTAMPING_RAW_HARDWARE |
> >+ do_test(family, SOF_TIMESTAMPING_TX_HARDWARE);
> > 
> > Signed-off-by: Willem de Bruijn 
> > ---
> >   drivers/net/virtio_net.c| 61 -
> >   drivers/virtio/virtio_ring.c|  3 +-
> >   include/linux/virtio.h  |  1 +
> >   include/uapi/linux/virtio_net.h |  1 +
> >   4 files changed, 56 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index ac44c5efa0bc..fc8ecd3a333a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -210,6 +210,12 @@ struct virtnet_info {
> > /* Device will pass rx timestamp. Requires has_rx_tstamp */
> > bool enable_rx_tstamp;
> > +   /* Device can pass CLOCK_TAI transmit time to the driver */
> > +   bool has_tx_tstamp;
> > +
> > +   /* Device will pass tx timestamp. Requires has_tx_tstamp */
> > +   bool enable_tx_tstamp;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> > @@ -1401,6 +1407,20 @@ static int virtnet_receive(struct receive_queue *rq, 
> > int budget,
> > return stats.packets;
> >   }
> > +static void virtnet_record_tx_tstamp(const struct send_queue *sq,
> > +struct sk_buff *skb)
> > +{
> > +   const struct virtio_net_hdr_hash_ts *h = skb_vnet_hdr_ht(skb);
> > +   const struct virtnet_info *vi = sq->vq->vdev->priv;
> > +   struct skb_shared_hwtstamps ts;
> > +
> > +   if (h->hdr.flags & VIRTIO_NET_HDR_F_TSTAMP &&
> > +   vi->enable_tx_tstamp) {
> > +   ts.hwtstamp = ns_to_ktime(le64_to_cpu(h->tstamp));
> > +   skb_tstamp_tx(skb, );
> 
> 
> This probably won't work since the buffer is read-only from the device. (See
> virtqueue_add_outbuf()).
> 
> Another issue that I vaguely remember that the virtio spec forbids out
> buffer after in buffer.

Both Driver Requirements: Message Framing and Driver Requirements: 
Scatter-Gather Support
have this statement:

The driver MUST place any device-writable descriptor elements after any 
device-readable descriptor ele-
ments.


similarly

Device Requirements: The Virtqueue Descriptor Table
A device MUST NOT write to a device-readable buffer, and a device 
SHOULD NOT read a device-writable
buffer.



> 
> > +   }
> > +}
> > +
> >   static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
> >   {
> > unsigned int len;
> > @@ -1412,6 +1432,7 @@ static void free_old_xmit_skbs(struct send_queue *sq, 
> > bool in_napi)
> > if (likely(!is_xdp_frame(ptr))) {
> > struct sk_buff *skb = ptr;
> > +   virtnet_record_tx_tstamp(sq, skb);
> > pr_debug("Sent skb %p\n", skb);
> > bytes += skb->len;
> > @@ -1558,7 +1579,7 @@ static int xmit_skb(struct send_queue *sq, struct 
> > sk_buff *skb)
> > struct virtio_net_hdr_mrg_rxbuf *hdr;
> > const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> > struct virtnet_info *vi = sq->vq->vdev->priv;
> > -   struct virtio_net_hdr_v1_hash *ht;
> > +   struct virtio_net_hdr_hash_ts *ht;
> > int num_sg;
> > unsigned hdr_len = vi->hdr_len;
> > bool can_push;
> > @@ -1567,7 +1588,8 @@ static int xmit_skb(struct send_queue *sq, struct 
> > sk_buff *skb)
> > can_push = vi->any_header_sg &&
> > !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
> > -   !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;
> > +   !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len &&
> > +   !vi->enable_tx_tstamp;
> > /* Even if we can, don't push here yet as this would skew
> >  *

Re: [PATCH 02/13] target: split target_submit_cmd_map_sgls

2021-02-09 Thread kernel test robot

Hi Mike,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on vhost/linux-next v5.11-rc7 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Mike-Christie/target-move-t_task_cdb-initialization/20210209-213926
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: h8300-randconfig-s031-20210209 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-215-g0fb77bb6-dirty
# 
https://github.com/0day-ci/linux/commit/3382952197b63f11c166ff293f819ce6ac4d52ae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Mike-Christie/target-move-t_task_cdb-initialization/20210209-213926
git checkout 3382952197b63f11c166ff293f819ce6ac4d52ae
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=h8300 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 


"sparse warnings: (new ones prefixed by >>)"
>> drivers/target/target_core_transport.c:1695:12: sparse: sparse: incorrect 
>> type in assignment (different base types) @@ expected int rc @@ got 
>> restricted sense_reason_t @@
   drivers/target/target_core_transport.c:1695:12: sparse: expected int rc
   drivers/target/target_core_transport.c:1695:12: sparse: got restricted 
sense_reason_t
   drivers/target/target_core_transport.c:1699:12: sparse: sparse: incorrect 
type in assignment (different base types) @@ expected int rc @@ got 
restricted sense_reason_t @@
   drivers/target/target_core_transport.c:1699:12: sparse: expected int rc
   drivers/target/target_core_transport.c:1699:12: sparse: got restricted 
sense_reason_t
>> drivers/target/target_core_transport.c:1736:51: sparse: sparse: incorrect 
>> type in argument 2 (different base types) @@ expected restricted 
>> sense_reason_t [usertype] @@ got int rc @@
   drivers/target/target_core_transport.c:1736:51: sparse: expected 
restricted sense_reason_t [usertype]
   drivers/target/target_core_transport.c:1736:51: sparse: got int rc

vim +1695 drivers/target/target_core_transport.c

  1678  
  1679  /**
  1680   * target_submit - perform final initialization and submit cmd to LIO 
core
  1681   * @se_cmd: command descriptor to submit
  1682   * @cdb: pointer to SCSI CDB
  1683   *
  1684   * target_submit_prep must have been called on the cmd, and this must be
  1685   * called from process context.
  1686   */
  1687  static void target_submit(struct se_cmd *se_cmd, unsigned char *cdb)
  1688  {
  1689  struct scatterlist *sgl = se_cmd->t_data_sg;
  1690  unsigned char *buf = NULL;
  1691  int rc;
  1692  
  1693  might_sleep();
  1694  
> 1695  rc = target_cmd_init_cdb(se_cmd, cdb);
  1696  if (rc)
  1697  goto fail;
  1698  
  1699  rc = target_cmd_parse_cdb(se_cmd);
  1700  if (rc != 0)
  1701  goto fail;
  1702  
  1703  if (se_cmd->t_data_nents != 0) {
  1704  BUG_ON(!sgl);
  1705  /*
  1706   * A work-around for tcm_loop as some userspace code via
  1707   * scsi-generic do not memset their associated read 
buffers,
  1708   * so go ahead and do that here for type non-data CDBs. 
 Also
  1709   * note that this is currently guaranteed to be a 
single SGL
  1710   * for this case by target core in 
target_setup_cmd_from_cdb()
  1711   * -> transport_generic_cmd_sequencer().
  1712   */
  1713  if (!(se_cmd->se_cmd_flags & SCF_SCSI_DATA_CDB) &&
  1714   se_cmd->data_direction == DMA_FROM_DEVICE) {
  1715  if (sgl)
  1716  buf = kmap(sg_page(sgl)) + sgl->offset;
  1717  
  1718  if (buf) {
  1719  memset(buf, 0, sgl->length);
  1720  kunmap(sg_page(sgl));
  1721  }
  1722  }
  1723  
  1724  }
  1725  
  1726  /*
  1727   * Check if we need to delay processing because of ALUA
  1728   * Active/NonOptimized primary access state..

Re: [PATCH net] virtio-net: suppress bad irq warning for tx napi

2021-02-09 Thread Willem de Bruijn

> >>> I have no preference. Just curious, especially if it complicates the 
> >>> patch.
> >>>
> >> My understanding is that. It's probably ok for net. But we probably need
> >> to document the assumptions to make sure it was not abused in other 
> >> drivers.
> >>
> >> Introduce new parameters for find_vqs() can help to eliminate the subtle
> >> stuffs but I agree it looks like a overkill.
> >>
> >> (Btw, I forget the numbers but wonder how much difference if we simple
> >> remove the free_old_xmits() from the rx NAPI path?)
> > The committed patchset did not record those numbers, but I found them
> > in an earlier iteration:
> >
> >[PATCH net-next 0/3] virtio-net tx napi
> >https://lists.openwall.net/netdev/2017/04/02/55
> >
> > It did seem to significantly reduce compute cycles ("Gcyc") at the
> > time. For instance:
> >
> >  TCP_RR Latency (us):
> >  1x:
> >p50  24   24   21
> >p99  27   27   27
> >Gcycles 299  432  308
> >
> > I'm concerned that removing it now may cause a regression report in a
> > few months. That is higher risk than the spurious interrupt warning
> > that was only reported after years of use.
>
>
> Right.
>
> So if Michael is fine with this approach, I'm ok with it. But we
> probably need to a TODO to invent the interrupt handlers that can be
> used for more than one virtqueues. When MSI-X is enabled, the interrupt
> handler (vring_interrup()) assumes the interrupt is used by a single
> virtqueue.

Thanks.

The approach to schedule tx-napi from virtnet_poll_cleantx instead of
cleaning directly in this rx-napi function was not effective at
suppressing the warning, I understand.

It should be easy to drop the spurious rate to a little under 99%
percent, if only to suppress the warning. By probabilistically
cleaning in virtnet_poll_cleantx only every 98/100, say. But that
really is a hack.

There does seem to be a huge potential for cycle savings if we can
really avoid the many spurious interrupts.

As scheduling napi_tx from virtnet_poll_cleantx is not effective,
agreed that we should probably look at the more complete solution to
handle both tx and rx virtqueues from the same IRQ.

And revert this explicit warning suppression patch once we have that.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 3/4] virtio-net: support transmit timestamp

2021-02-09 Thread Willem de Bruijn

On Tue, Feb 9, 2021 at 4:43 AM Michael S. Tsirkin  wrote:
>
> On Mon, Feb 08, 2021 at 01:55:57PM -0500, Willem de Bruijn wrote:
> > From: Willem de Bruijn 
> >
> > Add optional PTP hardware tx timestamp offload for virtio-net.
> >
> > Accurate RTT measurement requires timestamps close to the wire.
> > Introduce virtio feature VIRTIO_NET_F_TX_TSTAMP, the transmit
> > equivalent to VIRTIO_NET_F_RX_TSTAMP.
> >
> > The driver sets VIRTIO_NET_HDR_F_TSTAMP to request a timestamp
> > returned on completion. If the feature is negotiated, the device
> > either places the timestamp or clears the feature bit.
> >
> > The timestamp straddles (virtual) hardware domains. Like PTP, use
> > international atomic time (CLOCK_TAI) as global clock base. The driver
> > must sync with the device, e.g., through kvm-clock.
> >
> > Modify can_push to ensure that on tx completion the header, and thus
> > timestamp, is in a predicatable location at skb_vnet_hdr.
> >
> > RFC: this implementation relies on the device writing to the buffer.
> > That breaks DMA_TO_DEVICE semantics. For now, disable when DMA is on.
>
> If you do something like this, please do it in the validate
> callback and clear the features you aren't using.

Ah yes. Thanks for the tip. I'll do that ..

.. once I'm sure that this approach of using an outbuf for I/O is
actually allowed behavior. I'm not entirely convinced yet myself.
Jason also pointed out more specific concerns. I'll look into that
further.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v2 2/4] virtio-net: support receive timestamp

2021-02-09 Thread Willem de Bruijn

On Mon, Feb 8, 2021 at 11:13 PM Jason Wang  wrote:
>
>
> On 2021/2/9 上午2:55, Willem de Bruijn wrote:
> > From: Willem de Bruijn 
> >
> > Add optional PTP hardware rx timestamp offload for virtio-net.
> >
> > Accurate RTT measurement requires timestamps close to the wire.
> > Introduce virtio feature VIRTIO_NET_F_RX_TSTAMP. If negotiated, the
> > virtio-net header is expanded with room for a timestamp.
> >
> > A device may pass receive timestamps for all or some packets. Flag
> > VIRTIO_NET_HDR_F_TSTAMP signals whether a timestamp is recorded.
> >
> > A driver that supports hardware timestamping must also support
> > ioctl SIOCSHWTSTAMP. Implement that, as well as information getters
> > ioctl SIOCGHWTSTAMP and ethtool get_ts_info (`ethtool -T $DEV`).
> >
> > The timestamp straddles (virtual) hardware domains. Like PTP, use
> > international atomic time (CLOCK_TAI) as global clock base. The driver
> > must sync with the device, e.g., through kvm-clock.
> >
> > Tested:
> >guest: ./timestamping eth0 \
> >SOF_TIMESTAMPING_RAW_HARDWARE \
> >SOF_TIMESTAMPING_RX_HARDWARE
> >host: nc -4 -u 192.168.1.1 319
> >
> > Changes RFC -> RFCv2
> >- rename virtio_net_hdr_v12 to virtio_net_hdr_hash_ts
> >- add ethtool .get_ts_info to query capabilities
> >- add ioctl SIOC[GS]HWTSTAMP to configure feature
> >- add vi->enable_rx_tstamp to store configuration
> >- convert virtioXX_to_cpu to leXX_to_cpu
> >- convert reserved to __u32
> >
> > Signed-off-by: Willem de Bruijn 

> >   static const struct net_device_ops virtnet_netdev = {
> >   .ndo_open= virtnet_open,
> >   .ndo_stop= virtnet_close,
> > @@ -2573,6 +2676,7 @@ static const struct net_device_ops virtnet_netdev = {
> >   .ndo_features_check = passthru_features_check,
> >   .ndo_get_phys_port_name = virtnet_get_phys_port_name,
> >   .ndo_set_features   = virtnet_set_features,
> > + .ndo_do_ioctl   = virtnet_ioctl,
> >   };
> >
> >   static void virtnet_config_changed_work(struct work_struct *work)
> > @@ -3069,6 +3173,11 @@ static int virtnet_probe(struct virtio_device *vdev)
> >   vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> >   }
> >
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RX_TSTAMP)) {
> > + vi->has_rx_tstamp = true;
> > + vi->hdr_len = sizeof(struct virtio_net_hdr_hash_ts);
>
>
> Does this mean even if the device doesn't pass timestamp, the header
> still contains the timestamp fields.

Yes. As implemented, the size of the header is constant across
packets. If both sides negotiate the feature, then all headers reserve
space, whether or not the specific packet has a timestamp.

So far headers are fixed size. I suppose we could investigate variable
size headers. This goes back to our discussion in the previous
patchset, that we can always add a packed-header feature later, if the
number of optional features reaches a size that makes the complexity
worthwhile.

> > + }
> > +
> >   if (virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT) ||
> >   virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >   vi->any_header_sg = true;
> > @@ -3260,7 +3369,7 @@ static struct virtio_device_id id_table[] = {
> >   VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >   VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> >   VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > - VIRTIO_NET_F_TX_HASH
> > + VIRTIO_NET_F_TX_HASH, VIRTIO_NET_F_RX_TSTAMP
> >
> >   static unsigned int features[] = {
> >   VIRTNET_FEATURES,
> > diff --git a/include/uapi/linux/virtio_net.h 
> > b/include/uapi/linux/virtio_net.h
> > index 273d43c35f59..a5c84410cf92 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -57,6 +57,7 @@
> >* Steering */
> >   #define VIRTIO_NET_F_CTRL_MAC_ADDR 23   /* Set MAC address */
> >
> > +#define VIRTIO_NET_F_RX_TSTAMP 55/* Device sends TAI receive 
> > time */
> >   #define VIRTIO_NET_F_TX_HASH  56/* Driver sends hash report */
> >   #define VIRTIO_NET_F_HASH_REPORT  57/* Supports hash report */
> >   #define VIRTIO_NET_F_RSS  60/* Supports RSS RX steering */
> > @@ -126,6 +127,7 @@ struct virtio_net_hdr_v1 {
> >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1   /* Use csum_start, 
> > csum_offset */
> >   #define VIRTIO_NET_HDR_F_DATA_VALID 2   /* Csum is valid */
> >   #define VIRTIO_NET_HDR_F_RSC_INFO   4   /* rsc info in csum_ fields */
> > +#define VIRTIO_NET_HDR_F_TSTAMP  8   /* timestamp is 
> > recorded */
> >   __u8 flags;
> >   #define VIRTIO_NET_HDR_GSO_NONE 0   /* Not a GSO frame */
> >   #define VIRTIO_NET_HDR_GSO_TCPV41   /* GSO frame, IPv4 TCP (TSO) 
> > */
> > @@ -181,6 +183,17 @@ struct virtio_net_hdr_v1_hash {
> >   };
> >   };
> >
> > +struct virtio_net_hdr_hash_ts {
> >

Re: [RFC PATCH 05/18] virt/mshv: create partition ioctl

2021-02-09 Thread Vitaly Kuznetsov

Nuno Das Neves  writes:

> Add MSHV_CREATE_PARTITION, which creates an fd to track a new partition.
> Partition is not yet created in the hypervisor itself.
> Introduce header files for userspace-facing hyperv structures.
>
> Co-developed-by: Lillian Grassin-Drake 
> Signed-off-by: Lillian Grassin-Drake 
> Signed-off-by: Nuno Das Neves 
> ---
>  Documentation/virt/mshv/api.rst |  12 ++
>  arch/x86/include/asm/hyperv-tlfs.h  |   1 +
>  arch/x86/include/uapi/asm/hyperv-tlfs.h | 124 
>  include/asm-generic/hyperv-tlfs.h   |   1 +
>  include/linux/mshv.h|  16 +++
>  include/uapi/asm-generic/hyperv-tlfs.h  |  14 ++
>  include/uapi/linux/mshv.h   |   7 +
>  virt/mshv/mshv_main.c   | 179 +---
>  8 files changed, 338 insertions(+), 16 deletions(-)
>  create mode 100644 arch/x86/include/uapi/asm/hyperv-tlfs.h
>  create mode 100644 include/uapi/asm-generic/hyperv-tlfs.h
>
> diff --git a/Documentation/virt/mshv/api.rst b/Documentation/virt/mshv/api.rst
> index 82e32de48d03..ce651a1738e0 100644
> --- a/Documentation/virt/mshv/api.rst
> +++ b/Documentation/virt/mshv/api.rst
> @@ -39,6 +39,9 @@ root partition can use mshv APIs to create guest partitions.
>  
>  The module is named mshv and can be configured with CONFIG_HYPERV_ROOT_API.
>  
> +The uapi header files you need are linux/mshv.h, asm/hyperv-tlfs.h, and
> +asm-generic/hyperv-tlfs.h.
> +
>  Mshv is file descriptor-based, following a similar pattern to KVM.
>  
>  To get a handle to the mshv driver, use open("/dev/mshv").
> @@ -60,3 +63,12 @@ if one of them matches.
>  This /dev/mshv file descriptor will remain 'locked' to that version as long 
> as
>  it is open - this ioctl can only be called once per open.
>  
> +3.2 MSHV_CREATE_PARTITION
> +-
> +:Type: /dev/mshv ioctl
> +:Parameters: struct mshv_create_partition
> +:Returns: partition file descriptor, or -1 on failure
> +
> +This ioctl creates a guest partition, returning a file descriptor to use as a
> +handle for partition ioctls.
> +
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index 592c75e51e0f..4cd44ae9bffb 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -11,6 +11,7 @@
>  
>  #include 
>  #include 
> +#include 
>  /*
>   * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent
>   * is set by CPUID(HvCpuIdFunctionVersionAndFeatures).
> diff --git a/arch/x86/include/uapi/asm/hyperv-tlfs.h 
> b/arch/x86/include/uapi/asm/hyperv-tlfs.h
> new file mode 100644
> index ..72150c25ffe6
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/hyperv-tlfs.h
> @@ -0,0 +1,124 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_ASM_X86_HYPERV_TLFS_USER_H
> +#define _UAPI_ASM_X86_HYPERV_TLFS_USER_H
> +
> +#include 
> +
> +#define HV_PARTITION_PROCESSOR_FEATURE_BANKS 2
> +
> +union hv_partition_processor_features {
> + struct {
> + __u64 sse3_support:1;
> + __u64 lahf_sahf_support:1;
> + __u64 ssse3_support:1;
> + __u64 sse4_1_support:1;
> + __u64 sse4_2_support:1;
> + __u64 sse4a_support:1;
> + __u64 xop_support:1;
> + __u64 pop_cnt_support:1;
> + __u64 cmpxchg16b_support:1;
> + __u64 altmovcr8_support:1;
> + __u64 lzcnt_support:1;
> + __u64 mis_align_sse_support:1;
> + __u64 mmx_ext_support:1;
> + __u64 amd3dnow_support:1;
> + __u64 extended_amd3dnow_support:1;
> + __u64 page_1gb_support:1;
> + __u64 aes_support:1;
> + __u64 pclmulqdq_support:1;
> + __u64 pcid_support:1;
> + __u64 fma4_support:1;
> + __u64 f16c_support:1;
> + __u64 rd_rand_support:1;
> + __u64 rd_wr_fs_gs_support:1;
> + __u64 smep_support:1;
> + __u64 enhanced_fast_string_support:1;
> + __u64 bmi1_support:1;
> + __u64 bmi2_support:1;
> + __u64 hle_support_deprecated:1;
> + __u64 rtm_support_deprecated:1;
> + __u64 movbe_support:1;
> + __u64 npiep1_support:1;
> + __u64 dep_x87_fpu_save_support:1;
> + __u64 rd_seed_support:1;
> + __u64 adx_support:1;
> + __u64 intel_prefetch_support:1;
> + __u64 smap_support:1;
> + __u64 hle_support:1;
> + __u64 rtm_support:1;
> + __u64 rdtscp_support:1;
> + __u64 clflushopt_support:1;
> + __u64 clwb_support:1;
> + __u64 sha_support:1;
> + __u64 x87_pointers_saved_support:1;
> + __u64 invpcid_support:1;
> + __u64 ibrs_support:1;
> + __u64 stibp_support:1;
> + __u64 ibpb_support: 1;
> +

Re: [RFC PATCH 01/18] x86/hyperv: convert hyperv statuses to linux error codes

2021-02-09 Thread Vitaly Kuznetsov

Nuno Das Neves  writes:

> Return linux-friendly error codes from hypercall wrapper functions.
> This will be needed in the mshv module.
>
> Signed-off-by: Nuno Das Neves 
> ---
>  arch/x86/hyperv/hv_proc.c | 30 ++---
>  arch/x86/include/asm/mshyperv.h   |  1 +
>  include/asm-generic/hyperv-tlfs.h | 32 +--
>  3 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_proc.c b/arch/x86/hyperv/hv_proc.c
> index 0fd972c9129a..8f86f8e86748 100644
> --- a/arch/x86/hyperv/hv_proc.c
> +++ b/arch/x86/hyperv/hv_proc.c
> @@ -18,6 +18,30 @@
>  #define HV_DEPOSIT_MAX_ORDER (8)
>  #define HV_DEPOSIT_MAX (1 << HV_DEPOSIT_MAX_ORDER)
>  
> +int hv_status_to_errno(int hv_status)
> +{
> + switch (hv_status) {
> + case HV_STATUS_SUCCESS:
> + return 0;
> + case HV_STATUS_INVALID_PARAMETER:
> + case HV_STATUS_UNKNOWN_PROPERTY:
> + case HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE:
> + case HV_STATUS_INVALID_VP_INDEX:
> + case HV_STATUS_INVALID_REGISTER_VALUE:
> + case HV_STATUS_INVALID_LP_INDEX:
> + return EINVAL;
> + case HV_STATUS_ACCESS_DENIED:
> + case HV_STATUS_OPERATION_DENIED:
> + return EACCES;
> + case HV_STATUS_NOT_ACKNOWLEDGED:
> + case HV_STATUS_INVALID_VP_STATE:
> + case HV_STATUS_INVALID_PARTITION_STATE:
> + return EBADFD;
> + }
> + return ENOTRECOVERABLE;
> +}
> +EXPORT_SYMBOL_GPL(hv_status_to_errno);
> +
>  /*
>   * Deposits exact number of pages
>   * Must be called with interrupts enabled
> @@ -99,7 +123,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 
> num_pages)
>  
>   if (status != HV_STATUS_SUCCESS) {
>   pr_err("Failed to deposit pages: %d\n", status);
> - ret = status;
> + ret = -hv_status_to_errno(status);

"-hv_status_to_errno" looks weird, could we just return
'-EINVAL'/'-EACCES'/... from hv_status_to_errno() instead?

>   goto err_free_allocations;
>   }
>  
> @@ -155,7 +179,7 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 
> apic_id)
>   if (status != HV_STATUS_SUCCESS) {
>   pr_err("%s: cpu %u apic ID %u, %d\n", __func__,
>  lp_index, apic_id, status);
> - ret = status;
> + ret = -hv_status_to_errno(status);
>   }
>   break;
>   }
> @@ -203,7 +227,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 
> vp_index, u32 flags)
>   if (status != HV_STATUS_SUCCESS) {
>   pr_err("%s: vcpu %u, lp %u, %d\n", __func__,
>  vp_index, flags, status);
> - ret = status;
> + ret = -hv_status_to_errno(status);
>   }
>   break;
>   }
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index cbee72550a12..eb75faa4d4c5 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -243,6 +243,7 @@ int hyperv_flush_guest_mapping_range(u64 as,
>  int hyperv_fill_flush_guest_mapping_list(
>   struct hv_guest_mapping_flush_list *flush,
>   u64 start_gfn, u64 end_gfn);
> +int hv_status_to_errno(int hv_status);
>  
>  extern bool hv_root_partition;
>  
> diff --git a/include/asm-generic/hyperv-tlfs.h 
> b/include/asm-generic/hyperv-tlfs.h
> index dd385c6a71b5..445244192fa4 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -181,16 +181,28 @@ enum HV_GENERIC_SET_FORMAT {
>  #define HV_HYPERCALL_REP_START_MASK  GENMASK_ULL(59, 48)
>  
>  /* hypercall status code */
> -#define HV_STATUS_SUCCESS0
> -#define HV_STATUS_INVALID_HYPERCALL_CODE 2
> -#define HV_STATUS_INVALID_HYPERCALL_INPUT3
> -#define HV_STATUS_INVALID_ALIGNMENT  4
> -#define HV_STATUS_INVALID_PARAMETER  5
> -#define HV_STATUS_OPERATION_DENIED   8
> -#define HV_STATUS_INSUFFICIENT_MEMORY11
> -#define HV_STATUS_INVALID_PORT_ID17
> -#define HV_STATUS_INVALID_CONNECTION_ID  18
> -#define HV_STATUS_INSUFFICIENT_BUFFERS   19
> +#define HV_STATUS_SUCCESS0x0
> +#define HV_STATUS_INVALID_HYPERCALL_CODE 0x2
> +#define HV_STATUS_INVALID_HYPERCALL_INPUT0x3
> +#define HV_STATUS_INVALID_ALIGNMENT  0x4
> +#define HV_STATUS_INVALID_PARAMETER  0x5
> +#define HV_STATUS_ACCESS_DENIED  0x6
> +#define HV_STATUS_INVALID_PARTITION_STATE0x7
> +#define HV_STATUS_OPERATION_DENIED   0x8
> +#define HV_STATUS_UNKNOWN_PROPERTY   0x9
> +#define HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE0xA
> +#define

[PATCH v3 9/9] ALSA: virtio: introduce device suspend/resume support

2021-02-09 Thread Anton Yakovlev

All running PCM substreams are stopped on device suspend and restarted
on device resume.

Signed-off-by: Anton Yakovlev 
---
 sound/virtio/virtio_card.c| 57 +++
 sound/virtio/virtio_pcm.c |  1 +
 sound/virtio/virtio_pcm_ops.c | 44 ---
 3 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 787a4dec1da8..1f0a0fa7bbc0 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -373,6 +373,59 @@ static void virtsnd_config_changed(struct virtio_device 
*vdev)
 "sound device configuration was changed\n");
 }
 
+#ifdef CONFIG_PM_SLEEP
+/**
+ * virtsnd_freeze() - Suspend device.
+ * @vdev: VirtIO parent device.
+ *
+ * Context: Any context.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_freeze(struct virtio_device *vdev)
+{
+   struct virtio_snd *snd = vdev->priv;
+
+   /* Stop all the virtqueues. */
+   vdev->config->reset(vdev);
+   vdev->config->del_vqs(vdev);
+
+   virtsnd_ctl_msg_cancel_all(snd);
+
+   kfree(snd->event_msgs);
+
+   /*
+* If the virtsnd_restore() fails before re-allocating events, then we
+* get a dangling pointer here.
+*/
+   snd->event_msgs = NULL;
+
+   return 0;
+}
+
+/**
+ * virtsnd_restore() - Resume device.
+ * @vdev: VirtIO parent device.
+ *
+ * Context: Any context.
+ * Return: 0 on success, -errno on failure.
+ */
+static int virtsnd_restore(struct virtio_device *vdev)
+{
+   struct virtio_snd *snd = vdev->priv;
+   int rc;
+
+   rc = virtsnd_find_vqs(snd);
+   if (rc)
+   return rc;
+
+   virtio_device_ready(vdev);
+
+   virtsnd_enable_event_vq(snd);
+
+   return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
 static const struct virtio_device_id id_table[] = {
{ VIRTIO_ID_SOUND, VIRTIO_DEV_ANY_ID },
{ 0 },
@@ -386,6 +439,10 @@ static struct virtio_driver virtsnd_driver = {
.probe = virtsnd_probe,
.remove = virtsnd_remove,
.config_changed = virtsnd_config_changed,
+#ifdef CONFIG_PM_SLEEP
+   .freeze = virtsnd_freeze,
+   .restore = virtsnd_restore,
+#endif
 };
 
 static int __init init(void)
diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
index 1d98de878385..401d4c975d2b 100644
--- a/sound/virtio/virtio_pcm.c
+++ b/sound/virtio/virtio_pcm.c
@@ -109,6 +109,7 @@ static int virtsnd_pcm_build_hw(struct virtio_pcm_substream 
*vss,
SNDRV_PCM_INFO_BATCH |
SNDRV_PCM_INFO_BLOCK_TRANSFER |
SNDRV_PCM_INFO_INTERLEAVED |
+   SNDRV_PCM_INFO_RESUME |
SNDRV_PCM_INFO_PAUSE;
 
if (!info->channels_min || info->channels_min > info->channels_max) {
diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
index c2224f5461c4..207f4877a5ec 100644
--- a/sound/virtio/virtio_pcm_ops.c
+++ b/sound/virtio/virtio_pcm_ops.c
@@ -220,6 +220,10 @@ static int virtsnd_pcm_hw_params(struct snd_pcm_substream 
*substream,
if (rc)
return rc;
 
+   /* If messages have already been allocated before, do nothing. */
+   if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED)
+   return 0;
+
return virtsnd_pcm_msg_alloc(vss, periods, period_bytes);
 }
 
@@ -260,19 +264,21 @@ static int virtsnd_pcm_prepare(struct snd_pcm_substream 
*substream)
}
 
spin_lock_irqsave(>lock, flags);
-   /*
-* Since I/O messages are asynchronous, they can be completed
-* when the runtime structure no longer exists. Since each
-* completion implies incrementing the hw_ptr, we cache all the
-* current values needed to compute the new hw_ptr value.
-*/
-   vss->frame_bytes = runtime->frame_bits >> 3;
-   vss->period_size = runtime->period_size;
-   vss->buffer_size = runtime->buffer_size;
+   if (runtime->status->state != SNDRV_PCM_STATE_SUSPENDED) {
+   /*
+* Since I/O messages are asynchronous, they can be completed
+* when the runtime structure no longer exists. Since each
+* completion implies incrementing the hw_ptr, we cache all the
+* current values needed to compute the new hw_ptr value.
+*/
+   vss->frame_bytes = runtime->frame_bits >> 3;
+   vss->period_size = runtime->period_size;
+   vss->buffer_size = runtime->buffer_size;
 
-   vss->hw_ptr = 0;
+   vss->hw_ptr = 0;
+   vss->msg_last_enqueued = -1;
+   }
vss->xfer_xrun = false;
-   vss->msg_last_enqueued = -1;
vss->msg_count = 0;
spin_unlock_irqrestore(>lock, flags);
 
@@ -302,6 +308,21 @@ static int virtsnd_pcm_trigger(struct snd_pcm_substream 
*substream, int command)
int rc;
 
switch (command) {
+

[PATCH v3 5/9] ALSA: virtio: handling control and I/O messages for the PCM device

2021-02-09 Thread Anton Yakovlev

The driver implements a message-based transport for I/O substream
operations. Before the start of the substream, the hardware buffer is
sliced into I/O messages, the number of which is equal to the current
number of periods. The size of each message is equal to the current
size of one period.

I/O messages are organized in an ordered queue. The completion of the
I/O message indicates an elapsed period (the only exception is the end
of the stream for the capture substream). Upon completion, the message
is automatically re-added to the end of the queue.

Signed-off-by: Anton Yakovlev 
---
 sound/virtio/Makefile |   3 +-
 sound/virtio/virtio_card.c|  18 +-
 sound/virtio/virtio_card.h|   9 +
 sound/virtio/virtio_pcm.c |  32 +++
 sound/virtio/virtio_pcm.h |  42 
 sound/virtio/virtio_pcm_msg.c | 393 ++
 6 files changed, 494 insertions(+), 3 deletions(-)
 create mode 100644 sound/virtio/virtio_pcm_msg.c

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index 69162a545a41..626af3cc3ed7 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
 virtio_snd-objs := \
virtio_card.o \
virtio_ctl_msg.o \
-   virtio_pcm.o
+   virtio_pcm.o \
+   virtio_pcm_msg.o
 
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 235afc25fce7..a845978111d6 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -65,8 +65,16 @@ static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
spin_lock_irqsave(>lock, flags);
do {
virtqueue_disable_cb(vqueue);
-   while ((event = virtqueue_get_buf(vqueue, )))
+   while ((event = virtqueue_get_buf(vqueue, ))) {
+   switch (le32_to_cpu(event->hdr.code)) {
+   case VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED:
+   case VIRTIO_SND_EVT_PCM_XRUN:
+   virtsnd_pcm_event(snd, event);
+   break;
+   }
+
virtsnd_event_send(vqueue, event, true, GFP_ATOMIC);
+   }
if (unlikely(virtqueue_is_broken(vqueue)))
break;
} while (!virtqueue_enable_cb(vqueue));
@@ -87,7 +95,9 @@ static int virtsnd_find_vqs(struct virtio_snd *snd)
struct virtio_device *vdev = snd->vdev;
vq_callback_t *callbacks[VIRTIO_SND_VQ_MAX] = {
[VIRTIO_SND_VQ_CONTROL] = virtsnd_ctl_notify_cb,
-   [VIRTIO_SND_VQ_EVENT] = virtsnd_event_notify_cb
+   [VIRTIO_SND_VQ_EVENT] = virtsnd_event_notify_cb,
+   [VIRTIO_SND_VQ_TX] = virtsnd_pcm_tx_notify_cb,
+   [VIRTIO_SND_VQ_RX] = virtsnd_pcm_rx_notify_cb
};
const char *names[VIRTIO_SND_VQ_MAX] = {
[VIRTIO_SND_VQ_CONTROL] = "virtsnd-ctl",
@@ -299,6 +309,7 @@ static int virtsnd_probe(struct virtio_device *vdev)
 static void virtsnd_remove(struct virtio_device *vdev)
 {
struct virtio_snd *snd = vdev->priv;
+   unsigned int i;
 
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
@@ -310,6 +321,9 @@ static void virtsnd_remove(struct virtio_device *vdev)
 
vdev->config->del_vqs(vdev);
 
+   for (i = 0; snd->substreams && i < snd->nsubstreams; ++i)
+   virtsnd_pcm_msg_free(>substreams[i]);
+
kfree(snd->event_msgs);
 }
 
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index 687d3ed6d1c3..aca87059564e 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -81,4 +81,13 @@ virtsnd_rx_queue(struct virtio_snd *snd)
return >queues[VIRTIO_SND_VQ_RX];
 }
 
+static inline struct virtio_snd_queue *
+virtsnd_pcm_queue(struct virtio_pcm_substream *vss)
+{
+   if (vss->direction == SNDRV_PCM_STREAM_PLAYBACK)
+   return virtsnd_tx_queue(vss->snd);
+   else
+   return virtsnd_rx_queue(vss->snd);
+}
+
 #endif /* VIRTIO_SND_CARD_H */
diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
index 92f2f4cb338f..a74fbfb9f35c 100644
--- a/sound/virtio/virtio_pcm.c
+++ b/sound/virtio/virtio_pcm.c
@@ -338,6 +338,8 @@ int virtsnd_pcm_parse_cfg(struct virtio_snd *snd)
 
vss->snd = snd;
vss->sid = i;
+   init_waitqueue_head(>msg_empty);
+   spin_lock_init(>lock);
 
rc = virtsnd_pcm_build_hw(vss, [i]);
if (rc)
@@ -462,3 +464,33 @@ int virtsnd_pcm_build_devs(struct virtio_snd *snd)
 
return 0;
 }
+
+/**
+ * virtsnd_pcm_event() - Handle the PCM device event notification.
+ * @snd: VirtIO sound device.
+ * @event: VirtIO sound event.
+ *
+ * Context: Interrupt context.
+ */
+void virtsnd_pcm_event(struct virtio_snd *snd, struct virtio_snd_event *event)
+{
+   struct virtio_pcm_substream *vss;
+   unsigned int

[PATCH v3 4/9] ALSA: virtio: build PCM devices and substream hardware descriptors

2021-02-09 Thread Anton Yakovlev

Like the HDA specification, the virtio sound device specification links
PCM substreams, jacks and PCM channel maps into functional groups. For
each discovered group, a PCM device is created, the number of which
coincides with the group number.

Introduce the module parameters for setting the hardware buffer
parameters:
  pcm_buffer_ms [=160]
  pcm_periods_min [=2]
  pcm_periods_max [=16]
  pcm_period_ms_min [=10]
  pcm_period_ms_max [=80]

Signed-off-by: Anton Yakovlev 
---
 sound/virtio/Makefile  |   3 +-
 sound/virtio/virtio_card.c |  14 ++
 sound/virtio/virtio_card.h |  10 +
 sound/virtio/virtio_pcm.c  | 464 +
 sound/virtio/virtio_pcm.h  |  71 ++
 5 files changed, 561 insertions(+), 1 deletion(-)
 create mode 100644 sound/virtio/virtio_pcm.c
 create mode 100644 sound/virtio/virtio_pcm.h

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index dc551e637441..69162a545a41 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
 
 virtio_snd-objs := \
virtio_card.o \
-   virtio_ctl_msg.o
+   virtio_ctl_msg.o \
+   virtio_pcm.o
 
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index c62b3a2da148..235afc25fce7 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -200,6 +200,16 @@ static int virtsnd_build_devs(struct virtio_snd *snd)
 VIRTIO_SND_CARD_NAME " at %s/%s",
 dev_name(dev->parent), dev_name(dev));
 
+   rc = virtsnd_pcm_parse_cfg(snd);
+   if (rc)
+   return rc;
+
+   if (snd->nsubstreams) {
+   rc = virtsnd_pcm_build_devs(snd);
+   if (rc)
+   return rc;
+   }
+
return snd_card_register(snd->card);
 }
 
@@ -228,6 +238,9 @@ static int virtsnd_validate(struct virtio_device *vdev)
return -EINVAL;
}
 
+   if (virtsnd_pcm_validate(vdev))
+   return -EINVAL;
+
return 0;
 }
 
@@ -251,6 +264,7 @@ static int virtsnd_probe(struct virtio_device *vdev)
snd->vdev = vdev;
INIT_WORK(>reset_work, virtsnd_reset_fn);
INIT_LIST_HEAD(>ctl_msgs);
+   INIT_LIST_HEAD(>pcm_list);
 
vdev->priv = snd;
 
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index c51a71a79388..687d3ed6d1c3 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -12,9 +12,13 @@
 #include 
 
 #include "virtio_ctl_msg.h"
+#include "virtio_pcm.h"
 
 #define VIRTIO_SND_CARD_DRIVER "virtio-snd"
 #define VIRTIO_SND_CARD_NAME   "VirtIO SoundCard"
+#define VIRTIO_SND_PCM_NAME"VirtIO PCM"
+
+struct virtio_pcm_substream;
 
 /**
  * struct virtio_snd_queue - Virtqueue wrapper structure.
@@ -34,6 +38,9 @@ struct virtio_snd_queue {
  * @card: ALSA sound card.
  * @ctl_msgs: Pending control request list.
  * @event_msgs: Device events.
+ * @pcm_list: VirtIO PCM device list.
+ * @substreams: VirtIO PCM substreams.
+ * @nsubstreams: Number of PCM substreams.
  */
 struct virtio_snd {
struct virtio_device *vdev;
@@ -42,6 +49,9 @@ struct virtio_snd {
struct snd_card *card;
struct list_head ctl_msgs;
struct virtio_snd_event *event_msgs;
+   struct list_head pcm_list;
+   struct virtio_pcm_substream *substreams;
+   unsigned int nsubstreams;
 };
 
 /* Message completion timeout in milliseconds (module parameter). */
diff --git a/sound/virtio/virtio_pcm.c b/sound/virtio/virtio_pcm.c
new file mode 100644
index ..92f2f4cb338f
--- /dev/null
+++ b/sound/virtio/virtio_pcm.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * virtio-snd: Virtio sound device
+ * Copyright (C) 2021 OpenSynergy GmbH
+ */
+#include 
+#include 
+
+#include "virtio_card.h"
+
+static unsigned int pcm_buffer_ms = 160;
+module_param(pcm_buffer_ms, uint, 0644);
+MODULE_PARM_DESC(pcm_buffer_ms, "PCM substream buffer time in milliseconds");
+
+static unsigned int pcm_periods_min = 2;
+module_param(pcm_periods_min, uint, 0644);
+MODULE_PARM_DESC(pcm_periods_min, "Minimum number of PCM periods");
+
+static unsigned int pcm_periods_max = 16;
+module_param(pcm_periods_max, uint, 0644);
+MODULE_PARM_DESC(pcm_periods_max, "Maximum number of PCM periods");
+
+static unsigned int pcm_period_ms_min = 10;
+module_param(pcm_period_ms_min, uint, 0644);
+MODULE_PARM_DESC(pcm_period_ms_min, "Minimum PCM period time in milliseconds");
+
+static unsigned int pcm_period_ms_max = 80;
+module_param(pcm_period_ms_max, uint, 0644);
+MODULE_PARM_DESC(pcm_period_ms_max, "Maximum PCM period time in milliseconds");
+
+/* Map for converting VirtIO format to ALSA format. */
+static const unsigned int g_v2a_format_map[] = {
+   [VIRTIO_SND_PCM_FMT_IMA_ADPCM] = SNDRV_PCM_FORMAT_IMA_ADPCM,
+   [VIRTIO_SND_PCM_FMT_MU_LAW] = SNDRV_PCM_FORMAT_MU_LAW,
+   [VIRTIO_SND_PCM_FMT_A_LAW] = SNDRV_PCM_FORMAT_A_LAW,
+

[PATCH v3 8/9] ALSA: virtio: introduce PCM channel map support

2021-02-09 Thread Anton Yakovlev

Enumerate all available PCM channel maps and create ALSA controls.

Signed-off-by: Anton Yakovlev 
---
 sound/virtio/Makefile   |   1 +
 sound/virtio/virtio_card.c  |  10 ++
 sound/virtio/virtio_card.h  |   8 ++
 sound/virtio/virtio_chmap.c | 219 
 sound/virtio/virtio_pcm.h   |   4 +
 5 files changed, 242 insertions(+)
 create mode 100644 sound/virtio/virtio_chmap.c

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index 09f485291285..2742bddb8874 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
 
 virtio_snd-objs := \
virtio_card.o \
+   virtio_chmap.o \
virtio_ctl_msg.o \
virtio_jack.o \
virtio_pcm.o \
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 4578d0ce0726..787a4dec1da8 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -222,6 +222,10 @@ static int virtsnd_build_devs(struct virtio_snd *snd)
if (rc)
return rc;
 
+   rc = virtsnd_chmap_parse_cfg(snd);
+   if (rc)
+   return rc;
+
if (snd->njacks) {
rc = virtsnd_jack_build_devs(snd);
if (rc)
@@ -234,6 +238,12 @@ static int virtsnd_build_devs(struct virtio_snd *snd)
return rc;
}
 
+   if (snd->nchmaps) {
+   rc = virtsnd_chmap_build_devs(snd);
+   if (rc)
+   return rc;
+   }
+
return snd_card_register(snd->card);
 }
 
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index 9e6cd79eda25..8ec8bc3ea75e 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -44,6 +44,8 @@ struct virtio_snd_queue {
  * @njacks: Number of jacks.
  * @substreams: VirtIO PCM substreams.
  * @nsubstreams: Number of PCM substreams.
+ * @chmaps: VirtIO channel maps.
+ * @nchmaps: Number of channel maps.
  */
 struct virtio_snd {
struct virtio_device *vdev;
@@ -57,6 +59,8 @@ struct virtio_snd {
unsigned int njacks;
struct virtio_pcm_substream *substreams;
unsigned int nsubstreams;
+   struct virtio_snd_chmap_info *chmaps;
+   unsigned int nchmaps;
 };
 
 /* Message completion timeout in milliseconds (module parameter). */
@@ -102,4 +106,8 @@ int virtsnd_jack_build_devs(struct virtio_snd *snd);
 void virtsnd_jack_event(struct virtio_snd *snd,
struct virtio_snd_event *event);
 
+int virtsnd_chmap_parse_cfg(struct virtio_snd *snd);
+
+int virtsnd_chmap_build_devs(struct virtio_snd *snd);
+
 #endif /* VIRTIO_SND_CARD_H */
diff --git a/sound/virtio/virtio_chmap.c b/sound/virtio/virtio_chmap.c
new file mode 100644
index ..c54d7daa13e3
--- /dev/null
+++ b/sound/virtio/virtio_chmap.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * virtio-snd: Virtio sound device
+ * Copyright (C) 2021 OpenSynergy GmbH
+ */
+#include 
+
+#include "virtio_card.h"
+
+/* VirtIO->ALSA channel position map */
+static const u8 g_v2a_position_map[] = {
+   [VIRTIO_SND_CHMAP_NONE] = SNDRV_CHMAP_UNKNOWN,
+   [VIRTIO_SND_CHMAP_NA] = SNDRV_CHMAP_NA,
+   [VIRTIO_SND_CHMAP_MONO] = SNDRV_CHMAP_MONO,
+   [VIRTIO_SND_CHMAP_FL] = SNDRV_CHMAP_FL,
+   [VIRTIO_SND_CHMAP_FR] = SNDRV_CHMAP_FR,
+   [VIRTIO_SND_CHMAP_RL] = SNDRV_CHMAP_RL,
+   [VIRTIO_SND_CHMAP_RR] = SNDRV_CHMAP_RR,
+   [VIRTIO_SND_CHMAP_FC] = SNDRV_CHMAP_FC,
+   [VIRTIO_SND_CHMAP_LFE] = SNDRV_CHMAP_LFE,
+   [VIRTIO_SND_CHMAP_SL] = SNDRV_CHMAP_SL,
+   [VIRTIO_SND_CHMAP_SR] = SNDRV_CHMAP_SR,
+   [VIRTIO_SND_CHMAP_RC] = SNDRV_CHMAP_RC,
+   [VIRTIO_SND_CHMAP_FLC] = SNDRV_CHMAP_FLC,
+   [VIRTIO_SND_CHMAP_FRC] = SNDRV_CHMAP_FRC,
+   [VIRTIO_SND_CHMAP_RLC] = SNDRV_CHMAP_RLC,
+   [VIRTIO_SND_CHMAP_RRC] = SNDRV_CHMAP_RRC,
+   [VIRTIO_SND_CHMAP_FLW] = SNDRV_CHMAP_FLW,
+   [VIRTIO_SND_CHMAP_FRW] = SNDRV_CHMAP_FRW,
+   [VIRTIO_SND_CHMAP_FLH] = SNDRV_CHMAP_FLH,
+   [VIRTIO_SND_CHMAP_FCH] = SNDRV_CHMAP_FCH,
+   [VIRTIO_SND_CHMAP_FRH] = SNDRV_CHMAP_FRH,
+   [VIRTIO_SND_CHMAP_TC] = SNDRV_CHMAP_TC,
+   [VIRTIO_SND_CHMAP_TFL] = SNDRV_CHMAP_TFL,
+   [VIRTIO_SND_CHMAP_TFR] = SNDRV_CHMAP_TFR,
+   [VIRTIO_SND_CHMAP_TFC] = SNDRV_CHMAP_TFC,
+   [VIRTIO_SND_CHMAP_TRL] = SNDRV_CHMAP_TRL,
+   [VIRTIO_SND_CHMAP_TRR] = SNDRV_CHMAP_TRR,
+   [VIRTIO_SND_CHMAP_TRC] = SNDRV_CHMAP_TRC,
+   [VIRTIO_SND_CHMAP_TFLC] = SNDRV_CHMAP_TFLC,
+   [VIRTIO_SND_CHMAP_TFRC] = SNDRV_CHMAP_TFRC,
+   [VIRTIO_SND_CHMAP_TSL] = SNDRV_CHMAP_TSL,
+   [VIRTIO_SND_CHMAP_TSR] = SNDRV_CHMAP_TSR,
+   [VIRTIO_SND_CHMAP_LLFE] = SNDRV_CHMAP_LLFE,
+   [VIRTIO_SND_CHMAP_RLFE] = SNDRV_CHMAP_RLFE,
+   [VIRTIO_SND_CHMAP_BC] = SNDRV_CHMAP_BC,
+   [VIRTIO_SND_CHMAP_BLC] = SNDRV_CHMAP_BLC,
+   [VIRTIO_SND_CHMAP_BRC] = SNDRV_CHMAP_BRC
+};
+
+/**
+ * virtsnd_chmap_parse_cfg() -

[PATCH v3 7/9] ALSA: virtio: introduce jack support

2021-02-09 Thread Anton Yakovlev

Enumerate all available jacks and create ALSA controls.

At the moment jacks have a simple implementation and can only be used
to receive notifications about a plugged in/out device.

Signed-off-by: Anton Yakovlev 
---
 sound/virtio/Makefile  |   1 +
 sound/virtio/virtio_card.c |  14 +++
 sound/virtio/virtio_card.h |  12 ++
 sound/virtio/virtio_jack.c | 233 +
 4 files changed, 260 insertions(+)
 create mode 100644 sound/virtio/virtio_jack.c

diff --git a/sound/virtio/Makefile b/sound/virtio/Makefile
index 34493226793f..09f485291285 100644
--- a/sound/virtio/Makefile
+++ b/sound/virtio/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SND_VIRTIO) += virtio_snd.o
 virtio_snd-objs := \
virtio_card.o \
virtio_ctl_msg.o \
+   virtio_jack.o \
virtio_pcm.o \
virtio_pcm_msg.o \
virtio_pcm_ops.o
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index a845978111d6..4578d0ce0726 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -67,6 +67,10 @@ static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
virtqueue_disable_cb(vqueue);
while ((event = virtqueue_get_buf(vqueue, ))) {
switch (le32_to_cpu(event->hdr.code)) {
+   case VIRTIO_SND_EVT_JACK_CONNECTED:
+   case VIRTIO_SND_EVT_JACK_DISCONNECTED:
+   virtsnd_jack_event(snd, event);
+   break;
case VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED:
case VIRTIO_SND_EVT_PCM_XRUN:
virtsnd_pcm_event(snd, event);
@@ -210,10 +214,20 @@ static int virtsnd_build_devs(struct virtio_snd *snd)
 VIRTIO_SND_CARD_NAME " at %s/%s",
 dev_name(dev->parent), dev_name(dev));
 
+   rc = virtsnd_jack_parse_cfg(snd);
+   if (rc)
+   return rc;
+
rc = virtsnd_pcm_parse_cfg(snd);
if (rc)
return rc;
 
+   if (snd->njacks) {
+   rc = virtsnd_jack_build_devs(snd);
+   if (rc)
+   return rc;
+   }
+
if (snd->nsubstreams) {
rc = virtsnd_pcm_build_devs(snd);
if (rc)
diff --git a/sound/virtio/virtio_card.h b/sound/virtio/virtio_card.h
index aca87059564e..9e6cd79eda25 100644
--- a/sound/virtio/virtio_card.h
+++ b/sound/virtio/virtio_card.h
@@ -18,6 +18,7 @@
 #define VIRTIO_SND_CARD_NAME   "VirtIO SoundCard"
 #define VIRTIO_SND_PCM_NAME"VirtIO PCM"
 
+struct virtio_jack;
 struct virtio_pcm_substream;
 
 /**
@@ -39,6 +40,8 @@ struct virtio_snd_queue {
  * @ctl_msgs: Pending control request list.
  * @event_msgs: Device events.
  * @pcm_list: VirtIO PCM device list.
+ * @jacks: VirtIO jacks.
+ * @njacks: Number of jacks.
  * @substreams: VirtIO PCM substreams.
  * @nsubstreams: Number of PCM substreams.
  */
@@ -50,6 +53,8 @@ struct virtio_snd {
struct list_head ctl_msgs;
struct virtio_snd_event *event_msgs;
struct list_head pcm_list;
+   struct virtio_jack *jacks;
+   unsigned int njacks;
struct virtio_pcm_substream *substreams;
unsigned int nsubstreams;
 };
@@ -90,4 +95,11 @@ virtsnd_pcm_queue(struct virtio_pcm_substream *vss)
return virtsnd_rx_queue(vss->snd);
 }
 
+int virtsnd_jack_parse_cfg(struct virtio_snd *snd);
+
+int virtsnd_jack_build_devs(struct virtio_snd *snd);
+
+void virtsnd_jack_event(struct virtio_snd *snd,
+   struct virtio_snd_event *event);
+
 #endif /* VIRTIO_SND_CARD_H */
diff --git a/sound/virtio/virtio_jack.c b/sound/virtio/virtio_jack.c
new file mode 100644
index ..a78cf465171d
--- /dev/null
+++ b/sound/virtio/virtio_jack.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * virtio-snd: Virtio sound device
+ * Copyright (C) 2021 OpenSynergy GmbH
+ */
+#include 
+#include 
+#include 
+
+#include "virtio_card.h"
+
+/**
+ * DOC: Implementation Status
+ *
+ * At the moment jacks have a simple implementation and can only be used to
+ * receive notifications about a plugged in/out device.
+ *
+ * VIRTIO_SND_R_JACK_REMAP
+ *   is not supported
+ */
+
+/**
+ * struct virtio_jack - VirtIO jack.
+ * @jack: Kernel jack control.
+ * @nid: Functional group node identifier.
+ * @features: Jack virtio feature bit map (1 << VIRTIO_SND_JACK_F_XXX).
+ * @defconf: Pin default configuration value.
+ * @caps: Pin capabilities value.
+ * @connected: Current jack connection status.
+ * @type: Kernel jack type (SND_JACK_XXX).
+ */
+struct virtio_jack {
+   struct snd_jack *jack;
+   unsigned int nid;
+   unsigned int features;
+   unsigned int defconf;
+   unsigned int caps;
+   bool connected;
+   int type;
+};
+
+/**
+ * virtsnd_jack_get_label() - Get the name string for the jack.
+ * @vjack: VirtIO jack.
+ *
+ * Returns the jack name based

[PATCH v3 2/9] ALSA: virtio: add virtio sound driver

2021-02-09 Thread Anton Yakovlev

Introduce skeleton of the virtio sound driver. The driver implements
the virtio sound device specification, which has become part of the
virtio standard.

Initial initialization of the device, virtqueues and creation of an
empty ALSA sound device. Also, handling DEVICE_NEEDS_RESET device
status.

Signed-off-by: Anton Yakovlev 
---
 MAINTAINERS |   9 +
 include/uapi/linux/virtio_snd.h | 334 +++
 sound/Kconfig   |   2 +
 sound/Makefile  |   3 +-
 sound/virtio/Kconfig|  10 +
 sound/virtio/Makefile   |   7 +
 sound/virtio/virtio_card.c  | 340 
 sound/virtio/virtio_card.h  |  67 +++
 8 files changed, 771 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/virtio_snd.h
 create mode 100644 sound/virtio/Kconfig
 create mode 100644 sound/virtio/Makefile
 create mode 100644 sound/virtio/virtio_card.c
 create mode 100644 sound/virtio/virtio_card.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 2a0737dfca89..51f20656a5ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18945,6 +18945,15 @@ W: https://virtio-mem.gitlab.io/
 F: drivers/virtio/virtio_mem.c
 F: include/uapi/linux/virtio_mem.h
 
+VIRTIO SOUND DRIVER
+M: Anton Yakovlev 
+M: "Michael S. Tsirkin" 
+L: virtualization@lists.linux-foundation.org
+L: alsa-de...@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: include/uapi/linux/virtio_snd.h
+F: sound/virtio/*
+
 VIRTUAL BOX GUEST DEVICE DRIVER
 M: Hans de Goede 
 M: Arnd Bergmann 
diff --git a/include/uapi/linux/virtio_snd.h b/include/uapi/linux/virtio_snd.h
new file mode 100644
index ..dfe49547a7b0
--- /dev/null
+++ b/include/uapi/linux/virtio_snd.h
@@ -0,0 +1,334 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Copyright (C) 2021 OpenSynergy GmbH
+ */
+#ifndef VIRTIO_SND_IF_H
+#define VIRTIO_SND_IF_H
+
+#include 
+
+/***
+ * CONFIGURATION SPACE
+ */
+struct virtio_snd_config {
+   /* # of available physical jacks */
+   __le32 jacks;
+   /* # of available PCM streams */
+   __le32 streams;
+   /* # of available channel maps */
+   __le32 chmaps;
+};
+
+enum {
+   /* device virtqueue indexes */
+   VIRTIO_SND_VQ_CONTROL = 0,
+   VIRTIO_SND_VQ_EVENT,
+   VIRTIO_SND_VQ_TX,
+   VIRTIO_SND_VQ_RX,
+   /* # of device virtqueues */
+   VIRTIO_SND_VQ_MAX
+};
+
+/***
+ * COMMON DEFINITIONS
+ */
+
+/* supported dataflow directions */
+enum {
+   VIRTIO_SND_D_OUTPUT = 0,
+   VIRTIO_SND_D_INPUT
+};
+
+enum {
+   /* jack control request types */
+   VIRTIO_SND_R_JACK_INFO = 1,
+   VIRTIO_SND_R_JACK_REMAP,
+
+   /* PCM control request types */
+   VIRTIO_SND_R_PCM_INFO = 0x0100,
+   VIRTIO_SND_R_PCM_SET_PARAMS,
+   VIRTIO_SND_R_PCM_PREPARE,
+   VIRTIO_SND_R_PCM_RELEASE,
+   VIRTIO_SND_R_PCM_START,
+   VIRTIO_SND_R_PCM_STOP,
+
+   /* channel map control request types */
+   VIRTIO_SND_R_CHMAP_INFO = 0x0200,
+
+   /* jack event types */
+   VIRTIO_SND_EVT_JACK_CONNECTED = 0x1000,
+   VIRTIO_SND_EVT_JACK_DISCONNECTED,
+
+   /* PCM event types */
+   VIRTIO_SND_EVT_PCM_PERIOD_ELAPSED = 0x1100,
+   VIRTIO_SND_EVT_PCM_XRUN,
+
+   /* common status codes */
+   VIRTIO_SND_S_OK = 0x8000,
+   VIRTIO_SND_S_BAD_MSG,
+   VIRTIO_SND_S_NOT_SUPP,
+   VIRTIO_SND_S_IO_ERR
+};
+
+/* common header */
+struct virtio_snd_hdr {
+   __le32 code;
+};
+
+/* event notification */
+struct virtio_snd_event {
+   /* VIRTIO_SND_EVT_XXX */
+   struct virtio_snd_hdr hdr;
+   /* optional event data */
+   __le32 data;
+};
+
+/* common control request to query an item information */
+struct virtio_snd_query_info {
+   /* VIRTIO_SND_R_XXX_INFO */
+   struct virtio_snd_hdr hdr;
+   /* item start identifier */
+   __le32 start_id;
+   /* item count to query */
+   __le32 count;
+   /* item information size in bytes */
+   __le32 size;
+};
+
+/* common item information header */
+struct virtio_snd_info {
+   /* function group node id (High Definition Audio Specification 7.1.2) */
+   __le32 hda_fn_nid;
+};
+
+/***
+ * JACK CONTROL MESSAGES
+ */
+struct virtio_snd_jack_hdr {
+   /* VIRTIO_SND_R_JACK_XXX */
+   struct virtio_snd_hdr hdr;
+   /* 0 ... virtio_snd_config::jacks - 1 */
+   __le32 jack_id;
+};
+
+/* supported jack features */
+enum {
+   VIRTIO_SND_JACK_F_REMAP = 0
+};
+
+struct virtio_snd_jack_info {
+   /* common header */
+   struct virtio_snd_info hdr;
+   /* supported feature bit map (1 << VIRTIO_SND_JACK_F_XXX) */
+   __le32 features;
+

[PATCH v3 0/9] ALSA: add virtio sound driver

2021-02-09 Thread Anton Yakovlev

This series implements a driver part of the virtio sound device
specification v8 [1].

The driver supports PCM playback and capture substreams, jack and
channel map controls. A message-based transport is used to write/read
PCM frames to/from a device.

As a device part was used OpenSynergy proprietary implementation.

Any comments are very welcome.

v3 changes:
- Fixed license headers for all files.
- Many coding style and kernel doc fixes.
- Replaced devm_kmalloc/devm_kfree with kmalloc/kfree wherever appropriate.
- Made the names of the card and PCM devices more informative.
- To process the DEVICE_NEEDS_RESET status, simply call device_reprobe().
- For control messages replaced atomic_t by refcount_t for the reference counter
  and simplified the general logic.
- Use vmalloc'ed managed buffer for PCM substreams.
- Replaced all atomic fields in the virtio substream structure with
  non-atomic + spinlock.
- Use the non-atomic PCM ops.
- Use ops->sync_stop() to release the substream on the device side.
- Rebased and tested on top of v5.11-rc6.

v2 changes:
- For some reason, in the previous patch series, several patches were
  squashed. Fixed this issue to make the review easier.
- Added m...@redhat.com to the MAINTAINERS.
- When creating virtqueues, now only the event virtqueue is disabled.
  It's enabled only after successful initialization of the device.
- Added additional comments to the reset worker function:
  [2/9] virtio_card.c:virtsnd_reset_fn()
- Added check that VIRTIO_F_VERSION_1 feature bit is set.
- Added additional comments to the device removing function:
  [2/9] virtio_card.c:virtsnd_remove()
- Added additional comments to the tx/rx interrupt handler:
  [5/9] virtio_pcm_msg.c:virtsnd_pcm_msg_complete()
- Added additional comments to substream release wait function.
  [6/9] virtio_pcm_ops.c:virtsnd_pcm_released()

[1] https://lists.oasis-open.org/archives/virtio-dev/202003/msg00185.html


Anton Yakovlev (9):
  uapi: virtio_ids: add a sound device type ID from OASIS spec
  ALSA: virtio: add virtio sound driver
  ALSA: virtio: handling control messages
  ALSA: virtio: build PCM devices and substream hardware descriptors
  ALSA: virtio: handling control and I/O messages for the PCM device
  ALSA: virtio: PCM substream operators
  ALSA: virtio: introduce jack support
  ALSA: virtio: introduce PCM channel map support
  ALSA: virtio: introduce device suspend/resume support

 MAINTAINERS |   9 +
 include/uapi/linux/virtio_ids.h |   1 +
 include/uapi/linux/virtio_snd.h | 334 +
 sound/Kconfig   |   2 +
 sound/Makefile  |   3 +-
 sound/virtio/Kconfig|  10 +
 sound/virtio/Makefile   |  13 +
 sound/virtio/virtio_card.c  | 462 +
 sound/virtio/virtio_card.h  | 113 
 sound/virtio/virtio_chmap.c | 219 ++
 sound/virtio/virtio_ctl_msg.c   | 311 
 sound/virtio/virtio_ctl_msg.h   |  78 +
 sound/virtio/virtio_jack.c  | 233 +++
 sound/virtio/virtio_pcm.c   | 499 
 sound/virtio/virtio_pcm.h   | 121 
 sound/virtio/virtio_pcm_msg.c   | 393 +
 sound/virtio/virtio_pcm_ops.c   | 493 +++
 17 files changed, 3293 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/virtio_snd.h
 create mode 100644 sound/virtio/Kconfig
 create mode 100644 sound/virtio/Makefile
 create mode 100644 sound/virtio/virtio_card.c
 create mode 100644 sound/virtio/virtio_card.h
 create mode 100644 sound/virtio/virtio_chmap.c
 create mode 100644 sound/virtio/virtio_ctl_msg.c
 create mode 100644 sound/virtio/virtio_ctl_msg.h
 create mode 100644 sound/virtio/virtio_jack.c
 create mode 100644 sound/virtio/virtio_pcm.c
 create mode 100644 sound/virtio/virtio_pcm.h
 create mode 100644 sound/virtio/virtio_pcm_msg.c
 create mode 100644 sound/virtio/virtio_pcm_ops.c

-- 
2.30.0


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v3 1/9] uapi: virtio_ids: add a sound device type ID from OASIS spec

2021-02-09 Thread Anton Yakovlev

The OASIS virtio spec defines a sound device type ID that is not
present in the header yet.

Signed-off-by: Anton Yakovlev 
---
 include/uapi/linux/virtio_ids.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index bc1c0621f5ed..029a2e07a7f9 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -51,6 +51,7 @@
 #define VIRTIO_ID_PSTORE   22 /* virtio pstore device */
 #define VIRTIO_ID_IOMMU23 /* virtio IOMMU */
 #define VIRTIO_ID_MEM  24 /* virtio mem */
+#define VIRTIO_ID_SOUND25 /* virtio sound */
 #define VIRTIO_ID_FS   26 /* virtio filesystem */
 #define VIRTIO_ID_PMEM 27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM   29 /* virtio mac80211-hwsim */
-- 
2.30.0


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drm/qxl: properly handle device init failures

2021-02-09 Thread Gerd Hoffmann

On Mon, Feb 08, 2021 at 12:07:01PM -0500, Tong Zhang wrote:
> Does this patch fix an issue raised previously? Or should they be used 
> together?
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2466541.html 
> 
> IMHO using this patch alone won’t fix the issue

This patch on top of drm-misc-next fixes the initialization error issue
reported by you in my testing.

take care,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] swiotlb: Validate bounce size in the sync/unmap path

2021-02-09 Thread Christoph Hellwig

On Mon, Feb 08, 2021 at 12:14:49PM -0500, Konrad Rzeszutek Wilk wrote:
> > ring buffer or whatever because you know I/O will be copied anyway
> > and none of all the hard work higher layers do to make the I/O suitable
> > for a normal device apply.
> 
> I lost you here. Sorry, are you saying have a simple ring protocol
> (like NVME has), where the ring entries (SG or DMA phys) are statically
> allocated and whenever NVME driver gets data from user-space it
> would copy it in there?

Yes.  Basically extend the virtio or NVMe ring/queue concept to not just
cover commands and completions, but also the data transfers.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: fix param validation in mlx5_vdpa_get_config()

2021-02-09 Thread Michael S. Tsirkin

On Tue, Feb 09, 2021 at 11:24:03AM +0800, Jason Wang wrote:
> 
> On 2021/2/9 上午2:38, Michael S. Tsirkin wrote:
> > On Mon, Feb 08, 2021 at 05:17:41PM +0100, Stefano Garzarella wrote:
> > > It's legal to have 'offset + len' equal to
> > > sizeof(struct virtio_net_config), since 'ndev->config' is a
> > > 'struct virtio_net_config', so we can safely copy its content under
> > > this condition.
> > > 
> > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 
> > > devices")
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Stefano Garzarella 
> > > ---
> > >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index dc88559a8d49..10e9b09932eb 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -1820,7 +1820,7 @@ static void mlx5_vdpa_get_config(struct vdpa_device 
> > > *vdev, unsigned int offset,
> > >   struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> > >   struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> > > - if (offset + len < sizeof(struct virtio_net_config))
> > > + if (offset + len <= sizeof(struct virtio_net_config))
> > >   memcpy(buf, (u8 *)>config + offset, len);
> > >   }
> > Actually first I am not sure we need these checks at all.
> > vhost_vdpa_config_validate already validates the values, right?
> 
> 
> I think they're working at different levels. There's no guarantee that
> vhost-vdpa is the driver for this vdpa device.

In fact, get_config returns void, so userspace can easily get
trash if it passes incorrect parameters by mistake, there is
no way for userspace to find out whether that is the case :(

Any objections to returning the # of bytes copied, or -1
on error?

> 
> > 
> > Second, what will happen when we extend the struct and then
> > run new userspace on an old kernel? Looks like it will just
> > fail right? So what is the plan?
> 
> 
> In this case, get_config() should match the spec behaviour. That is to say
> the size of config space depends on the feature negotiated.
> 
> Thanks

Yes but spec says config space can be bigger than specified by features:

Drivers MUST NOT limit structure size and device configuration space 
size. Instead, drivers SHOULD only
check that device configuration space is large enough to contain the 
fields necessary for device operation.



> 
> >   I think we should
> > allow a bigger size, and return the copied config size to userspace.
> > 
> > 
> > > -- 
> > > 2.29.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: fix param validation in mlx5_vdpa_get_config()

2021-02-09 Thread Stefano Garzarella


On Tue, Feb 09, 2021 at 11:24:03AM +0800, Jason Wang wrote:


On 2021/2/9 上午2:38, Michael S. Tsirkin wrote:

On Mon, Feb 08, 2021 at 05:17:41PM +0100, Stefano Garzarella wrote:

It's legal to have 'offset + len' equal to
sizeof(struct virtio_net_config), since 'ndev->config' is a
'struct virtio_net_config', so we can safely copy its content under
this condition.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Cc: sta...@vger.kernel.org
Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index dc88559a8d49..10e9b09932eb 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1820,7 +1820,7 @@ static void mlx5_vdpa_get_config(struct vdpa_device 
*vdev, unsigned int offset,
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
-   if (offset + len < sizeof(struct virtio_net_config))
+   if (offset + len <= sizeof(struct virtio_net_config))
memcpy(buf, (u8 *)>config + offset, len);
 }

Actually first I am not sure we need these checks at all.
vhost_vdpa_config_validate already validates the values, right?



I think they're working at different levels. There's no guarantee that 
vhost-vdpa is the driver for this vdpa device.




Maybe we can do these checks in the vdpa_get_config() helper and we can 
also add a vdpa_set_config() to do the same.


Thanks,
Stefano





Second, what will happen when we extend the struct and then
run new userspace on an old kernel? Looks like it will just
fail right? So what is the plan?



In this case, get_config() should match the spec behaviour. That is to 
say the size of config space depends on the feature negotiated.


Thanks



 I think we should
allow a bigger size, and return the copied config size to userspace.



--
2.29.2




___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 16/19] virtio-pci: introduce modern device module

2021-02-09 Thread Michael S. Tsirkin

On Tue, Feb 09, 2021 at 11:29:46AM +0800, Jason Wang wrote:
> 
> On 2021/2/8 下午8:04, Michael S. Tsirkin wrote:
> > On Mon, Feb 08, 2021 at 01:42:27PM +0800, Jason Wang wrote:
> > > On 2021/2/5 下午11:34, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 04, 2021 at 02:55:00PM +0800, Jason Wang wrote:
> > > > > Signed-off-by: Jason Wang
> > > > I don't exactly get why we need to split the modern driver out,
> > > > and it can confuse people who are used to be seeing virtio-pci.
> > > 
> > > The virtio-pci module still there. No user visible changes. Just some 
> > > codes
> > > that could be shared with other driver were split out.
> > > 
> > What I am saying is this: we can have virtio-vdpa depend on
> > virtio-pci without splitting the common code out to an
> > extra module.
> 
> 
> Ok.
> 
> 
> > 
> > > > The vdpa thing so far looks like a development tool, why do
> > > > we care that it depends on a bit of extra code?
> > > 
> > > If I'm not misunderstanding, trying to share codes is proposed by you 
> > > here:
> > > 
> > > https://lkml.org/lkml/2020/6/10/232
> > > 
> > > We also had the plan to convert IFCVF to use this library.
> > > 
> > > Thanks
> > If that happens then an extra module might become useful.
> 
> 
> So does it make sense that I post a new version and let's merge it first.
> Then Intel or I can convert IFCVF to use the library?
> 
> Thanks

Generally it's best if we actually have a couple of users before we bother
with refactoring - it's hard to predict the future,
so we don't really know what kind of refactoring will work for IFCVF ...

> 
> > 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: fix param validation in mlx5_vdpa_get_config()

2021-02-09 Thread Stefano Garzarella


On Tue, Feb 09, 2021 at 07:43:02AM +0200, Eli Cohen wrote:

On Mon, Feb 08, 2021 at 05:17:41PM +0100, Stefano Garzarella wrote:

It's legal to have 'offset + len' equal to
sizeof(struct virtio_net_config), since 'ndev->config' is a
'struct virtio_net_config', so we can safely copy its content under
this condition.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Cc: sta...@vger.kernel.org
Signed-off-by: Stefano Garzarella 


Acked-by: Eli Cohen 

BTW, same error in vdpa_sim you may want to fix.



Commit 65b709586e22 ("vdpa_sim: add get_config callback in 
vdpasim_dev_attr") unintentionally solved it.


Since it's a simulator, maybe we can avoid solving it in the stable 
branches. Or does it matter?


Thanks,
Stefano


---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index dc88559a8d49..10e9b09932eb 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1820,7 +1820,7 @@ static void mlx5_vdpa_get_config(struct vdpa_device 
*vdev, unsigned int offset,
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);

-   if (offset + len < sizeof(struct virtio_net_config))
+   if (offset + len <= sizeof(struct virtio_net_config))
memcpy(buf, (u8 *)>config + offset, len);
 }

--
2.29.2





___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

38 matches

Mail list logo