[PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Feng Liu via Virtualization]

This patch series performs a clean up of the code in virtio_ring and
virtio_pci, modifying it to conform with the Linux kernel coding style
guidance [1]. The modifications ensure the code easy to read and
understand. This small series does few short cleanups in the code.

Patch-1 Allow non power of 2 sizes for virtqueues
Patch-2 Avoid using inline for small functions.
Patch-3 Use const to annotate read-only pointer params.

[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease

All of the patches have been verified based on the kernel code
commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")

Feng Liu (3):
  virtio_pci_modern: Allow non power of 2 sizes for virtqueues
  virtio_ring: Avoid using inline for small functions
  virtio_ring: Use const to annotate read-only pointer params

 drivers/virtio/virtio_pci_modern.c |  5 ----
 drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
 include/linux/virtio.h             | 14 ++++-----
 3 files changed, 31 insertions(+), 36 deletions(-)

-- 
2.34.1

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

[PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Feng Liu via Virtualization]

According to the Virtio Specification, the Queue Size parameter of a
virtqueue corresponds to the maximum number of descriptors in that
queue, and it does not have to be a power of 2 for packed virtqueues.
However, the virtio_pci_modern driver enforced a power of 2 check for
virtqueue sizes, which is unnecessary and restrictive. This commit
removes that check, allowing virtqueues to have arbitrary sizes as
specified by the Virtio Specification.

To validate this change, we tested various virtqueue sizes for packed
rings, including 128, 256, 512, 100, 200, 500, and 1000, with
CONFIG_PAGE_POISONING enabled, and all tests passed successfully.

Signed-off-by: Feng Liu <feliu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Bodong Wang <bodong@nvidia.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>

---
v1 -> v2
feedbacks from Jason Wang and Michael S. Tsirkin
- remove power_of_2 check of virtqueue size
---
 drivers/virtio/virtio_pci_modern.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 9e496e288cfa..6e713904d8e8 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -310,11 +310,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
 	if (!num || vp_modern_get_queue_enable(mdev, index))
 		return ERR_PTR(-ENOENT);
 
-	if (!is_power_of_2(num)) {
-		dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
-		return ERR_PTR(-EINVAL);
-	}
-
 	info->msix_vector = msix_vec;
 
 	/* create the vring */
-- 
2.34.1

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

RE: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Parav Pandit via Virtualization]

> From: Feng Liu <feliu@nvidia.com>
> Sent: Friday, March 10, 2023 12:34 AM

> 
> -	if (!is_power_of_2(num)) {
> -		dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
> num);
> -		return ERR_PTR(-EINVAL);
> -	}
> -

The check is still valid for split q.
Maybe the right place for such a check is not the pci transport driver.
But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Feng Liu via Virtualization]

On 2023-03-10 a.m.8:36, Parav Pandit wrote:
> 
> 
>> From: Feng Liu <feliu@nvidia.com>
>> Sent: Friday, March 10, 2023 12:34 AM
> 
>>
>> -	if (!is_power_of_2(num)) {
>> -		dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
>> num);
>> -		return ERR_PTR(-EINVAL);
>> -	}
>> -
> 
> The check is still valid for split q.
> Maybe the right place for such a check is not the pci transport driver.
> But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.

Hi, Parav
     I think you are right, I checked the virtio spec, only packed 
virtqueue can use queue size which is not power_of_2; so, I think the 
check can be reserved only for split queue here, something like

struct virtio_device *vdev = &vp_dev->vdev;
if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
  && !is_power_of_2(num)){
     dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
     return ERR_PTR(-EINVAL);
}

I will fix it in next version

Hi, Michael and Jason
Do you have any comments on this?




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

Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Michael S. Tsirkin]

On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
> 
> 
> On 2023-03-10 a.m.8:36, Parav Pandit wrote:
> > 
> > 
> > > From: Feng Liu <feliu@nvidia.com>
> > > Sent: Friday, March 10, 2023 12:34 AM
> > 
> > > 
> > > -	if (!is_power_of_2(num)) {
> > > -		dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
> > > num);
> > > -		return ERR_PTR(-EINVAL);
> > > -	}
> > > -
> > 
> > The check is still valid for split q.
> > Maybe the right place for such a check is not the pci transport driver.
> > But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.
> 
> Hi, Parav
>     I think you are right, I checked the virtio spec, only packed virtqueue
> can use queue size which is not power_of_2; so, I think the check can be
> reserved only for split queue here, something like
> 
> struct virtio_device *vdev = &vp_dev->vdev;
> if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
>  && !is_power_of_2(num)){
>     dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
>     return ERR_PTR(-EINVAL);
> }
> 
> I will fix it in next version
> 
> Hi, Michael and Jason
> Do you have any comments on this?
> 

Hmm good point. Which raises the question: so how did you test it then?


-- 
MST

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

Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Feng Liu via Virtualization]

On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
>>
>>
>> On 2023-03-10 a.m.8:36, Parav Pandit wrote:
>>>
>>>
>>>> From: Feng Liu <feliu@nvidia.com>
>>>> Sent: Friday, March 10, 2023 12:34 AM
>>>
>>>>
>>>> - if (!is_power_of_2(num)) {
>>>> -         dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
>>>> num);
>>>> -         return ERR_PTR(-EINVAL);
>>>> - }
>>>> -
>>>
>>> The check is still valid for split q.
>>> Maybe the right place for such a check is not the pci transport driver.
>>> But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.
>>
>> Hi, Parav
>>      I think you are right, I checked the virtio spec, only packed virtqueue
>> can use queue size which is not power_of_2; so, I think the check can be
>> reserved only for split queue here, something like
>>
>> struct virtio_device *vdev = &vp_dev->vdev;
>> if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
>>   && !is_power_of_2(num)){
>>      dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
>>      return ERR_PTR(-EINVAL);
>> }
>>
>> I will fix it in next version
>>
>> Hi, Michael and Jason
>> Do you have any comments on this?
>>
> 
> Hmm good point. Which raises the question: so how did you test it then?
> 
Hi Michael

I will construct a non power of 2 size packed virtqueue device to test 
whether the driver is loaded successfully and whether the traffic is 
normal; at the same time, I will also construct a non power of 2 size 
split virtqueue device for testing to see if an warning is given and the 
driver is loaded fail

The method of constructing the device, such as the reply steps in the 
previous email

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

Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Michael S. Tsirkin]

