Re: [PATCH] vhost_net: Use fdget() and fdput()
On Fri, May 05, 2023 at 02:24:04PM +0800, ye.xingc...@zte.com.cn wrote:
> From: Ye Xingchen
>
> convert the fget()/fput() uses to fdget()/fdput().
>
> Signed-off-by: Ye Xingchen
> ---
> drivers/vhost/net.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index ae2273196b0c..5b3fe4805182 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1466,17 +1466,17 @@ static struct ptr_ring *get_tap_ptr_ring(struct file
> *file)
>
> static struct socket *get_tap_socket(int fd)
> {
> - struct file *file = fget(fd);
> + struct fd f = fdget(fd);
> struct socket *sock;
>
> - if (!file)
> + if (!f.file)
> return ERR_PTR(-EBADF);
> - sock = tun_get_socket(file);
> + sock = tun_get_socket(f.file);
> if (!IS_ERR(sock))
> return sock;
> - sock = tap_get_socket(file);
> + sock = tap_get_socket(f.file);
> if (IS_ERR(sock))
> - fput(file);
> + fdput(f);
> return sock;
NAK. For the same reason why the sockfd_lookup() counterpart of that
patch is broken. After your change there's no way for the caller
to tell whether we have bumped the refcount of file in question;
this can't possibly work.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Add missing documentation for structure fields
On Wed, May 10, 2023 at 10:18:33PM +0200, Simon Horman wrote: > On Wed, May 10, 2023 at 04:14:53PM -0400, Michael S. Tsirkin wrote: > > On Wed, May 10, 2023 at 10:01:17PM +0200, Simon Horman wrote: > > > On Wed, May 10, 2023 at 12:04:21PM -0400, Michael S. Tsirkin wrote: > > > > On Wed, May 10, 2023 at 02:23:12PM +0200, Simon Horman wrote: > > > > > Add missing documentation for the vqs_list_lock field of struct > > > > > virtio_device, > > > > > and the validate field of struct virtio_driver. > > > > > > > > > > ./scripts/kernel-doc says: > > > > > > > > > > .../virtio.h:131: warning: Function parameter or member > > > > > 'vqs_list_lock' not described in 'virtio_device' > > > > > .../virtio.h:192: warning: Function parameter or member 'validate' > > > > > not described in 'virtio_driver' > > > > > 2 warnings as Errors > > > > > > > > > > No functional changes intended. > > > > > > > > > > Signed-off-by: Simon Horman > > > > > --- > > > > > include/linux/virtio.h | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > > > > > index b93238db94e3..0b2b82ee3220 100644 > > > > > --- a/include/linux/virtio.h > > > > > +++ b/include/linux/virtio.h > > > > > @@ -103,6 +103,7 @@ int virtqueue_resize(struct virtqueue *vq, u32 > > > > > num, > > > > > * @config_enabled: configuration change reporting enabled > > > > > * @config_change_pending: configuration change reported while > > > > > disabled > > > > > * @config_lock: protects configuration change reporting > > > > > + * @vqs_list_lock: protects @vqs. > > > > > * @dev: underlying device. > > > > > * @id: the device type identification (used to match it with a > > > > > driver). > > > > > * @config: the configuration ops for this device. > > > > > @@ -160,6 +161,7 @@ size_t virtio_max_dma_size(const struct > > > > > virtio_device *vdev); > > > > > * @feature_table_size: number of entries in the feature table array. > > > > > * @feature_table_legacy: same as feature_table but when working in > > > > > legacy mode. > > > > > * @feature_table_size_legacy: number of entries in feature table > > > > > legacy array. > > > > > + * @validate: the function to call to vaidate features at probe time. > > > > > > > > typo > > > > > > > > and this is called before probe actually not at probe time > > > > > > Thanks, how about the following? > > > > > > * @validate: the function to call to validate features > > > > ... and config space > > Ok, so: > > * @validate: the function to call to validate features and config spaces config space > > > > > * @probe: the function to call when a device is found. Returns 0 > > > > > or -errno. > > > > > * @scan: optional function to call after successful probe; intended > > > > > *for virtio-scsi to invoke a scan. > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Add missing documentation for structure fields
On Wed, May 10, 2023 at 10:01:17PM +0200, Simon Horman wrote: > On Wed, May 10, 2023 at 12:04:21PM -0400, Michael S. Tsirkin wrote: > > On Wed, May 10, 2023 at 02:23:12PM +0200, Simon Horman wrote: > > > Add missing documentation for the vqs_list_lock field of struct > > > virtio_device, > > > and the validate field of struct virtio_driver. > > > > > > ./scripts/kernel-doc says: > > > > > > .../virtio.h:131: warning: Function parameter or member 'vqs_list_lock' > > > not described in 'virtio_device' > > > .../virtio.h:192: warning: Function parameter or member 'validate' not > > > described in 'virtio_driver' > > > 2 warnings as Errors > > > > > > No functional changes intended. > > > > > > Signed-off-by: Simon Horman > > > --- > > > include/linux/virtio.h | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > > > index b93238db94e3..0b2b82ee3220 100644 > > > --- a/include/linux/virtio.h > > > +++ b/include/linux/virtio.h > > > @@ -103,6 +103,7 @@ int virtqueue_resize(struct virtqueue *vq, u32 num, > > > * @config_enabled: configuration change reporting enabled > > > * @config_change_pending: configuration change reported while disabled > > > * @config_lock: protects configuration change reporting > > > + * @vqs_list_lock: protects @vqs. > > > * @dev: underlying device. > > > * @id: the device type identification (used to match it with a driver). > > > * @config: the configuration ops for this device. > > > @@ -160,6 +161,7 @@ size_t virtio_max_dma_size(const struct virtio_device > > > *vdev); > > > * @feature_table_size: number of entries in the feature table array. > > > * @feature_table_legacy: same as feature_table but when working in > > > legacy mode. > > > * @feature_table_size_legacy: number of entries in feature table legacy > > > array. > > > + * @validate: the function to call to vaidate features at probe time. > > > > typo > > > > and this is called before probe actually not at probe time > > Thanks, how about the following? > > * @validate: the function to call to validate features ... and config space > > > * @probe: the function to call when a device is found. Returns 0 or > > > -errno. > > > * @scan: optional function to call after successful probe; intended > > > *for virtio-scsi to invoke a scan. > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] iommu/virtio: Detach domain on endpoint release
On Wed, May 10, 2023 at 05:37:22PM +0200, Eric Auger wrote:
> Hi Jean,
>
> On 4/14/23 17:07, Jean-Philippe Brucker wrote:
> > When an endpoint is released, for example a PCIe VF is disabled or a
> > function hot-unplugged, it should be detached from its domain. Send a
> > DETACH request.
> >
> > Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
> > Reported-by: Akihiko Odaki
> > Link:
> > https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41...@daynix.com/
> > Signed-off-by: Jean-Philippe Brucker
> > ---
> > drivers/iommu/virtio-iommu.c | 23 +++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> > index 5b8fe9bfa9a5..3d3d4462359e 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -788,6 +788,28 @@ static int viommu_attach_dev(struct iommu_domain
> > *domain, struct device *dev)
> > return 0;
> > }
> >
> > +static void viommu_detach_dev(struct viommu_endpoint *vdev)
> > +{
> > + int i;
> > + struct virtio_iommu_req_detach req;
> > + struct viommu_domain *vdomain = vdev->vdomain;
> > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
> > +
> > + if (!vdomain)
> > + return;
> > +
> > + req = (struct virtio_iommu_req_detach) {
> > + .head.type = VIRTIO_IOMMU_T_DETACH,
> > + .domain = cpu_to_le32(vdomain->id),
> > + };
> > +
> > + for (i = 0; i < fwspec->num_ids; i++) {
> > + req.endpoint = cpu_to_le32(fwspec->ids[i]);
> > + WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
> > + }
> just a late question: don't you need to decrement vdomain's nr_endpoints?
>
Ah yes, I'll fix that, thank you. I think attach() could use some cleanup
as well: if the request fails then we should keep the nr_endpoints
reference on the previous domain. But that's less urgent.
Thanks,
Jean
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Add missing documentation for structure fields
On Wed, May 10, 2023 at 02:23:12PM +0200, Simon Horman wrote: > Add missing documentation for the vqs_list_lock field of struct virtio_device, > and the validate field of struct virtio_driver. > > ./scripts/kernel-doc says: > > .../virtio.h:131: warning: Function parameter or member 'vqs_list_lock' not > described in 'virtio_device' > .../virtio.h:192: warning: Function parameter or member 'validate' not > described in 'virtio_driver' > 2 warnings as Errors > > No functional changes intended. > > Signed-off-by: Simon Horman > --- > include/linux/virtio.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index b93238db94e3..0b2b82ee3220 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -103,6 +103,7 @@ int virtqueue_resize(struct virtqueue *vq, u32 num, > * @config_enabled: configuration change reporting enabled > * @config_change_pending: configuration change reported while disabled > * @config_lock: protects configuration change reporting > + * @vqs_list_lock: protects @vqs. > * @dev: underlying device. > * @id: the device type identification (used to match it with a driver). > * @config: the configuration ops for this device. > @@ -160,6 +161,7 @@ size_t virtio_max_dma_size(const struct virtio_device > *vdev); > * @feature_table_size: number of entries in the feature table array. > * @feature_table_legacy: same as feature_table but when working in legacy > mode. > * @feature_table_size_legacy: number of entries in feature table legacy > array. > + * @validate: the function to call to vaidate features at probe time. typo and this is called before probe actually not at probe time > * @probe: the function to call when a device is found. Returns 0 or -errno. > * @scan: optional function to call after successful probe; intended > *for virtio-scsi to invoke a scan. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] iommu/virtio: Detach domain on endpoint release
Hi Jean,
On 4/14/23 17:07, Jean-Philippe Brucker wrote:
> When an endpoint is released, for example a PCIe VF is disabled or a
> function hot-unplugged, it should be detached from its domain. Send a
> DETACH request.
>
> Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
> Reported-by: Akihiko Odaki
> Link:
> https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41...@daynix.com/
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/virtio-iommu.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 5b8fe9bfa9a5..3d3d4462359e 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -788,6 +788,28 @@ static int viommu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> return 0;
> }
>
> +static void viommu_detach_dev(struct viommu_endpoint *vdev)
> +{
> + int i;
> + struct virtio_iommu_req_detach req;
> + struct viommu_domain *vdomain = vdev->vdomain;
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
> +
> + if (!vdomain)
> + return;
> +
> + req = (struct virtio_iommu_req_detach) {
> + .head.type = VIRTIO_IOMMU_T_DETACH,
> + .domain = cpu_to_le32(vdomain->id),
> + };
> +
> + for (i = 0; i < fwspec->num_ids; i++) {
> + req.endpoint = cpu_to_le32(fwspec->ids[i]);
> + WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
> + }
just a late question: don't you need to decrement vdomain's nr_endpoints?
Thanks
Eric
> + vdev->vdomain = NULL;
> +}
> +
> static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
> phys_addr_t paddr, size_t pgsize, size_t pgcount,
> int prot, gfp_t gfp, size_t *mapped)
> @@ -990,6 +1012,7 @@ static void viommu_release_device(struct device *dev)
> {
> struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
>
> + viommu_detach_dev(vdev);
> iommu_put_resv_regions(dev, &vdev->resv_regions);
> kfree(vdev);
> }
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vsock: bugfix port residue in server
Hi,
thanks for the patch, the change LGTM, but I have the following
suggestions:
Please avoid "bugfix" in the subject, "fix" should be enough:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes
Anyway, I suggest to change the subject in
"vsock: avoid to close connected socket after the timeout"
On Wed, May 10, 2023 at 10:25:02PM +0800, Zhuang Shengen wrote:
When client and server establish a connection through vsock,
the client send a request to the server to initiate the connection,
then start a timer to wait for the server's response. When the server's
RESPONSE message arrives, the timer also times out and exits. The
server's RESPONSE message is processed first, and the connection is
established. However, the client's timer also times out, the original
processing logic of the client is to directly set the state of this vsock
to CLOSE and return ETIMEDOUT, User will release the port. It will not
What to you mean with "User" here?
notify the server when the port is released, causing the server port remain
Can we remove this blank line?
when client's vsock_connect timeout，it should check sk state is
The remote peer can't trust the other peer, indeed it will receive an
error after sending the first message and it will remove the connection,
right?
ESTABLISHED or not. if sk state is ESTABLISHED, it means the connection
is established, the client should not set the sk state to CLOSE
Note: I encountered this issue on kernel-4.18, which can be fixed by
this patch. Then I checked the latest code in the community
and found similar issue.
In order to backport it to the stable kernels, we should add a Fixes tag:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes
Thanks,
Stefano
Signed-off-by: Zhuang Shengen
---
net/vmw_vsock/af_vsock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 413407bb646c..efb8a0937a13 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1462,7 +1462,7 @@ static int vsock_connect(struct socket *sock, struct
sockaddr *addr,
vsock_transport_cancel_pkt(vsk);
vsock_remove_connected(vsk);
goto out_wait;
- } else if (timeout == 0) {
+ } else if ((sk->sk_state != TCP_ESTABLISHED) && (timeout == 0))
{
err = -ETIMEDOUT;
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
--
2.27.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net v3] virtio_net: Fix error unwinding of XDP initialization
On 2023-05-10 a.m.1:00, Jason Wang wrote:
External email: Use caution opening links or attachments
在 2023/5/9 09:43, Xuan Zhuo 写道:
On Mon, 8 May 2023 11:00:10 -0400, Feng Liu wrote:
On 2023-05-07 p.m.9:45, Xuan Zhuo wrote:
External email: Use caution opening links or attachments
On Sat, 6 May 2023 08:08:02 -0400, Feng Liu wrote:
On 2023-05-05 p.m.10:33, Xuan Zhuo wrote:
External email: Use caution opening links or attachments
On Tue, 2 May 2023 20:35:25 -0400, Feng Liu wrote:
When initializing XDP in virtnet_open(), some rq xdp initialization
may hit an error causing net device open failed. However, previous
rqs have already initialized XDP and enabled NAPI, which is not the
expected behavior. Need to roll back the previous rq initialization
to avoid leaks in error unwinding of init code.
Also extract a helper function of disable queue pairs, and use newly
introduced helper function in error unwinding and virtnet_close;
Issue: 3383038
Fixes: 754b8a21a96d ("virtio_net: setup xdp_rxq_info")
Signed-off-by: Feng Liu
Reviewed-by: William Tu
Reviewed-by: Parav Pandit
Reviewed-by: Simon Horman
Acked-by: Michael S. Tsirkin
Change-Id: Ib4c6a97cb7b837cfa484c593dd43a435c47ea68f
---
drivers/net/virtio_net.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8d8038538fc4..3737cf120cb7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1868,6 +1868,13 @@ static int virtnet_poll(struct napi_struct
*napi, int budget)
return received;
}
+static void virtnet_disable_qp(struct virtnet_info *vi, int
qp_index)
+{
+ virtnet_napi_tx_disable(&vi->sq[qp_index].napi);
+ napi_disable(&vi->rq[qp_index].napi);
+ xdp_rxq_info_unreg(&vi->rq[qp_index].xdp_rxq);
+}
+
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -1883,20 +1890,26 @@ static int virtnet_open(struct net_device
*dev)
err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev,
i, vi->rq[i].napi.napi_id);
if (err < 0)
- return err;
+ goto err_xdp_info_reg;
err =
xdp_rxq_info_reg_mem_model(&vi->rq[i].xdp_rxq,
MEM_TYPE_PAGE_SHARED, NULL);
- if (err < 0) {
- xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
- return err;
- }
+ if (err < 0)
+ goto err_xdp_reg_mem_model;
virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
virtnet_napi_tx_enable(vi, vi->sq[i].vq,
&vi->sq[i].napi);
}
return 0;
+
+err_xdp_reg_mem_model:
+ xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
+err_xdp_info_reg:
+ for (i = i - 1; i >= 0; i--)
+ virtnet_disable_qp(vi, i);
I would to know should we handle for these:
disable_delayed_refill(vi);
cancel_delayed_work_sync(&vi->refill);
Maybe we should call virtnet_close() with "i" directly.
Thanks.
Can’t use i directly here, because if xdp_rxq_info_reg fails, napi has
not been enabled for current qp yet, I should roll back from the queue
pairs where napi was enabled before(i--), otherwise it will hang at
napi
disable api
This is not the point, the key is whether we should handle with:
disable_delayed_refill(vi);
cancel_delayed_work_sync(&vi->refill);
Thanks.
OK, get the point. Thanks for your careful review. And I check the code
again.
There are two points that I need to explain:
1. All refill delay work calls(vi->refill, vi->refill_enabled) are based
on that the virtio interface is successfully opened, such as
virtnet_receive, virtnet_rx_resize, _virtnet_set_queues, etc. If there
is an error in the xdp reg here, it will not trigger these subsequent
functions. There is no need to call disable_delayed_refill() and
cancel_delayed_work_sync().
Maybe something is wrong. I think these lines may call delay work.
static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
int i, err;
enable_delayed_refill(vi);
for (i = 0; i < vi->max_queue_pairs; i++) {
if (i < vi->curr_queue_pairs)
/* Make sure we have some buffers: if oom use
wq. */
--> if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
--> schedule_delayed_work(&vi->refill, 0);
err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i,
vi->rq[i].napi.napi_id);
if (err < 0)
return err;
err = xdp_rxq_info_reg_mem_model(&vi->rq[i].xdp_rxq,
MEM_TYPE_PAGE_SHARED,
NULL);
if (err < 0) {
xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
Re: [RFC PATCH 1/2] mm: multigen-LRU: working set reporting
On Wed, May 10, 2023 at 02:54:18AM +0800, Yuanchu Xie wrote: > From: talumbau Please fix the name here. > > A single patch to be broken up into multiple patches. What does this mean? > - Add working set reporting structure. > - Add per-node and per-memcg interfaces for working set reporting. > - Implement working set backend for MGLRU. Please break it up to be reviewable, otherwise no one will review it. thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] iommu/virtio: Detach domain on endpoint release
Hi Joerg,
On Fri, Apr 14, 2023 at 04:07:45PM +0100, Jean-Philippe Brucker wrote:
> When an endpoint is released, for example a PCIe VF is disabled or a
> function hot-unplugged, it should be detached from its domain. Send a
> DETACH request.
>
> Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
> Reported-by: Akihiko Odaki
> Link:
> https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41...@daynix.com/
> Signed-off-by: Jean-Philippe Brucker
This patch fixes device unregistration in the virtio-iommu driver, could
you please pick it up for the next batch of fixes? It applies cleanly on
v6.4-rc1
Thanks,
Jean
> ---
> drivers/iommu/virtio-iommu.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 5b8fe9bfa9a5..3d3d4462359e 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -788,6 +788,28 @@ static int viommu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> return 0;
> }
>
> +static void viommu_detach_dev(struct viommu_endpoint *vdev)
> +{
> + int i;
> + struct virtio_iommu_req_detach req;
> + struct viommu_domain *vdomain = vdev->vdomain;
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
> +
> + if (!vdomain)
> + return;
> +
> + req = (struct virtio_iommu_req_detach) {
> + .head.type = VIRTIO_IOMMU_T_DETACH,
> + .domain = cpu_to_le32(vdomain->id),
> + };
> +
> + for (i = 0; i < fwspec->num_ids; i++) {
> + req.endpoint = cpu_to_le32(fwspec->ids[i]);
> + WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
> + }
> + vdev->vdomain = NULL;
> +}
> +
> static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
> phys_addr_t paddr, size_t pgsize, size_t pgcount,
> int prot, gfp_t gfp, size_t *mapped)
> @@ -990,6 +1012,7 @@ static void viommu_release_device(struct device *dev)
> {
> struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
>
> + viommu_detach_dev(vdev);
> iommu_put_resv_regions(dev, &vdev->resv_regions);
> kfree(vdev);
> }
> --
> 2.40.0
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: [PATCH] virtio_ring: use u32 for virtio_max_dma_size
On 5/10/23 11:26, Xuan Zhuo wrote:
On Wed, 10 May 2023 10:54:37 +0800, zhenwei pi wrote:
Both split ring and packed ring use 32bits to describe the length of
a descriptor: see struct vring_desc and struct vring_packed_desc.
This means the max segment size supported by virtio is U32_MAX.
An example of virtio_max_dma_size in virtio_blk.c:
u32 v, max_size;
max_size = virtio_max_dma_size(vdev); -> implicit convert
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
struct virtio_blk_config, size_max, &v);
max_size = min(max_size, v);
There is a risk during implicit convert here, once virtio_max_dma_size
returns 4G, max_size becomes 0.
Fixes: e6d6dd6c875e ("virtio: Introduce virtio_max_dma_size()")
Cc: Joerg Roedel
Signed-off-by: zhenwei pi
---
drivers/virtio/virtio_ring.c | 12
include/linux/virtio.h | 2 +-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c5310eaf8b46..55cfecf030a1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -289,12 +289,16 @@ static bool vring_use_dma_api(const struct virtio_device
*vdev)
return false;
}
-size_t virtio_max_dma_size(const struct virtio_device *vdev)
+u32 virtio_max_dma_size(const struct virtio_device *vdev)
LGTM
But, should we change the parameter to vq, then use the dma_dev?
@Jason
Thanks.
The max DMA size is a attribute of a virtio device rather than any VQ,
so I guess virtio_max_dma_size(const struct virtio_device *vdev) is clear.
On the other hand, if changing the parameter to vq, we need select a VQ,
then the question is:
1, which VQ to select? VQ0 or a random one? this leads confusing.
2, The virtio spec defines: Each device can have zero or more virtqueues
{
- size_t max_segment_size = SIZE_MAX;
+ u32 max_segment_size = U32_MAX;
- if (vring_use_dma_api(vdev))
- max_segment_size = dma_max_mapping_size(vdev->dev.parent);
+ if (vring_use_dma_api(vdev)) {
+ size_t max_dma_size = dma_max_mapping_size(vdev->dev.parent);
+
+ if (max_dma_size < max_segment_size)
+ max_segment_size = max_dma_size;
+ }
return max_segment_size;
}
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b93238db94e3..1a605f408329 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -147,7 +147,7 @@ int virtio_device_restore(struct virtio_device *dev);
#endif
void virtio_reset_device(struct virtio_device *dev);
-size_t virtio_max_dma_size(const struct virtio_device *vdev);
+u32 virtio_max_dma_size(const struct virtio_device *vdev);
#define virtio_device_for_each_vq(vdev, vq) \
list_for_each_entry(vq, &vdev->vqs, list)
--
2.20.1
--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
