[PATCH RFC v2 4/5] virtio-mmio: read shm region page size

Sergio Lopez posted 5 patches 10 months, 1 week ago
There is a newer version of this series
[PATCH RFC v2 4/5] virtio-mmio: read shm region page size
Posted by Sergio Lopez 10 months, 1 week ago
Use the newly introduced SHM_PAGE_SHIFT register to read the page shift
for the shared memory region, derive the page size from it and store the
resulting value into virtio_shm_region.

Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 drivers/virtio/virtio_mmio.c     | 11 ++++++++++-
 include/uapi/linux/virtio_mmio.h |  3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 1f594b626d7a7734e8ec58766737a118c26bad94..0f892770739ea84b3e7be5615332773049b10ab1 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -537,6 +537,7 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
 			      struct virtio_shm_region *region, u8 id)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+	u8 page_shift = 0;
 	u64 len, addr;
 
 	/* Select the region we're interested in */
@@ -560,7 +561,15 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
 
 	region->addr = addr;
 
-	region->page_size = 4096;
+	/* If supported by the device transport, read the region page size.
+	 * The page_shift variable is initialized to zero above, so if this
+	 * feature isn't supported it will result in a page_size of 4096, a
+	 * default safe value.
+	 */
+	if (__virtio_test_bit(vdev, VIRTIO_F_SHM_PAGE_SIZE))
+		page_shift = (u8) readl(vm_dev->base + VIRTIO_MMIO_SHM_PAGE_SHIFT);
+
+	region->page_size = 1 << (page_shift + 12);
 
 	return true;
 }
diff --git a/include/uapi/linux/virtio_mmio.h b/include/uapi/linux/virtio_mmio.h
index 0650f91bea6c70f935764070d825d181a2379afb..43348be30eff90ee228b6490b9d3c35ba4c50aa5 100644
--- a/include/uapi/linux/virtio_mmio.h
+++ b/include/uapi/linux/virtio_mmio.h
@@ -133,6 +133,9 @@
 #define VIRTIO_MMIO_SHM_BASE_LOW        0x0b8
 #define VIRTIO_MMIO_SHM_BASE_HIGH       0x0bc
 
+/* Shared memory region page shift */
+#define VIRTIO_MMIO_SHM_PAGE_SHIFT      0x0c4
+
 /* Configuration atomicity value */
 #define VIRTIO_MMIO_CONFIG_GENERATION	0x0fc
 

-- 
2.48.1
Re: [PATCH RFC v2 4/5] virtio-mmio: read shm region page size
Posted by Dmitry Osipenko 10 months ago
On 2/14/25 18:16, Sergio Lopez wrote:
> Use the newly introduced SHM_PAGE_SHIFT register to read the page shift
> for the shared memory region, derive the page size from it and store the
> resulting value into virtio_shm_region.
> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>
> ---
>  drivers/virtio/virtio_mmio.c     | 11 ++++++++++-
>  include/uapi/linux/virtio_mmio.h |  3 +++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 1f594b626d7a7734e8ec58766737a118c26bad94..0f892770739ea84b3e7be5615332773049b10ab1 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -537,6 +537,7 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>  			      struct virtio_shm_region *region, u8 id)
>  {
>  	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> +	u8 page_shift = 0;
>  	u64 len, addr;
>  
>  	/* Select the region we're interested in */
> @@ -560,7 +561,15 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>  
>  	region->addr = addr;
>  
> -	region->page_size = 4096;
> +	/* If supported by the device transport, read the region page size.
> +	 * The page_shift variable is initialized to zero above, so if this
> +	 * feature isn't supported it will result in a page_size of 4096, a
> +	 * default safe value.
> +	 */
> +	if (__virtio_test_bit(vdev, VIRTIO_F_SHM_PAGE_SIZE))
> +		page_shift = (u8) readl(vm_dev->base + VIRTIO_MMIO_SHM_PAGE_SHIFT);
> +
> +	region->page_size = 1 << (page_shift + 12);
>  
>  	return true;
>  }
> diff --git a/include/uapi/linux/virtio_mmio.h b/include/uapi/linux/virtio_mmio.h
> index 0650f91bea6c70f935764070d825d181a2379afb..43348be30eff90ee228b6490b9d3c35ba4c50aa5 100644
> --- a/include/uapi/linux/virtio_mmio.h
> +++ b/include/uapi/linux/virtio_mmio.h
> @@ -133,6 +133,9 @@
>  #define VIRTIO_MMIO_SHM_BASE_LOW        0x0b8
>  #define VIRTIO_MMIO_SHM_BASE_HIGH       0x0bc
>  
> +/* Shared memory region page shift */
> +#define VIRTIO_MMIO_SHM_PAGE_SHIFT      0x0c4

What's the logic behind choosing 0x0c4, why not 0x0c0?