On Sat, Mar 11, 2023 at 05:25:04PM -0500, Feng Liu wrote:
> 
> 
> On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
> > > 
> > > 
> > > On 2023-03-10 a.m.8:36, Parav Pandit wrote:
> > > > 
> > > > 
> > > > > From: Feng Liu <feliu@nvidia.com>
> > > > > Sent: Friday, March 10, 2023 12:34 AM
> > > > 
> > > > > 
> > > > > - if (!is_power_of_2(num)) {
> > > > > -         dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
> > > > > num);
> > > > > -         return ERR_PTR(-EINVAL);
> > > > > - }
> > > > > -
> > > > 
> > > > The check is still valid for split q.
> > > > Maybe the right place for such a check is not the pci transport driver.
> > > > But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.
> > > 
> > > Hi, Parav
> > >      I think you are right, I checked the virtio spec, only packed virtqueue
> > > can use queue size which is not power_of_2; so, I think the check can be
> > > reserved only for split queue here, something like
> > > 
> > > struct virtio_device *vdev = &vp_dev->vdev;
> > > if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
> > >   && !is_power_of_2(num)){
> > >      dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
> > >      return ERR_PTR(-EINVAL);
> > > }
> > > 
> > > I will fix it in next version
> > > 
> > > Hi, Michael and Jason
> > > Do you have any comments on this?
> > > 
> > 
> > Hmm good point. Which raises the question: so how did you test it then?
> > 
> Hi Michael
> 
> I will construct a non power of 2 size packed virtqueue device to test
> whether the driver is loaded successfully and whether the traffic is normal;
> at the same time, I will also construct a non power of 2 size split
> virtqueue device for testing to see if an warning is given and the driver is
> loaded fail
> 
> The method of constructing the device, such as the reply steps in the
> previous email

Okay but previously you said you tested ring sizes 100 and 200 with
iperf. How did you construct these?


> > 
> > --
> > MST
> > 

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

Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues

[Feng Liu via Virtualization]

On 2023-03-12 a.m.1:46, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Sat, Mar 11, 2023 at 05:25:04PM -0500, Feng Liu wrote:
>>
>>
>> On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
>>>>
>>>>
>>>> On 2023-03-10 a.m.8:36, Parav Pandit wrote:
>>>>>
>>>>>
>>>>>> From: Feng Liu <feliu@nvidia.com>
>>>>>> Sent: Friday, March 10, 2023 12:34 AM
>>>>>
>>>>>>
>>>>>> - if (!is_power_of_2(num)) {
>>>>>> -         dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u",
>>>>>> num);
>>>>>> -         return ERR_PTR(-EINVAL);
>>>>>> - }
>>>>>> -
>>>>>
>>>>> The check is still valid for split q.
>>>>> Maybe the right place for such a check is not the pci transport driver.
>>>>> But layer below where split vs packed q knowledge resides and hence, power of 2 check can be omitted for packed vq.
>>>>
>>>> Hi, Parav
>>>>       I think you are right, I checked the virtio spec, only packed virtqueue
>>>> can use queue size which is not power_of_2; so, I think the check can be
>>>> reserved only for split queue here, something like
>>>>
>>>> struct virtio_device *vdev = &vp_dev->vdev;
>>>> if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
>>>>    && !is_power_of_2(num)){
>>>>       dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
>>>>       return ERR_PTR(-EINVAL);
>>>> }
>>>>
>>>> I will fix it in next version
>>>>
>>>> Hi, Michael and Jason
>>>> Do you have any comments on this?
>>>>
>>>
>>> Hmm good point. Which raises the question: so how did you test it then?
>>>
>> Hi Michael
>>
>> I will construct a non power of 2 size packed virtqueue device to test
>> whether the driver is loaded successfully and whether the traffic is normal;
>> at the same time, I will also construct a non power of 2 size split
>> virtqueue device for testing to see if an warning is given and the driver is
>> loaded fail
>>
>> The method of constructing the device, such as the reply steps in the
>> previous email
> 
> Okay but previously you said you tested ring sizes 100 and 200 with
> iperf. How did you construct these?
> 

Hi, Michael

After the driver has been successfully loaded, I will use iperf to test 
whether the network traffic is functioning properly. I will start the 
iperf server on the peer-end and the iperf client within the vm. I will 
then initiate a data transfer to perform the test.

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

[PATCH v2 2/3] virtio_ring: Avoid using inline for small functions

[Feng Liu via Virtualization]

According to kernel coding style [1], defining inline functions is not
necessary and beneficial for simple functions. Hence clean up the code
by removing the inline keyword.

It is verified with GCC 12.2.0, the generated code with/without inline
is same. Additionally tested with pktgen and iperf, and verified the
result, the pps test results are the same in the cases of with/without
inline.

Iperf and pps of pktgen for virtio-net didn't change before and after
the change.

[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease

Signed-off-by: Feng Liu <feliu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Bodong Wang <bodong@nvidia.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
---
 drivers/virtio/virtio_ring.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..a26fab91c59f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
 
 #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
 
-static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
-					  unsigned int total_sg)
+static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+				   unsigned int total_sg)
 {
 	/*
 	 * If the host supports indirect descriptor tables, and we have multiple
@@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev, size_t size,
  * making all of the arch DMA ops work on the vring device itself
  * is a mess.
  */
-static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
+static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
 {
 	return vq->dma_dev;
 }
@@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
 	}
 }
 
-static inline bool more_used_split(const struct vring_virtqueue *vq)
+static bool more_used_split(const struct vring_virtqueue *vq)
 {
 	return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
 			vq->split.vring.used->idx);
@@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue *_vq, u32 num)
 /*
  * Packed ring specific functions - *_packed().
  */
-static inline bool packed_used_wrap_counter(u16 last_used_idx)
+static bool packed_used_wrap_counter(u16 last_used_idx)
 {
 	return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
 }
 
-static inline u16 packed_last_used(u16 last_used_idx)
+static u16 packed_last_used(u16 last_used_idx)
 {
 	return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
 }
@@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct vring_virtqueue *vq,
 	return avail == used && used == used_wrap_counter;
 }
 
-static inline bool more_used_packed(const struct vring_virtqueue *vq)
+static bool more_used_packed(const struct vring_virtqueue *vq)
 {
 	u16 last_used;
 	u16 last_used_idx;
-- 
2.34.1

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

[PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Feng Liu via Virtualization]

Add const to make the read-only pointer parameters clear, similar to
many existing functions.

To implement this change, the commit also introduces the use of
`container_of_const` to implement `to_vvq`, which ensures the const-ness
of read-only parameters and avoids accidental modification of their
members.

Signed-off-by: Feng Liu <feliu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Gavin Li <gavinl@nvidia.com>
Reviewed-by: Bodong Wang <bodong@nvidia.com>

---
v1 -> v2
feedbacks from Michael S. Tsirkin
- use `container_of_const` to implement `to_vvq`
---
 drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
 include/linux/virtio.h       | 14 +++++++-------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a26fab91c59f..4c3bb0ddeb9b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
  * Helpers.
  */
 
-#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
 
-static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
 				   unsigned int total_sg)
 {
 	/*
@@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
  * unconditionally on data path.
  */
 
-static bool vring_use_dma_api(struct virtio_device *vdev)
+static bool vring_use_dma_api(const struct virtio_device *vdev)
 {
 	if (!virtio_has_dma_quirk(vdev))
 		return true;
@@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
 	return false;
 }
 
-size_t virtio_max_dma_size(struct virtio_device *vdev)
+size_t virtio_max_dma_size(const struct virtio_device *vdev)
 {
 	size_t max_segment_size = SIZE_MAX;
 
@@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
  */
 
 static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
-					   struct vring_desc *desc)
+					   const struct vring_desc *desc)
 {
 	u16 flags;
 
@@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
 }
 
 static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
