linux-kernel - Re: [PATCH RFC v2 3/5] virtio-pci: extend virtio_pci_cap with page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7f5a1552-f143-40c8-b796-20568d373d14@collabora.com>
Date: Thu, 20 Feb 2025 06:14:02 +0300
From: Dmitry Osipenko <dmitry.osipenko@...labora.com>
To: Sergio Lopez <slp@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
 Daniel Verkamp <dverkamp@...omium.org>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, David Airlie <airlied@...hat.com>,
 Gerd Hoffmann <kraxel@...hat.com>,
 Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu
 <olvaffe@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 Simona Vetter <simona@...ll.ch>, Rob Clark <robdclark@...il.com>,
 fnkl.kernel@...il.com
Cc: virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH RFC v2 3/5] virtio-pci: extend virtio_pci_cap with
 page_shift

On 2/14/25 18:16, Sergio Lopez wrote:
> If VIRTIO_F_SHM_PAGE_SIZE has been negotiated and cfg_type is
> VIRTIO_PCI_CAP_SHARED_MEMORY_CFG, the driver must read the page_shift
> field, derive the supported page size from it, and honor it when
> requesting the map of memory into the shared memory region to the
> device.
> 
> Extend virtio_pci_cap to hold that field, and use it to feed
> virtio_shm_region with the corresponding page size.
> 
> Signed-off-by: Sergio Lopez <slp@...hat.com>
> ---
>  drivers/virtio/virtio_pci_modern.c | 22 +++++++++++++++++-----
>  include/uapi/linux/virtio_pci.h    |  3 ++-
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index ab0e1d9148057c431676bfacfe5f68cc97eebb12..562a8e1c2bfe6876cffabe26f02cd61ad7fea2cd 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -770,14 +770,15 @@ static void del_vq(struct virtio_pci_vq_info *info)
>  	vring_del_virtqueue(vq);
>  }
>  
> -static int virtio_pci_find_shm_cap(struct pci_dev *dev, u8 required_id,
> -				   u8 *bar, u64 *offset, u64 *len)
> +static int virtio_pci_find_shm_cap(struct virtio_device *vdev, struct pci_dev *dev,
> +				   u8 required_id, u8 *bar, u64 *offset, u64 *len,
> +				   u32 *page_size)
>  {
>  	int pos;
>  
>  	for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); pos > 0;
>  	     pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> -		u8 type, cap_len, id, res_bar;
> +		u8 type, cap_len, id, res_bar, page_shift = 0;
>  		u32 tmp32;
>  		u64 res_offset, res_length;
>  
> @@ -808,6 +809,15 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, u8 required_id,
>  		 * Looks good.
>  		 */
>  
> +		/* Read the page shift if supported. 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)) {
> +			pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
> +								 page_shift), &page_shift);
> +		}
> +
>  		/* Read the lower 32bit of length and offset */
>  		pci_read_config_dword(dev, pos + offsetof(struct virtio_pci_cap,
>  							  offset), &tmp32);
> @@ -829,6 +839,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, u8 required_id,
>  		*bar = res_bar;
>  		*offset = res_offset;
>  		*len = res_length;
> +		*page_size = 1 << (page_shift + 12);
>  
>  		return pos;
>  	}
> @@ -841,11 +852,12 @@ static bool vp_get_shm_region(struct virtio_device *vdev,
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	struct pci_dev *pci_dev = vp_dev->pci_dev;
>  	u8 bar;
> +	u32 page_size;
>  	u64 offset, len;
>  	phys_addr_t phys_addr;
>  	size_t bar_len;
>  
> -	if (!virtio_pci_find_shm_cap(pci_dev, id, &bar, &offset, &len))
> +	if (!virtio_pci_find_shm_cap(vdev, pci_dev, id, &bar, &offset, &len, &page_size))
>  		return false;
>  
>  	phys_addr = pci_resource_start(pci_dev, bar);
> @@ -865,7 +877,7 @@ static bool vp_get_shm_region(struct virtio_device *vdev,
>  
>  	region->len = len;
>  	region->addr = (u64) phys_addr + offset;
> -	region->page_size = 4096;
> +	region->page_size = page_size;
>  
>  	return true;
>  }
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index 8549d4571257142ac6c9dad5c01369923791a85a..1a76df52b4eccf548df78a8ee7d3a04591f55522 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -127,7 +127,8 @@ struct virtio_pci_cap {
>  	__u8 cfg_type;		/* Identifies the structure. */
>  	__u8 bar;		/* Where to find it. */
>  	__u8 id;		/* Multiple capabilities of the same type */
> -	__u8 padding[2];	/* Pad to full dword. */
> +	__u8 page_shift;	/* Page shift for VIRTIO_PCI_CAP_SHARED_MEMORY_CFG. */
> +	__u8 padding[1];	/* Pad to full dword. */
>  	__le32 offset;		/* Offset within bar. */
>  	__le32 length;		/* Length of the structure, in bytes. */
>  };
> 

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

-- 
Best regards,
Dmitry