Re: [PATCH V2 06/12] virtio_pci: harden MSI-X interrupts

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:21PM +0800, Jason Wang wrote: > We used to synchronize pending MSI-X irq handlers via > synchronize_irq(), this may not work for the untrusted device which > may keep sending interrupts after reset which may lead unexpected > results. Similarly, we should not enable

Re: [PATCH V2 01/12] virtio-blk: validate num_queues during probe

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:16PM +0800, Jason Wang wrote: > If an untrusted device neogitates BLK_F_MQ but advertises a zero > num_queues, the driver may end up trying to allocating zero size > buffers where ZERO_SIZE_PTR is returned which may pass the checking > against the NULL. This will lead

Re: [PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread Viresh Kumar
On 13-10-21, 06:55, Michael S. Tsirkin wrote: > This will enable cleanups down the road. > The idea is to disable cbs, then add "flush_queued_cbs" callback > as a parameter, this way drivers can flush any work > queued after callbacks have been disabled. > > Signed-off-by: Michael S. Tsirkin >

[PATCH] Revert "virtio-blk: Add validation for block size in config space"

2021-10-13 Thread Michael S. Tsirkin
It turns out that access to config space before completing the feature negotiation is broken for big endian guests at least with QEMU hosts up to 6.1 inclusive. This affects any device that accesses config space in the validate callback: at the moment that is virtio-net with VIRTIO_NET_F_MTU but

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 02:44:08PM +0200, Halil Pasic wrote: > On Wed, 13 Oct 2021 08:24:53 -0400 > "Michael S. Tsirkin" wrote: > > > > > OK this looks good! How about a QEMU patch to make it spec compliant on > > > > BE? > > > > > > Who is going to do that? Halil? you? Conny? > > > >

Re: [PATCH V2 05/12] virtio_config: introduce a new ready method

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:20PM +0800, Jason Wang wrote: > Signed-off-by: Jason Wang > --- > include/linux/virtio_config.h | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > index 8519b3ae5d52..f2891c6221a1 100644 >

Re: [PATCH v2] vduse: Fix race condition between resetting and irq injecting

2021-10-13 Thread Michael S. Tsirkin
On Wed, Sep 29, 2021 at 04:50:24PM +0800, Yongji Xie wrote: > On Wed, Sep 29, 2021 at 4:41 PM Jason Wang wrote: > > > > On Wed, Sep 29, 2021 at 4:32 PM Xie Yongji wrote: > > > > > > The interrupt might be triggered after a reset since there is > > > no synchronization between resetting and irq

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Christian Borntraeger
Am 13.10.21 um 12:10 schrieb Michael S. Tsirkin: On Mon, Oct 11, 2021 at 07:39:21AM +0200, Halil Pasic wrote: The virtio specification virtio-v1.1-cs01 states: "Transitional devices MUST detect Legacy drivers by detecting that VIRTIO_F_VERSION_1 has not been acknowledged by the driver."

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Cornelia Huck
On Wed, Oct 13 2021, "Michael S. Tsirkin" wrote: > On Wed, Oct 13, 2021 at 01:23:50PM +0200, Christian Borntraeger wrote: >> Can we get this kernel patch queued for 5.15 and stable without waiting for >> the QEMU patch >> as we have a regression with 4.14? > > Probably. Still trying to decide

Re: [PATCH v5] virtio-blk: Add validation for block size in config space

2021-10-13 Thread Michael S. Tsirkin
On Mon, Oct 11, 2021 at 01:40:41PM +0200, Christoph Hellwig wrote: > On Tue, Oct 05, 2021 at 06:42:43AM -0400, Michael S. Tsirkin wrote: > > Stefan also pointed out this duplicates the logic from > > > > if (blksize < 512 || blksize > PAGE_SIZE || !is_power_of_2(blksize)) > >

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Michael S. Tsirkin
On Mon, Oct 11, 2021 at 07:39:21AM +0200, Halil Pasic wrote: > The virtio specification virtio-v1.1-cs01 states: "Transitional devices > MUST detect Legacy drivers by detecting that VIRTIO_F_VERSION_1 has not > been acknowledged by the driver." This is exactly what QEMU as of 6.1 > has done

Re: [PATCH v2] vduse: Fix race condition between resetting and irq injecting

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 08:30:40PM +0800, Yongji Xie wrote: > On Wed, Oct 13, 2021 at 7:10 PM Michael S. Tsirkin wrote: > > > > On Wed, Sep 29, 2021 at 04:30:50PM +0800, Xie Yongji wrote: > > > The interrupt might be triggered after a reset since there is > > > no synchronization between

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Halil Pasic
On Wed, 13 Oct 2021 08:24:53 -0400 "Michael S. Tsirkin" wrote: > > > OK this looks good! How about a QEMU patch to make it spec compliant on > > > BE? > > > > Who is going to do that? Halil? you? Conny? > > Halil said he'll do it... Right, Halil? I can do it but not right away. Maybe in a

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 02:52:38PM +0200, Cornelia Huck wrote: > On Wed, Oct 13 2021, "Michael S. Tsirkin" wrote: > > > On Wed, Oct 13, 2021 at 01:23:50PM +0200, Christian Borntraeger wrote: > >> Can we get this kernel patch queued for 5.15 and stable without waiting > >> for the QEMU patch >

Re: [PATCH V2 09/12] virtio_ring: validate used buffer length

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:24PM +0800, Jason Wang wrote: > This patch validate the used buffer length provided by the device > before trying to use it. This is done by record the in buffer length > in a new field in desc_state structure during virtqueue_add(), then we > can fail the

Re: [PATCH] vduse: Disallow injecting interrupt before DRIVER_OK is set

2021-10-13 Thread Michael S. Tsirkin
On Thu, Sep 23, 2021 at 03:57:22PM +0800, Xie Yongji wrote: > The interrupt callback should not be triggered before DRIVER_OK > is set. Otherwise, it might break the virtio device driver. > So let's add a check to avoid the unexpected behavior. > > Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE -

Re: [PATCH v3 1/1] virtio: write back F_VERSION_1 before validate

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 01:23:50PM +0200, Christian Borntraeger wrote: > > > Am 13.10.21 um 12:10 schrieb Michael S. Tsirkin: > > On Mon, Oct 11, 2021 at 07:39:21AM +0200, Halil Pasic wrote: > > > The virtio specification virtio-v1.1-cs01 states: "Transitional devices > > > MUST detect Legacy

Re: [PATCH v2] vduse: Fix race condition between resetting and irq injecting

2021-10-13 Thread Michael S. Tsirkin
On Wed, Sep 29, 2021 at 04:30:50PM +0800, Xie Yongji wrote: > The interrupt might be triggered after a reset since there is > no synchronization between resetting and irq injecting. In fact, irq_lock is already used to synchronize with irqs. Why isn't taking and releasing it enough? > And it >

[PATCH 5/5] iommu/virtio: Support identity-mapped domains

2021-10-13 Thread Jean-Philippe Brucker
Support identity domains for devices that do not offer the VIRTIO_IOMMU_F_BYPASS_CONFIG feature, by creating 1:1 mappings between the virtual and physical address space. Identity domains created this way still perform noticeably better than DMA domains, because they don't have the overhead of

[PATCH 0/5] iommu/virtio: Add identity domains

2021-10-13 Thread Jean-Philippe Brucker
Support identity domains, allowing to only enable IOMMU protection for a subset of endpoints (those assigned to userspace, for example). Users may enable identity domains at compile time (CONFIG_IOMMU_DEFAULT_PASSTHROUGH), boot time (iommu.passthrough=1) or runtime (/sys/kernel/iommu_groups/*/type

[PATCH 3/5] iommu/virtio: Sort reserved regions

2021-10-13 Thread Jean-Philippe Brucker
To ease identity mapping support, keep the list of reserved regions sorted. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/virtio-iommu.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c index

[PATCH 2/5] iommu/virtio: Support bypass domains

2021-10-13 Thread Jean-Philippe Brucker
The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a new flag to the ATTACH request, that creates a bypass domain. Use it to enable identity domains. When VIRTIO_IOMMU_F_BYPASS_CONFIG is not supported by the device, we currently fail attaching to an identity domain. Future patches will instead create

[PATCH 1/5] iommu/virtio: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG

2021-10-13 Thread Jean-Philippe Brucker
Add definitions for the VIRTIO_IOMMU_F_BYPASS_CONFIG, which supersedes VIRTIO_IOMMU_F_BYPASS. Signed-off-by: Jean-Philippe Brucker --- include/uapi/linux/virtio_iommu.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/virtio_iommu.h

[PATCH 4/5] iommu/virtio: Pass end address to viommu_add_mapping()

2021-10-13 Thread Jean-Philippe Brucker
To support identity mappings, the virtio-iommu driver must be able to represent full 64-bit ranges internally. Pass (start, end) instead of (start, size) to viommu_add/del_mapping(). Clean comments. The one about the returned size was never true: when sweeping the whole address space the returned

Re: [PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 01:03:46PM +0200, David Hildenbrand wrote: > On 13.10.21 12:55, Michael S. Tsirkin wrote: > > This will enable cleanups down the road. > > The idea is to disable cbs, then add "flush_queued_cbs" callback > > as a parameter, this way drivers can flush any work > > queued

Re: [PATCH v5] virtio-blk: Add validation for block size in config space

2021-10-13 Thread Michael S. Tsirkin
On Wed, Oct 13, 2021 at 08:34:22PM +0800, Yongji Xie wrote: > On Wed, Oct 13, 2021 at 8:21 PM Michael S. Tsirkin wrote: > > > > On Mon, Oct 11, 2021 at 01:40:41PM +0200, Christoph Hellwig wrote: > > > On Tue, Oct 05, 2021 at 06:42:43AM -0400, Michael S. Tsirkin wrote: > > > > Stefan also pointed

[PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread Michael S. Tsirkin
This will enable cleanups down the road. The idea is to disable cbs, then add "flush_queued_cbs" callback as a parameter, this way drivers can flush any work queued after callbacks have been disabled. Signed-off-by: Michael S. Tsirkin --- arch/um/drivers/virt-pci.c | 2 +-

Re: [PATCH V2 07/12] virtio-pci: harden INTX interrupts

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:22PM +0800, Jason Wang wrote: > This patch tries to make sure the virtio interrupt handler for INTX > won't be called after a reset and before virtio_device_ready(). We > can't use IRQF_NO_AUTOEN since we're using shared interrupt > (IRQF_SHARED). So this patch tracks

Re: [PATCH V2 03/12] virtio-console: switch to use .validate()

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:18PM +0800, Jason Wang wrote: > This patch switches to use validate() to filter out the features that > is not supported by the rproc. are not supported > > Cc: Amit Shah > Signed-off-by: Jason Wang Does this have anything to do with hardening? It seems

Re: [PATCH V2 02/12] virtio: add doc for validate() method

2021-10-13 Thread Michael S. Tsirkin
On Tue, Oct 12, 2021 at 02:52:17PM +0800, Jason Wang wrote: > This patch adds doc for validate() method. > > Signed-off-by: Jason Wang And maybe document __virtio_clear_bit : it says "for use by transports" and now it's also legal in the validate callback. > --- > include/linux/virtio.h | 1 +

Re: [PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread David Hildenbrand
On 13.10.21 12:55, Michael S. Tsirkin wrote: This will enable cleanups down the road. The idea is to disable cbs, then add "flush_queued_cbs" callback as a parameter, this way drivers can flush any work queued after callbacks have been disabled. Signed-off-by: Michael S. Tsirkin ---

Re: [PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread David Hildenbrand
On 13.10.21 14:17, Michael S. Tsirkin wrote: > On Wed, Oct 13, 2021 at 01:03:46PM +0200, David Hildenbrand wrote: >> On 13.10.21 12:55, Michael S. Tsirkin wrote: >>> This will enable cleanups down the road. >>> The idea is to disable cbs, then add "flush_queued_cbs" callback >>> as a parameter,

Re: [PATCH RFC] virtio: wrap config->reset calls

2021-10-13 Thread Vivek Goyal
On Wed, Oct 13, 2021 at 06:55:31AM -0400, Michael S. Tsirkin wrote: > This will enable cleanups down the road. > The idea is to disable cbs, then add "flush_queued_cbs" callback > as a parameter, this way drivers can flush any work > queued after callbacks have been disabled. > > Signed-off-by:

Re: [PATCH V2 06/12] virtio_pci: harden MSI-X interrupts

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 5:59 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:21PM +0800, Jason Wang wrote: > > We used to synchronize pending MSI-X irq handlers via > > synchronize_irq(), this may not work for the untrusted device which > > may keep sending interrupts after reset

Re: [PATCH V2 03/12] virtio-console: switch to use .validate()

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 5:51 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:18PM +0800, Jason Wang wrote: > > This patch switches to use validate() to filter out the features that > > is not supported by the rproc. > > are not supported > > > > > Cc: Amit Shah > > Signed-off-by:

Re: [PATCH V2 07/12] virtio-pci: harden INTX interrupts

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 5:42 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:22PM +0800, Jason Wang wrote: > > This patch tries to make sure the virtio interrupt handler for INTX > > won't be called after a reset and before virtio_device_ready(). We > > can't use IRQF_NO_AUTOEN

Re: [PATCH V2 01/12] virtio-blk: validate num_queues during probe

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 6:04 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:16PM +0800, Jason Wang wrote: > > If an untrusted device neogitates BLK_F_MQ but advertises a zero > > num_queues, the driver may end up trying to allocating zero size > > buffers where ZERO_SIZE_PTR is

Re: [PATCH V2 02/12] virtio: add doc for validate() method

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 6:09 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:17PM +0800, Jason Wang wrote: > > This patch adds doc for validate() method. > > > > Signed-off-by: Jason Wang > > And maybe document __virtio_clear_bit : it says > "for use by transports" and now it's

RE: [PATCH 0/5] iommu/virtio: Add identity domains

2021-10-13 Thread Tian, Kevin
> From: Jean-Philippe Brucker > Sent: Wednesday, October 13, 2021 8:11 PM > > Support identity domains, allowing to only enable IOMMU protection for a > subset of endpoints (those assigned to userspace, for example). Users > may enable identity domains at compile time >

Re: [PATCH V2 07/12] virtio-pci: harden INTX interrupts

2021-10-13 Thread Michael S. Tsirkin
On Thu, Oct 14, 2021 at 10:35:48AM +0800, Jason Wang wrote: > On Wed, Oct 13, 2021 at 5:42 PM Michael S. Tsirkin wrote: > > > > On Tue, Oct 12, 2021 at 02:52:22PM +0800, Jason Wang wrote: > > > This patch tries to make sure the virtio interrupt handler for INTX > > > won't be called after a reset

RE: [PATCH 2/5] iommu/virtio: Support bypass domains

2021-10-13 Thread Tian, Kevin
> From: Tian, Kevin > Sent: Thursday, October 14, 2021 11:25 AM > > > From: Jean-Philippe Brucker > > Sent: Wednesday, October 13, 2021 8:11 PM > > > > The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a new flag to the > > ATTACH > > request, that creates a bypass domain. Use it to enable identity

Re: [PATCH V2 01/12] virtio-blk: validate num_queues during probe

2021-10-13 Thread Michael S. Tsirkin
On Thu, Oct 14, 2021 at 10:32:32AM +0800, Jason Wang wrote: > On Wed, Oct 13, 2021 at 6:04 PM Michael S. Tsirkin wrote: > > > > On Tue, Oct 12, 2021 at 02:52:16PM +0800, Jason Wang wrote: > > > If an untrusted device neogitates BLK_F_MQ but advertises a zero > > > num_queues, the driver may end

Re: [PATCH V2 03/12] virtio-console: switch to use .validate()

2021-10-13 Thread Michael S. Tsirkin
On Thu, Oct 14, 2021 at 10:28:06AM +0800, Jason Wang wrote: > On Wed, Oct 13, 2021 at 5:51 PM Michael S. Tsirkin wrote: > > > > On Tue, Oct 12, 2021 at 02:52:18PM +0800, Jason Wang wrote: > > > This patch switches to use validate() to filter out the features that > > > is not supported by the

RE: [PATCH 2/5] iommu/virtio: Support bypass domains

2021-10-13 Thread Tian, Kevin
> From: Jean-Philippe Brucker > Sent: Wednesday, October 13, 2021 8:11 PM > > The VIRTIO_IOMMU_F_BYPASS_CONFIG feature adds a new flag to the > ATTACH > request, that creates a bypass domain. Use it to enable identity > domains. > > When VIRTIO_IOMMU_F_BYPASS_CONFIG is not supported by the

Re: [PATCH] Revert "virtio-blk: Add validation for block size in config space"

2021-10-13 Thread Michael S. Tsirkin
On Thu, Oct 14, 2021 at 10:45:25AM +0800, Jason Wang wrote: > On Wed, Oct 13, 2021 at 8:46 PM Michael S. Tsirkin wrote: > > > > It turns out that access to config space before completing the feature > > negotiation is broken for big endian guests at least with QEMU hosts up > > to 6.1 inclusive.

Re: [PATCH] Revert "virtio-blk: Add validation for block size in config space"

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 8:46 PM Michael S. Tsirkin wrote: > > It turns out that access to config space before completing the feature > negotiation is broken for big endian guests at least with QEMU hosts up > to 6.1 inclusive. This affects any device that accesses config space in > the validate

Re: [PATCH V2 09/12] virtio_ring: validate used buffer length

2021-10-13 Thread Jason Wang
On Wed, Oct 13, 2021 at 6:02 PM Michael S. Tsirkin wrote: > > On Tue, Oct 12, 2021 at 02:52:24PM +0800, Jason Wang wrote: > > This patch validate the used buffer length provided by the device > > before trying to use it. This is done by record the in buffer length > > in a new field in desc_state

Re: [PATCH v3 3/3] vdpa: Check for iova range at mappings changes

2021-10-13 Thread Jason Wang
On Tue, Oct 12, 2021 at 10:07 PM Eugenio Pérez wrote: > > Check vdpa device range before updating memory regions so we don't add > any outside of it, and report the invalid change if any. > > Signed-off-by: Eugenio Pérez > --- > include/hw/virtio/vhost-vdpa.h | 2 ++ > hw/virtio/vhost-vdpa.c

[mst-vhost:vhost 14/16] drivers/virtio/virtio.c:207:13: error: 'virtio_reset_device' defined but not used

2021-10-13 Thread kernel test robot
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost head: 825f5e35666901a3cf6963c8ea3aea0898846a8e commit: e958cd4aadfac0d2968c71208d39069e7968164c [14/16] virtio: wrap config->reset calls config: microblaze-buildonly-randconfig-r001-20211013 (attached as .con