-				     struct vring_desc_extra *extra)
+				     const struct vring_desc_extra *extra)
 {
 	u16 flags;
 
@@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
 }
 
 static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
-				   struct vring_packed_desc *desc)
+				    const struct vring_packed_desc *desc)
 {
 	u16 flags;
 
@@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
  * Returns the size of the vring.  This is mainly used for boasting to
  * userspace.  Unlike other operations, this need not be serialized.
  */
-unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
+unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
 {
 
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
 }
@@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
 
-bool virtqueue_is_broken(struct virtqueue *_vq)
+bool virtqueue_is_broken(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	return READ_ONCE(vq->broken);
 }
@@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
 
-dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
 
-dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
 
-dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
 EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
 
 /* Only available for split ring */
-const struct vring *virtqueue_get_vring(struct virtqueue *vq)
+const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
 {
 	return &to_vvq(vq)->split.vring;
 }
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 2b472514c49b..c4225653f949 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
 
 void *virtqueue_detach_unused_buf(struct virtqueue *vq);
 
-unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
+unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
 
-bool virtqueue_is_broken(struct virtqueue *vq);
+bool virtqueue_is_broken(const struct virtqueue *vq);
 
-const struct vring *virtqueue_get_vring(struct virtqueue *vq);
-dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
-dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
-dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
+const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
 
 int virtqueue_resize(struct virtqueue *vq, u32 num,
 		     void (*recycle)(struct virtqueue *vq, void *buf));
@@ -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(struct virtio_device *vdev);
+size_t 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.34.1

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

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Michael S. Tsirkin]

On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> This patch series performs a clean up of the code in virtio_ring and
> virtio_pci, modifying it to conform with the Linux kernel coding style
> guidance [1]. The modifications ensure the code easy to read and
> understand. This small series does few short cleanups in the code.
> 
> Patch-1 Allow non power of 2 sizes for virtqueues
> Patch-2 Avoid using inline for small functions.
> Patch-3 Use const to annotate read-only pointer params.
> 
> [1]
> https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
> 
> All of the patches have been verified based on the kernel code
> commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")

verified how?

> Feng Liu (3):
>   virtio_pci_modern: Allow non power of 2 sizes for virtqueues
>   virtio_ring: Avoid using inline for small functions
>   virtio_ring: Use const to annotate read-only pointer params
> 
>  drivers/virtio/virtio_pci_modern.c |  5 ----
>  drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
>  include/linux/virtio.h             | 14 ++++-----
>  3 files changed, 31 insertions(+), 36 deletions(-)
> 
> -- 
> 2.34.1

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

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Feng Liu via Virtualization]

On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
>> This patch series performs a clean up of the code in virtio_ring and
>> virtio_pci, modifying it to conform with the Linux kernel coding style
>> guidance [1]. The modifications ensure the code easy to read and
>> understand. This small series does few short cleanups in the code.
>>
>> Patch-1 Allow non power of 2 sizes for virtqueues
>> Patch-2 Avoid using inline for small functions.
>> Patch-3 Use const to annotate read-only pointer params.
>>
>> [1]
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C08831607a6fb4f58881408db213f8638%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638140328946332918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FnD4GINUds2HLLo47aY5Ps%2B9nKWPW2XRI35z1Hp0yx4%3D&reserved=0
>>
>> All of the patches have been verified based on the kernel code
>> commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> 
> verified how?
> 
Hi Michael
1. Applied the patches on lastest kernel source(44889ba56cbb), compile 
and install the kernel, and use iperf to test traffic
2. To validate this change, we tested various virtqueue sizes for packed
rings, including 128, 256, 512, 100, 200, 500, and 1000, with
CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests 
passed successfully.


>> Feng Liu (3):
>>    virtio_pci_modern: Allow non power of 2 sizes for virtqueues
>>    virtio_ring: Avoid using inline for small functions
>>    virtio_ring: Use const to annotate read-only pointer params
>>
>>   drivers/virtio/virtio_pci_modern.c |  5 ----
>>   drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
>>   include/linux/virtio.h             | 14 ++++-----
>>   3 files changed, 31 insertions(+), 36 deletions(-)
>>
>> --
>> 2.34.1
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Michael S. Tsirkin]

On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
> 
> 
> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> > > This patch series performs a clean up of the code in virtio_ring and
> > > virtio_pci, modifying it to conform with the Linux kernel coding style
> > > guidance [1]. The modifications ensure the code easy to read and
> > > understand. This small series does few short cleanups in the code.
> > > 
> > > Patch-1 Allow non power of 2 sizes for virtqueues
> > > Patch-2 Avoid using inline for small functions.
> > > Patch-3 Use const to annotate read-only pointer params.
> > > 
> > > [1]
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C08831607a6fb4f58881408db213f8638%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638140328946332918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FnD4GINUds2HLLo47aY5Ps%2B9nKWPW2XRI35z1Hp0yx4%3D&reserved=0
> > > 
> > > All of the patches have been verified based on the kernel code
> > > commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> > 
> > verified how?
> > 
> Hi Michael
> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
> install the kernel, and use iperf to test traffic
> 2. To validate this change, we tested various virtqueue sizes for packed
> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
> passed successfully.

Given split ring does not support non power of 2 how exactly
did you configure non power of 2?

> 
> > > Feng Liu (3):
> > >    virtio_pci_modern: Allow non power of 2 sizes for virtqueues
> > >    virtio_ring: Avoid using inline for small functions
> > >    virtio_ring: Use const to annotate read-only pointer params
> > > 
> > >   drivers/virtio/virtio_pci_modern.c |  5 ----
> > >   drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
> > >   include/linux/virtio.h             | 14 ++++-----
> > >   3 files changed, 31 insertions(+), 36 deletions(-)
> > > 
> > > --
> > > 2.34.1
> > 

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

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Feng Liu via Virtualization]

On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
>>
>>
>> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
>>>> This patch series performs a clean up of the code in virtio_ring and
>>>> virtio_pci, modifying it to conform with the Linux kernel coding style
>>>> guidance [1]. The modifications ensure the code easy to read and
>>>> understand. This small series does few short cleanups in the code.
>>>>
>>>> Patch-1 Allow non power of 2 sizes for virtqueues
>>>> Patch-2 Avoid using inline for small functions.
>>>> Patch-3 Use const to annotate read-only pointer params.
>>>>
>>>> [1]
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D&reserved=0
>>>>
>>>> All of the patches have been verified based on the kernel code
>>>> commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
>>>
>>> verified how?
>>>
>> Hi Michael
>> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
>> install the kernel, and use iperf to test traffic
>> 2. To validate this change, we tested various virtqueue sizes for packed
>> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
>> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
>> passed successfully.
> 
> Given split ring does not support non power of 2 how exactly
> did you configure non power of 2?
> 