-- 
Best regards,
Dmitry
Re: [PATCH RFC v2 4/5] virtio-mmio: read shm region page size
Posted by Sergio Lopez Pascual 10 months ago
Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> On 2/14/25 18:16, Sergio Lopez wrote:
>> Use the newly introduced SHM_PAGE_SHIFT register to read the page shift
>> for the shared memory region, derive the page size from it and store the
>> resulting value into virtio_shm_region.
>>
>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>> ---
>>  drivers/virtio/virtio_mmio.c     | 11 ++++++++++-
>>  include/uapi/linux/virtio_mmio.h |  3 +++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index 1f594b626d7a7734e8ec58766737a118c26bad94..0f892770739ea84b3e7be5615332773049b10ab1 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -537,6 +537,7 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>>  			      struct virtio_shm_region *region, u8 id)
>>  {
>>  	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
>> +	u8 page_shift = 0;
>>  	u64 len, addr;
>>
>>  	/* Select the region we're interested in */
>> @@ -560,7 +561,15 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>>
>>  	region->addr = addr;
>>
>> -	region->page_size = 4096;
>> +	/* If supported by the device transport, read the region page size.
>> +	 * The page_shift variable is initialized to zero above, so if this
>> +	 * feature isn't supported it will result in a page_size of 4096, a
>> +	 * default safe value.
>> +	 */
>> +	if (__virtio_test_bit(vdev, VIRTIO_F_SHM_PAGE_SIZE))
>> +		page_shift = (u8) readl(vm_dev->base + VIRTIO_MMIO_SHM_PAGE_SHIFT);
>> +
>> +	region->page_size = 1 << (page_shift + 12);
>>
>>  	return true;
>>  }
>> diff --git a/include/uapi/linux/virtio_mmio.h b/include/uapi/linux/virtio_mmio.h
>> index 0650f91bea6c70f935764070d825d181a2379afb..43348be30eff90ee228b6490b9d3c35ba4c50aa5 100644
>> --- a/include/uapi/linux/virtio_mmio.h
>> +++ b/include/uapi/linux/virtio_mmio.h
>> @@ -133,6 +133,9 @@
>>  #define VIRTIO_MMIO_SHM_BASE_LOW        0x0b8
>>  #define VIRTIO_MMIO_SHM_BASE_HIGH       0x0bc
>>
>> +/* Shared memory region page shift */
>> +#define VIRTIO_MMIO_SHM_PAGE_SHIFT      0x0c4
>
> What's the logic behind choosing 0x0c4, why not 0x0c0?

It seems like Linux doesn't support it yet, but in the specs 0x0c0 is
already reserved for QueueReset.

Sergio.
Re: [PATCH RFC v2 4/5] virtio-mmio: read shm region page size
Posted by Dmitry Osipenko 10 months ago
On 2/20/25 13:24, Sergio Lopez Pascual wrote:
> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
> 
>> On 2/14/25 18:16, Sergio Lopez wrote:
>>> Use the newly introduced SHM_PAGE_SHIFT register to read the page shift
>>> for the shared memory region, derive the page size from it and store the
>>> resulting value into virtio_shm_region.
>>>
>>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>>> ---
>>>  drivers/virtio/virtio_mmio.c     | 11 ++++++++++-
>>>  include/uapi/linux/virtio_mmio.h |  3 +++
>>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>>> index 1f594b626d7a7734e8ec58766737a118c26bad94..0f892770739ea84b3e7be5615332773049b10ab1 100644
>>> --- a/drivers/virtio/virtio_mmio.c
>>> +++ b/drivers/virtio/virtio_mmio.c
>>> @@ -537,6 +537,7 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>>>  			      struct virtio_shm_region *region, u8 id)
>>>  {
>>>  	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
>>> +	u8 page_shift = 0;
>>>  	u64 len, addr;
>>>
>>>  	/* Select the region we're interested in */
>>> @@ -560,7 +561,15 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
>>>
>>>  	region->addr = addr;
>>>
>>> -	region->page_size = 4096;
>>> +	/* If supported by the device transport, read the region page size.
>>> +	 * The page_shift variable is initialized to zero above, so if this
>>> +	 * feature isn't supported it will result in a page_size of 4096, a
>>> +	 * default safe value.
>>> +	 */
>>> +	if (__virtio_test_bit(vdev, VIRTIO_F_SHM_PAGE_SIZE))
>>> +		page_shift = (u8) readl(vm_dev->base + VIRTIO_MMIO_SHM_PAGE_SHIFT);
>>> +
>>> +	region->page_size = 1 << (page_shift + 12);
>>>
>>>  	return true;
>>>  }
>>> diff --git a/include/uapi/linux/virtio_mmio.h b/include/uapi/linux/virtio_mmio.h
>>> index 0650f91bea6c70f935764070d825d181a2379afb..43348be30eff90ee228b6490b9d3c35ba4c50aa5 100644
>>> --- a/include/uapi/linux/virtio_mmio.h
>>> +++ b/include/uapi/linux/virtio_mmio.h
>>> @@ -133,6 +133,9 @@
>>>  #define VIRTIO_MMIO_SHM_BASE_LOW        0x0b8
>>>  #define VIRTIO_MMIO_SHM_BASE_HIGH       0x0bc
>>>
>>> +/* Shared memory region page shift */
>>> +#define VIRTIO_MMIO_SHM_PAGE_SHIFT      0x0c4
>>
>> What's the logic behind choosing 0x0c4, why not 0x0c0?
> 
> It seems like Linux doesn't support it yet, but in the specs 0x0c0 is
> already reserved for QueueReset.

See QueueReset in the spec now, thanks.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry