[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f5328e6f-170c-4984-9a5d-e81761fed8b1@nvidia.com>
Date: Sun, 18 Feb 2024 14:07:17 +0200
From: Yishai Hadas <yishaih@...dia.com>
To: <ankita@...dia.com>, <jgg@...dia.com>, <alex.williamson@...hat.com>,
<shameerali.kolothum.thodi@...wei.com>, <kevin.tian@...el.com>,
<mst@...hat.com>, <eric.auger@...hat.com>, <jgg@...pe.ca>,
<oleksandr@...alenko.name>, <clg@...hat.com>,
<satyanarayana.k.v.p@...el.com>, <brett.creeley@....com>, <horms@...nel.org>,
<shannon.nelson@....com>
CC: <aniketa@...dia.com>, <cjia@...dia.com>, <kwankhede@...dia.com>,
<targupta@...dia.com>, <vsethi@...dia.com>, <acurrid@...dia.com>,
<apopple@...dia.com>, <jhubbard@...dia.com>, <danw@...dia.com>,
<anuaggarwal@...dia.com>, <mochs@...dia.com>, <kvm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH v18 2/3] vfio/pci: rename and export range_intersect_range
On 16/02/2024 5:01, ankita@...dia.com wrote:
> From: Ankit Agrawal <ankita@...dia.com>
>
> range_intersect_range determines an overlap between two ranges. If an
> overlap, the helper function returns the overlapping offset and size.
>
> The VFIO PCI variant driver emulates the PCI config space BAR offset
> registers. These offset may be accessed for read/write with a variety
> of lengths including sub-word sizes from sub-word offsets. The driver
> makes use of this helper function to read/write the targeted part of
> the emulated register.
>
> Make this a vfio_pci_core function, rename and export as GPL. Also
> update references in virtio driver.
>
> Reviewed-by: Kevin Tian <kevin.tian@...el.com>
> Signed-off-by: Ankit Agrawal <ankita@...dia.com>
> ---
> drivers/vfio/pci/vfio_pci_config.c | 42 +++++++++++++++++
> drivers/vfio/pci/virtio/main.c | 72 +++++++++++-------------------
> include/linux/vfio_pci_core.h | 5 +++
> 3 files changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 672a1804af6a..e2e6173a3375 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1966,3 +1966,45 @@ ssize_t vfio_pci_config_rw(struct vfio_pci_core_device *vdev, char __user *buf,
>
> return done;
> }
> +
> +/**
> + * vfio_pci_core_range_intersect_range() - Determine overlap between a buffer
> + * and register offset ranges.
> + * @buf_start: start offset of the buffer
> + * @buf_cnt: number of buffer bytes.
You could drop the '.' at the end to be consistent with the other.
> + * @reg_start: start register offset
> + * @reg_cnt: number of register bytes
> + * @buf_offset: start offset of overlap in the buffer
> + * @intersect_count: number of overlapping bytes
> + * @register_offset: start offset of overlap in register
> + *
> + * Returns: true if there is overlap, false if not.
> + * The overlap start and size is returned through function args.
> + */
> +bool vfio_pci_core_range_intersect_range(loff_t buf_start, size_t buf_cnt,
> + loff_t reg_start, size_t reg_cnt,
> + loff_t *buf_offset,
> + size_t *intersect_count,
> + size_t *register_offset)
> +{
> + if (buf_start <= reg_start &&
> + buf_start + buf_cnt > reg_start) {
> + *buf_offset = reg_start - buf_start;
> + *intersect_count = min_t(size_t, reg_cnt,
> + buf_start + buf_cnt - reg_start);
> + *register_offset = 0;
> + return true;
> + }
> +
> + if (buf_start > reg_start &&
> + buf_start < reg_start + reg_cnt) {
> + *buf_offset = 0;
> + *intersect_count = min_t(size_t, buf_cnt,
> + reg_start + reg_cnt - buf_start);
> + *register_offset = buf_start - reg_start;
> + return true;
> + }
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(vfio_pci_core_range_intersect_range);
> diff --git a/drivers/vfio/pci/virtio/main.c b/drivers/vfio/pci/virtio/main.c
> index d5af683837d3..b5d3a8c5bbc9 100644
> --- a/drivers/vfio/pci/virtio/main.c
> +++ b/drivers/vfio/pci/virtio/main.c
> @@ -132,33 +132,6 @@ virtiovf_pci_bar0_rw(struct virtiovf_pci_core_device *virtvdev,
> return ret ? ret : count;
> }
>
> -static bool range_intersect_range(loff_t range1_start, size_t count1,
> - loff_t range2_start, size_t count2,
> - loff_t *start_offset,
> - size_t *intersect_count,
> - size_t *register_offset)
> -{
> - if (range1_start <= range2_start &&
> - range1_start + count1 > range2_start) {
> - *start_offset = range2_start - range1_start;
> - *intersect_count = min_t(size_t, count2,
> - range1_start + count1 - range2_start);
> - *register_offset = 0;
> - return true;
> - }
> -
> - if (range1_start > range2_start &&
> - range1_start < range2_start + count2) {
> - *start_offset = 0;
> - *intersect_count = min_t(size_t, count1,
> - range2_start + count2 - range1_start);
> - *register_offset = range1_start - range2_start;
> - return true;
> - }
> -
> - return false;
> -}
> -
> static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> char __user *buf, size_t count,
> loff_t *ppos)
> @@ -178,16 +151,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> if (ret < 0)
> return ret;
>
> - if (range_intersect_range(pos, count, PCI_DEVICE_ID, sizeof(val16),
> - ©_offset, ©_count, ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_DEVICE_ID,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> val16 = cpu_to_le16(VIRTIO_TRANS_ID_NET);
> if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset, copy_count))
> return -EFAULT;
> }
>
> if ((le16_to_cpu(virtvdev->pci_cmd) & PCI_COMMAND_IO) &&
> - range_intersect_range(pos, count, PCI_COMMAND, sizeof(val16),
> - ©_offset, ©_count, ®ister_offset)) {
> + vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> if (copy_from_user((void *)&val16 + register_offset, buf + copy_offset,
> copy_count))
> return -EFAULT;
> @@ -197,16 +172,18 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> return -EFAULT;
> }
>
> - if (range_intersect_range(pos, count, PCI_REVISION_ID, sizeof(val8),
> - ©_offset, ©_count, ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_REVISION_ID,
> + sizeof(val8), ©_offset,
> + ©_count, ®ister_offset)) {
> /* Transional needs to have revision 0 */
> val8 = 0;
> if (copy_to_user(buf + copy_offset, &val8, copy_count))
> return -EFAULT;
> }
>
> - if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0, sizeof(val32),
> - ©_offset, ©_count, ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
> + sizeof(val32), ©_offset,
> + ©_count, ®ister_offset)) {
> u32 bar_mask = ~(virtvdev->bar0_virtual_buf_size - 1);
> u32 pci_base_addr_0 = le32_to_cpu(virtvdev->pci_base_addr_0);
>
> @@ -215,8 +192,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> return -EFAULT;
> }
>
> - if (range_intersect_range(pos, count, PCI_SUBSYSTEM_ID, sizeof(val16),
> - ©_offset, ©_count, ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_ID,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> /*
> * Transitional devices use the PCI subsystem device id as
> * virtio device id, same as legacy driver always did.
> @@ -227,8 +205,9 @@ static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> return -EFAULT;
> }
>
> - if (range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID, sizeof(val16),
> - ©_offset, ©_count, ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_SUBSYSTEM_VENDOR_ID,
> + sizeof(val16), ©_offset,
> + ©_count, ®ister_offset)) {
> val16 = cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET);
> if (copy_to_user(buf + copy_offset, (void *)&val16 + register_offset,
> copy_count))
> @@ -270,19 +249,20 @@ static ssize_t virtiovf_pci_write_config(struct vfio_device *core_vdev,
> loff_t copy_offset;
> size_t copy_count;
>
> - if (range_intersect_range(pos, count, PCI_COMMAND, sizeof(virtvdev->pci_cmd),
> - ©_offset, ©_count,
> - ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_COMMAND,
> + sizeof(virtvdev->pci_cmd),
> + ©_offset, ©_count,
> + ®ister_offset)) {
> if (copy_from_user((void *)&virtvdev->pci_cmd + register_offset,
> buf + copy_offset,
> copy_count))
> return -EFAULT;
> }
>
> - if (range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
> - sizeof(virtvdev->pci_base_addr_0),
> - ©_offset, ©_count,
> - ®ister_offset)) {
> + if (vfio_pci_core_range_intersect_range(pos, count, PCI_BASE_ADDRESS_0,
> + sizeof(virtvdev->pci_base_addr_0),
> + ©_offset, ©_count,
> + ®ister_offset)) {
> if (copy_from_user((void *)&virtvdev->pci_base_addr_0 + register_offset,
> buf + copy_offset,
> copy_count))
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index cf9480a31f3e..a2c8b8bba711 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -134,6 +134,11 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
> void __iomem *io, char __user *buf,
> loff_t off, size_t count, size_t x_start,
> size_t x_end, bool iswrite);
> +bool vfio_pci_core_range_intersect_range(loff_t buf_start, size_t buf_cnt,
> + loff_t reg_start, size_t reg_cnt,
> + loff_t *buf_offset,
> + size_t *intersect_count,
> + size_t *register_offset);
> #define VFIO_IOWRITE_DECLATION(size) \
> int vfio_pci_core_iowrite##size(struct vfio_pci_core_device *vdev, \
> bool test_mem, u##size val, void __iomem *io);
Reviewed-by: Yishai Hadas <yishaih@...dia.com>
Powered by blists - more mailing lists