Hi, Michael
We can implement the test by modifying qemu; 1. force the 
VIRTIO_F_RING_PACKED feature bit to be set, 2. set 
VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to 
the value of non power_of_2, 3. remove the check of is_power_of 
virtqueue, then qemu can create the required virtual device (non power_2 
size , packed virtqueue device) ;In this way, any length and packed ring 
test can be performed;
remove the modified code, I can test split vq, and can see that the size 
of power_of_2 can load the driver normally, and the size of non 
power_of_2 will give an warning and fail to load the driver

>>
>>>> Feng Liu (3):
>>>>     virtio_pci_modern: Allow non power of 2 sizes for virtqueues
>>>>     virtio_ring: Avoid using inline for small functions
>>>>     virtio_ring: Use const to annotate read-only pointer params
>>>>
>>>>    drivers/virtio/virtio_pci_modern.c |  5 ----
>>>>    drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
>>>>    include/linux/virtio.h             | 14 ++++-----
>>>>    3 files changed, 31 insertions(+), 36 deletions(-)
>>>>
>>>> --
>>>> 2.34.1
>>>
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Michael S. Tsirkin]

On Sat, Mar 11, 2023 at 05:19:43PM -0500, Feng Liu wrote:
> 
> 
> On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
> > > 
> > > 
> > > On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> > > > > This patch series performs a clean up of the code in virtio_ring and
> > > > > virtio_pci, modifying it to conform with the Linux kernel coding style
> > > > > guidance [1]. The modifications ensure the code easy to read and
> > > > > understand. This small series does few short cleanups in the code.
> > > > > 
> > > > > Patch-1 Allow non power of 2 sizes for virtqueues
> > > > > Patch-2 Avoid using inline for small functions.
> > > > > Patch-3 Use const to annotate read-only pointer params.
> > > > > 
> > > > > [1]
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D&reserved=0
> > > > > 
> > > > > All of the patches have been verified based on the kernel code
> > > > > commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> > > > 
> > > > verified how?
> > > > 
> > > Hi Michael
> > > 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
> > > install the kernel, and use iperf to test traffic
> > > 2. To validate this change, we tested various virtqueue sizes for packed
> > > rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> > > CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
> > > passed successfully.
> > 
> > Given split ring does not support non power of 2 how exactly
> > did you configure non power of 2?
> > 
> 
> Hi, Michael
> We can implement the test by modifying qemu; 1. force the
> VIRTIO_F_RING_PACKED feature bit to be set, 2. set
> VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to the
> value of non power_of_2, 3. remove the check of is_power_of virtqueue, then
> qemu can create the required virtual device (non power_2 size , packed
> virtqueue device) ;In this way, any length and packed ring test can be
> performed;
> remove the modified code, I can test split vq, and can see that the size of
> power_of_2 can load the driver normally, and the size of non power_of_2 will
> give an warning and fail to load the driver

Sounds like a plan but what exactly did you do previously?
You indicated you tested non powers of 2.

> > > 
> > > > > Feng Liu (3):
> > > > >     virtio_pci_modern: Allow non power of 2 sizes for virtqueues
> > > > >     virtio_ring: Avoid using inline for small functions
> > > > >     virtio_ring: Use const to annotate read-only pointer params
> > > > > 
> > > > >    drivers/virtio/virtio_pci_modern.c |  5 ----
> > > > >    drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
> > > > >    include/linux/virtio.h             | 14 ++++-----
> > > > >    3 files changed, 31 insertions(+), 36 deletions(-)
> > > > > 
> > > > > --
> > > > > 2.34.1
> > > > 
> > 

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

Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Feng Liu via Virtualization]

On 2023-03-12 a.m.1:47, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Sat, Mar 11, 2023 at 05:19:43PM -0500, Feng Liu wrote:
>>
>>
>> On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
>>>>
>>>>
>>>> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
>>>>>> This patch series performs a clean up of the code in virtio_ring and
>>>>>> virtio_pci, modifying it to conform with the Linux kernel coding style
>>>>>> guidance [1]. The modifications ensure the code easy to read and
>>>>>> understand. This small series does few short cleanups in the code.
>>>>>>
>>>>>> Patch-1 Allow non power of 2 sizes for virtqueues
>>>>>> Patch-2 Avoid using inline for small functions.
>>>>>> Patch-3 Use const to annotate read-only pointer params.
>>>>>>
>>>>>> [1]
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease&data=05%7C01%7Cfeliu%40nvidia.com%7C329325e3773c46e5cadc08db22c5bc87%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638142004918816377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=57roDDMfOEOIuEo%2FqgBgUklMBGHfdSWRI0KTRXQCNgM%3D&reserved=0
>>>>>>
>>>>>> All of the patches have been verified based on the kernel code
>>>>>> commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
>>>>>
>>>>> verified how?
>>>>>
>>>> Hi Michael
>>>> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
>>>> install the kernel, and use iperf to test traffic
>>>> 2. To validate this change, we tested various virtqueue sizes for packed
>>>> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
>>>> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
>>>> passed successfully.
>>>
>>> Given split ring does not support non power of 2 how exactly
>>> did you configure non power of 2?
>>>
>>
>> Hi, Michael
>> We can implement the test by modifying qemu; 1. force the
>> VIRTIO_F_RING_PACKED feature bit to be set, 2. set
>> VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to the
>> value of non power_of_2, 3. remove the check of is_power_of virtqueue, then
>> qemu can create the required virtual device (non power_2 size , packed
>> virtqueue device) ;In this way, any length and packed ring test can be
>> performed;
>> remove the modified code, I can test split vq, and can see that the size of
>> power_of_2 can load the driver normally, and the size of non power_of_2 will
>> give an warning and fail to load the driver
> 
> Sounds like a plan but what exactly did you do previously?
> You indicated you tested non powers of 2.
> 

Hi, Michael
It is not just a plan. Previously, I have already created the 
non_power_of_2 packed virtqueue device and the standard split vq device 
for test, and after startup, I performed related tests using iperf and 
ping -f. This time I will construct a split vq device of non_power_of_2 
virtqueue for more abnormal tests

>>>>
>>>>>> Feng Liu (3):
>>>>>>      virtio_pci_modern: Allow non power of 2 sizes for virtqueues
>>>>>>      virtio_ring: Avoid using inline for small functions
>>>>>>      virtio_ring: Use const to annotate read-only pointer params
>>>>>>
>>>>>>     drivers/virtio/virtio_pci_modern.c |  5 ----
>>>>>>     drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
>>>>>>     include/linux/virtio.h             | 14 ++++-----
>>>>>>     3 files changed, 31 insertions(+), 36 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>
>>>
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci

[Feng Liu via Virtualization]

This patch series performs a clean up of the code in virtio_ring and
virtio_pci, modifying it to conform with the Linux kernel coding style
guidance [1]. The modifications ensure the code easy to read and
understand. This small series does few short cleanups in the code.

Patch-1 Allow non power of 2 sizes for packed virtqueues.
Patch-2 Avoid using inline for small functions.
Patch-3 Use const to annotate read-only pointer params.

[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease

All of the patches have been verified based on the kernel code
commit 81ff855485a3 ("Merge tag 'i2c-for-6.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux")

Feng Liu (3):
  virtio_ring: Allow non power of 2 sizes for packed virtqueue
  virtio_ring: Avoid using inline for small functions
  virtio_ring: Use const to annotate read-only pointer params

 drivers/virtio/virtio_pci_modern.c |  5 ----
 drivers/virtio/virtio_ring.c       | 48 +++++++++++++++---------------
 include/linux/virtio.h             | 14 ++++-----
 3 files changed, 31 insertions(+), 36 deletions(-)

-- 
2.34.1

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

[PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Feng Liu via Virtualization]

Add const to make the read-only pointer parameters clear, similar to
many existing functions.

Use `container_of_const` to implement `to_vvq`, which ensures the
const-ness of read-only parameters and avoids accidental modification
of their members.

Signed-off-by: Feng Liu <feliu@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>

---
v0 -> v1
feedbacks from Michael S. Tsirkin
- use `container_of_const` to implement `to_vvq`
---
 drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
 include/linux/virtio.h       | 14 +++++++-------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a26fab91c59f..4c3bb0ddeb9b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
  * Helpers.
  */
 
-#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
 
-static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
 				   unsigned int total_sg)
 {
 	/*
@@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
  * unconditionally on data path.
  */
 
-static bool vring_use_dma_api(struct virtio_device *vdev)
+static bool vring_use_dma_api(const struct virtio_device *vdev)
 {
 	if (!virtio_has_dma_quirk(vdev))
 		return true;
@@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
 	return false;
 }
 
-size_t virtio_max_dma_size(struct virtio_device *vdev)
+size_t virtio_max_dma_size(const struct virtio_device *vdev)
 {
 	size_t max_segment_size = SIZE_MAX;
 
@@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
  */
 
 static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
-					   struct vring_desc *desc)
+					   const struct vring_desc *desc)
 {
 	u16 flags;
 
@@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
 }
 
 static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
-				     struct vring_desc_extra *extra)
+				     const struct vring_desc_extra *extra)
 {
 	u16 flags;
 
@@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
 }
 
 static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
-				   struct vring_packed_desc *desc)
+				    const struct vring_packed_desc *desc)
 {
 	u16 flags;
 
@@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
  * Returns the size of the vring.  This is mainly used for boasting to
  * userspace.  Unlike other operations, this need not be serialized.
  */
-unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
+unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
 {
 
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
 }
@@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
 
-bool virtqueue_is_broken(struct virtqueue *_vq)
+bool virtqueue_is_broken(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	return READ_ONCE(vq->broken);
 }
@@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
 
-dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
 
-dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
 
-dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
+dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
 {
-	struct vring_virtqueue *vq = to_vvq(_vq);
+	const struct vring_virtqueue *vq = to_vvq(_vq);
 
 	BUG_ON(!vq->we_own_ring);
 
@@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
 EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
 
 /* Only available for split ring */
-const struct vring *virtqueue_get_vring(struct virtqueue *vq)
+const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
 {
 	return &to_vvq(vq)->split.vring;
 }
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 2b472514c49b..c4225653f949 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
 
 void *virtqueue_detach_unused_buf(struct virtqueue *vq);
 
-unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
+unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
 
-bool virtqueue_is_broken(struct virtqueue *vq);
+bool virtqueue_is_broken(const struct virtqueue *vq);
 
-const struct vring *virtqueue_get_vring(struct virtqueue *vq);
-dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
-dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
-dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
+const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
+dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
 
 int virtqueue_resize(struct virtqueue *vq, u32 num,
 		     void (*recycle)(struct virtqueue *vq, void *buf));
@@ -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(struct virtio_device *vdev);
+size_t 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.34.1

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

Re: [PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Jason Wang]

On Thu, Mar 16, 2023 at 2:55 AM Feng Liu <feliu@nvidia.com> wrote:
>
> Add const to make the read-only pointer parameters clear, similar to
> many existing functions.
>
> Use `container_of_const` to implement `to_vvq`, which ensures the
> const-ness of read-only parameters and avoids accidental modification
> of their members.
>
> Signed-off-by: Feng Liu <feliu@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>
> ---
> v0 -> v1
> feedbacks from Michael S. Tsirkin
> - use `container_of_const` to implement `to_vvq`
> ---
>  drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
>  include/linux/virtio.h       | 14 +++++++-------
>  2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index a26fab91c59f..4c3bb0ddeb9b 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
>   * Helpers.
>   */
>
> -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
> +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
>
> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
>                                    unsigned int total_sg)
>  {
>         /*
> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>   * unconditionally on data path.
>   */
>
> -static bool vring_use_dma_api(struct virtio_device *vdev)
> +static bool vring_use_dma_api(const struct virtio_device *vdev)
>  {
>         if (!virtio_has_dma_quirk(vdev))
>                 return true;
> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>         return false;
>  }
>
> -size_t virtio_max_dma_size(struct virtio_device *vdev)
> +size_t virtio_max_dma_size(const struct virtio_device *vdev)
>  {
>         size_t max_segment_size = SIZE_MAX;
>
> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
>   */
>
>  static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
> -                                          struct vring_desc *desc)
> +                                          const struct vring_desc *desc)
>  {
>         u16 flags;
>
> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
>  }
>
>  static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
> -                                    struct vring_desc_extra *extra)
> +                                    const struct vring_desc_extra *extra)
>  {
>         u16 flags;
>
> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>  }
>
>  static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
> -                                  struct vring_packed_desc *desc)
> +                                   const struct vring_packed_desc *desc)
>  {
>         u16 flags;
>
> @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
>   * Returns the size of the vring.  This is mainly used for boasting to
>   * userspace.  Unlike other operations, this need not be serialized.
>   */
> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
>  {
>
> -       struct vring_virtqueue *vq = to_vvq(_vq);
> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>
>         return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
>  }
> @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
>
> -bool virtqueue_is_broken(struct virtqueue *_vq)
> +bool virtqueue_is_broken(const struct virtqueue *_vq)
>  {
> -       struct vring_virtqueue *vq = to_vvq(_vq);
> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>
>         return READ_ONCE(vq->broken);
>  }
> @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
>
> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
>  {
> -       struct vring_virtqueue *vq = to_vvq(_vq);
> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>
>         BUG_ON(!vq->we_own_ring);
>
> @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
>
> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
>  {
> -       struct vring_virtqueue *vq = to_vvq(_vq);
> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>
>         BUG_ON(!vq->we_own_ring);
>
> @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
>
> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
>  {
> -       struct vring_virtqueue *vq = to_vvq(_vq);
> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>
>         BUG_ON(!vq->we_own_ring);
>
> @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>  EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
>
>  /* Only available for split ring */
> -const struct vring *virtqueue_get_vring(struct virtqueue *vq)
> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
>  {
>         return &to_vvq(vq)->split.vring;
>  }
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 2b472514c49b..c4225653f949 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
>
>  void *virtqueue_detach_unused_buf(struct virtqueue *vq);
>
> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
>
> -bool virtqueue_is_broken(struct virtqueue *vq);
> +bool virtqueue_is_broken(const struct virtqueue *vq);
>
> -const struct vring *virtqueue_get_vring(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
>
>  int virtqueue_resize(struct virtqueue *vq, u32 num,
>                      void (*recycle)(struct virtqueue *vq, void *buf));
> @@ -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(struct virtio_device *vdev);
> +size_t 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.34.1
>

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

Re: [PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Feng Liu via Virtualization]

On 2023-03-16 p.m.11:17, Jason Wang wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Mar 16, 2023 at 2:55 AM Feng Liu <feliu@nvidia.com> wrote:
>>
>> Add const to make the read-only pointer parameters clear, similar to
>> many existing functions.
>>
>> Use `container_of_const` to implement `to_vvq`, which ensures the
>> const-ness of read-only parameters and avoids accidental modification
>> of their members.
>>
>> Signed-off-by: Feng Liu <feliu@nvidia.com>
>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 
> Thanks
> 
Hi Michael & Jason
         Could you please help to take these reviewed/acked patches forward?
Thanks so much


>>
>> ---
>> v0 -> v1
>> feedbacks from Michael S. Tsirkin
>> - use `container_of_const` to implement `to_vvq`
>> ---
>>   drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
>>   include/linux/virtio.h       | 14 +++++++-------
>>   2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> index a26fab91c59f..4c3bb0ddeb9b 100644
>> --- a/drivers/virtio/virtio_ring.c
>> +++ b/drivers/virtio/virtio_ring.c
>> @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
>>    * Helpers.
>>    */
>>
>> -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
>> +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
>>
>> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
>>                                     unsigned int total_sg)
>>   {
>>          /*
>> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>>    * unconditionally on data path.
>>    */
>>
>> -static bool vring_use_dma_api(struct virtio_device *vdev)
>> +static bool vring_use_dma_api(const struct virtio_device *vdev)
>>   {
>>          if (!virtio_has_dma_quirk(vdev))
>>                  return true;
>> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>          return false;
>>   }
>>
>> -size_t virtio_max_dma_size(struct virtio_device *vdev)
>> +size_t virtio_max_dma_size(const struct virtio_device *vdev)
>>   {
>>          size_t max_segment_size = SIZE_MAX;
>>
>> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
>>    */
>>
>>   static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
>> -                                          struct vring_desc *desc)
>> +                                          const struct vring_desc *desc)
>>   {
>>          u16 flags;
>>
>> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
>>   }
>>
>>   static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>> -                                    struct vring_desc_extra *extra)
>> +                                    const struct vring_desc_extra *extra)
>>   {
>>          u16 flags;
>>
>> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>>   }
>>
>>   static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
>> -                                  struct vring_packed_desc *desc)
>> +                                   const struct vring_packed_desc *desc)
>>   {
>>          u16 flags;
>>
>> @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
>>    * Returns the size of the vring.  This is mainly used for boasting to
>>    * userspace.  Unlike other operations, this need not be serialized.
>>    */
>> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
>>   {
>>
>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>
>>          return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
>>   }
>> @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
>>   }
>>   EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
>>
>> -bool virtqueue_is_broken(struct virtqueue *_vq)
>> +bool virtqueue_is_broken(const struct virtqueue *_vq)
>>   {
>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>
>>          return READ_ONCE(vq->broken);
>>   }
>> @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
>>   }
>>   EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
>>
>> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
>>   {
>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>
>>          BUG_ON(!vq->we_own_ring);
>>
>> @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>>   }
>>   EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
>>
>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
>>   {
>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>
>>          BUG_ON(!vq->we_own_ring);
>>
>> @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>>   }
>>   EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
>>
>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
>>   {
>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>
>>          BUG_ON(!vq->we_own_ring);
>>
>> @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>>   EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
>>
>>   /* Only available for split ring */
>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq)
>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
>>   {
>>          return &to_vvq(vq)->split.vring;
>>   }
>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
>> index 2b472514c49b..c4225653f949 100644
>> --- a/include/linux/virtio.h
>> +++ b/include/linux/virtio.h
>> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
>>
>>   void *virtqueue_detach_unused_buf(struct virtqueue *vq);
>>
>> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
>>
>> -bool virtqueue_is_broken(struct virtqueue *vq);
>> +bool virtqueue_is_broken(const struct virtqueue *vq);
>>
>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq);
>> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
>> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
>>
>>   int virtqueue_resize(struct virtqueue *vq, u32 num,
>>                       void (*recycle)(struct virtqueue *vq, void *buf));
>> @@ -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(struct virtio_device *vdev);
>> +size_t 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.34.1
>>
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Michael S. Tsirkin]

On Thu, Mar 30, 2023 at 02:22:44PM -0400, Feng Liu wrote:
> 
> 
> On 2023-03-16 p.m.11:17, Jason Wang wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Thu, Mar 16, 2023 at 2:55 AM Feng Liu <feliu@nvidia.com> wrote:
> > > 
> > > Add const to make the read-only pointer parameters clear, similar to
> > > many existing functions.
> > > 
> > > Use `container_of_const` to implement `to_vvq`, which ensures the
> > > const-ness of read-only parameters and avoids accidental modification
> > > of their members.
> > > 
> > > Signed-off-by: Feng Liu <feliu@nvidia.com>
> > > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> > 
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > 
> > Thanks
> > 
> Hi Michael & Jason
>         Could you please help to take these reviewed/acked patches forward?
> Thanks so much


this is not going in this linux, only the next.

> 
> > > 
> > > ---
> > > v0 -> v1
> > > feedbacks from Michael S. Tsirkin
> > > - use `container_of_const` to implement `to_vvq`
> > > ---
> > >   drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
> > >   include/linux/virtio.h       | 14 +++++++-------
> > >   2 files changed, 25 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index a26fab91c59f..4c3bb0ddeb9b 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
> > >    * Helpers.
> > >    */
> > > 
> > > -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
> > > +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
> > > 
> > > -static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> > > +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
> > >                                     unsigned int total_sg)
> > >   {
> > >          /*
> > > @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> > >    * unconditionally on data path.
> > >    */
> > > 
> > > -static bool vring_use_dma_api(struct virtio_device *vdev)
> > > +static bool vring_use_dma_api(const struct virtio_device *vdev)
> > >   {
> > >          if (!virtio_has_dma_quirk(vdev))
> > >                  return true;
> > > @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > >          return false;
> > >   }
> > > 
> > > -size_t virtio_max_dma_size(struct virtio_device *vdev)
> > > +size_t virtio_max_dma_size(const struct virtio_device *vdev)
> > >   {
> > >          size_t max_segment_size = SIZE_MAX;
> > > 
> > > @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
> > >    */
> > > 
> > >   static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
> > > -                                          struct vring_desc *desc)
> > > +                                          const struct vring_desc *desc)
> > >   {
> > >          u16 flags;
> > > 
> > > @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
> > >   }
> > > 
> > >   static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
> > > -                                    struct vring_desc_extra *extra)
> > > +                                    const struct vring_desc_extra *extra)
> > >   {
> > >          u16 flags;
> > > 
> > > @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
> > >   }
> > > 
> > >   static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
> > > -                                  struct vring_packed_desc *desc)
> > > +                                   const struct vring_packed_desc *desc)
> > >   {
> > >          u16 flags;
> > > 
> > > @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
> > >    * Returns the size of the vring.  This is mainly used for boasting to
> > >    * userspace.  Unlike other operations, this need not be serialized.
> > >    */
> > > -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
> > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
> > >   {
> > > 
> > > -       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       const struct vring_virtqueue *vq = to_vvq(_vq);
> > > 
> > >          return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
> > >   }
> > > @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
> > >   }
> > >   EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
> > > 
> > > -bool virtqueue_is_broken(struct virtqueue *_vq)
> > > +bool virtqueue_is_broken(const struct virtqueue *_vq)
> > >   {
> > > -       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       const struct vring_virtqueue *vq = to_vvq(_vq);
> > > 
> > >          return READ_ONCE(vq->broken);
> > >   }
> > > @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
> > >   }
> > >   EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
> > > 
> > > -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
> > > +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
> > >   {
> > > -       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       const struct vring_virtqueue *vq = to_vvq(_vq);
> > > 
> > >          BUG_ON(!vq->we_own_ring);
> > > 
> > > @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
> > >   }
> > >   EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
> > > 
> > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
> > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
> > >   {
> > > -       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       const struct vring_virtqueue *vq = to_vvq(_vq);
> > > 
> > >          BUG_ON(!vq->we_own_ring);
> > > 
> > > @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
> > >   }
> > >   EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
> > > 
> > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
> > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
> > >   {
> > > -       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +       const struct vring_virtqueue *vq = to_vvq(_vq);
> > > 
> > >          BUG_ON(!vq->we_own_ring);
> > > 
> > > @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
> > >   EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
> > > 
> > >   /* Only available for split ring */
> > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq)
> > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
> > >   {
> > >          return &to_vvq(vq)->split.vring;
> > >   }
> > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > > index 2b472514c49b..c4225653f949 100644
> > > --- a/include/linux/virtio.h
> > > +++ b/include/linux/virtio.h
> > > @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
> > > 
> > >   void *virtqueue_detach_unused_buf(struct virtqueue *vq);
> > > 
> > > -unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
> > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
> > > 
> > > -bool virtqueue_is_broken(struct virtqueue *vq);
> > > +bool virtqueue_is_broken(const struct virtqueue *vq);
> > > 
> > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq);
> > > -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
> > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
> > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
> > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
> > > +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
> > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
> > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
> > > 
> > >   int virtqueue_resize(struct virtqueue *vq, u32 num,
> > >                       void (*recycle)(struct virtqueue *vq, void *buf));
> > > @@ -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(struct virtio_device *vdev);
> > > +size_t 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.34.1
> > > 
> > 

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

Re: [PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[Feng Liu via Virtualization]

On 2023-03-30 p.m.4:27, Michael S. Tsirkin wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Mar 30, 2023 at 02:22:44PM -0400, Feng Liu wrote:
>>
>>
>> On 2023-03-16 p.m.11:17, Jason Wang wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Thu, Mar 16, 2023 at 2:55 AM Feng Liu <feliu@nvidia.com> wrote:
>>>>
>>>> Add const to make the read-only pointer parameters clear, similar to
>>>> many existing functions.
>>>>
>>>> Use `container_of_const` to implement `to_vvq`, which ensures the
>>>> const-ness of read-only parameters and avoids accidental modification
>>>> of their members.
>>>>
>>>> Signed-off-by: Feng Liu <feliu@nvidia.com>
>>>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>>>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>
>>> Thanks
>>>
>> Hi Michael & Jason
>>          Could you please help to take these reviewed/acked patches forward?
>> Thanks so much
> 
> 
> this is not going in this linux, only the next.
> 
Hi, Michael
	Thanks for the update.

>>
>>>>
>>>> ---
>>>> v0 -> v1
>>>> feedbacks from Michael S. Tsirkin
>>>> - use `container_of_const` to implement `to_vvq`
>>>> ---
>>>>    drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
>>>>    include/linux/virtio.h       | 14 +++++++-------
>>>>    2 files changed, 25 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>> index a26fab91c59f..4c3bb0ddeb9b 100644
>>>> --- a/drivers/virtio/virtio_ring.c
>>>> +++ b/drivers/virtio/virtio_ring.c
>>>> @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
>>>>     * Helpers.
>>>>     */
>>>>
>>>> -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
>>>> +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
>>>>
>>>> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>>>> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
>>>>                                      unsigned int total_sg)
>>>>    {
>>>>           /*
>>>> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>>>>     * unconditionally on data path.
>>>>     */
>>>>
>>>> -static bool vring_use_dma_api(struct virtio_device *vdev)
>>>> +static bool vring_use_dma_api(const struct virtio_device *vdev)
>>>>    {
>>>>           if (!virtio_has_dma_quirk(vdev))
>>>>                   return true;
>>>> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>>>           return false;
>>>>    }
>>>>
>>>> -size_t virtio_max_dma_size(struct virtio_device *vdev)
>>>> +size_t virtio_max_dma_size(const struct virtio_device *vdev)
>>>>    {
>>>>           size_t max_segment_size = SIZE_MAX;
>>>>
>>>> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
>>>>     */
>>>>
>>>>    static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
>>>> -                                          struct vring_desc *desc)
>>>> +                                          const struct vring_desc *desc)
>>>>    {
>>>>           u16 flags;
>>>>
>>>> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
>>>>    }
>>>>
>>>>    static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>>>> -                                    struct vring_desc_extra *extra)
>>>> +                                    const struct vring_desc_extra *extra)
>>>>    {
>>>>           u16 flags;
>>>>
>>>> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>>>>    }
>>>>
>>>>    static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
>>>> -                                  struct vring_packed_desc *desc)
>>>> +                                   const struct vring_packed_desc *desc)
>>>>    {
>>>>           u16 flags;
>>>>
>>>> @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
>>>>     * Returns the size of the vring.  This is mainly used for boasting to
>>>>     * userspace.  Unlike other operations, this need not be serialized.
>>>>     */
>>>> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
>>>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
>>>>    {
>>>>
>>>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>>>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>>>
>>>>           return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
>>>>    }
>>>> @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
>>>>
>>>> -bool virtqueue_is_broken(struct virtqueue *_vq)
>>>> +bool virtqueue_is_broken(const struct virtqueue *_vq)
>>>>    {
>>>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>>>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>>>
>>>>           return READ_ONCE(vq->broken);
>>>>    }
>>>> @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
>>>>
>>>> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>>>> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
>>>>    {
>>>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>>>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>>>
>>>>           BUG_ON(!vq->we_own_ring);
>>>>
>>>> @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
>>>>
>>>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>>>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
>>>>    {
>>>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>>>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>>>
>>>>           BUG_ON(!vq->we_own_ring);
>>>>
>>>> @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
>>>>
>>>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>>>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
>>>>    {
>>>> -       struct vring_virtqueue *vq = to_vvq(_vq);
>>>> +       const struct vring_virtqueue *vq = to_vvq(_vq);
>>>>
>>>>           BUG_ON(!vq->we_own_ring);
>>>>
>>>> @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>>>>    EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
>>>>
>>>>    /* Only available for split ring */
>>>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq)
>>>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
>>>>    {
>>>>           return &to_vvq(vq)->split.vring;
>>>>    }
>>>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
>>>> index 2b472514c49b..c4225653f949 100644
>>>> --- a/include/linux/virtio.h
>>>> +++ b/include/linux/virtio.h
>>>> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
>>>>
>>>>    void *virtqueue_detach_unused_buf(struct virtqueue *vq);
>>>>
>>>> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
>>>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
>>>>
>>>> -bool virtqueue_is_broken(struct virtqueue *vq);
>>>> +bool virtqueue_is_broken(const struct virtqueue *vq);
>>>>
>>>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq);
>>>> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
>>>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
>>>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
>>>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
>>>> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
>>>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
>>>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
>>>>
>>>>    int virtqueue_resize(struct virtqueue *vq, u32 num,
>>>>                        void (*recycle)(struct virtqueue *vq, void *buf));
>>>> @@ -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(struct virtio_device *vdev);
>>>> +size_t 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.34.1
>>>>
>>>
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params

[David Edmondson]

Feng Liu <feliu@nvidia.com> writes:

> Add const to make the read-only pointer parameters clear, similar to
> many existing functions.
>
> Use `container_of_const` to implement `to_vvq`, which ensures the
> const-ness of read-only parameters and avoids accidental modification
> of their members.
>
> Signed-off-by: Feng Liu <feliu@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
> v0 -> v1
> feedbacks from Michael S. Tsirkin
> - use `container_of_const` to implement `to_vvq`
> ---
>  drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------
>  include/linux/virtio.h       | 14 +++++++-------
>  2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index a26fab91c59f..4c3bb0ddeb9b 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq);
>   * Helpers.
>   */
>  
> -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
> +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq)
>  
> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq,
>  				   unsigned int total_sg)
>  {
>  	/*
> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
>   * unconditionally on data path.
>   */
>  
> -static bool vring_use_dma_api(struct virtio_device *vdev)
> +static bool vring_use_dma_api(const struct virtio_device *vdev)
>  {
>  	if (!virtio_has_dma_quirk(vdev))
>  		return true;
> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>  	return false;
>  }
>  
> -size_t virtio_max_dma_size(struct virtio_device *vdev)
> +size_t virtio_max_dma_size(const struct virtio_device *vdev)
>  {
>  	size_t max_segment_size = SIZE_MAX;
>  
> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
>   */
>  
>  static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
> -					   struct vring_desc *desc)
> +					   const struct vring_desc *desc)
>  {
>  	u16 flags;
>  
> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx)
>  }
>  
>  static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
> -				     struct vring_desc_extra *extra)
> +				     const struct vring_desc_extra *extra)
>  {
>  	u16 flags;
>  
> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
>  }
>  
>  static void vring_unmap_desc_packed(const struct vring_virtqueue *vq,
> -				   struct vring_packed_desc *desc)
> +				    const struct vring_packed_desc *desc)
>  {
>  	u16 flags;
>  
> @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features);
>   * Returns the size of the vring.  This is mainly used for boasting to
>   * userspace.  Unlike other operations, this need not be serialized.
>   */
> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq)
> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq)
>  {
>  
> -	struct vring_virtqueue *vq = to_vvq(_vq);
> +	const struct vring_virtqueue *vq = to_vvq(_vq);
>  
>  	return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
>  }
> @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(__virtqueue_unbreak);
>  
> -bool virtqueue_is_broken(struct virtqueue *_vq)
> +bool virtqueue_is_broken(const struct virtqueue *_vq)
>  {
> -	struct vring_virtqueue *vq = to_vvq(_vq);
> +	const struct vring_virtqueue *vq = to_vvq(_vq);
>  
>  	return READ_ONCE(vq->broken);
>  }
> @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(__virtio_unbreak_device);
>  
> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq)
>  {
> -	struct vring_virtqueue *vq = to_vvq(_vq);
> +	const struct vring_virtqueue *vq = to_vvq(_vq);
>  
>  	BUG_ON(!vq->we_own_ring);
>  
> @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr);
>  
> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq)
>  {
> -	struct vring_virtqueue *vq = to_vvq(_vq);
> +	const struct vring_virtqueue *vq = to_vvq(_vq);
>  
>  	BUG_ON(!vq->we_own_ring);
>  
> @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr);
>  
> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq)
>  {
> -	struct vring_virtqueue *vq = to_vvq(_vq);
> +	const struct vring_virtqueue *vq = to_vvq(_vq);
>  
>  	BUG_ON(!vq->we_own_ring);
>  
> @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq)
>  EXPORT_SYMBOL_GPL(virtqueue_get_used_addr);
>  
>  /* Only available for split ring */
> -const struct vring *virtqueue_get_vring(struct virtqueue *vq)
> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq)
>  {
>  	return &to_vvq(vq)->split.vring;
>  }
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 2b472514c49b..c4225653f949 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
>  
>  void *virtqueue_detach_unused_buf(struct virtqueue *vq);
>  
> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq);
>  
> -bool virtqueue_is_broken(struct virtqueue *vq);
> +bool virtqueue_is_broken(const struct virtqueue *vq);
>  
> -const struct vring *virtqueue_get_vring(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq);
> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq);
>  
>  int virtqueue_resize(struct virtqueue *vq, u32 num,
>  		     void (*recycle)(struct virtqueue *vq, void *buf));
> @@ -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(struct virtio_device *vdev);
> +size_t 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.34.1
-- 
I do believe it's Madame Joy.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization