[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74673bae-191a-b17d-d0e8-01ad07cdf180@huawei.com>
Date: Tue, 15 Jul 2025 15:25:15 +0800
From: liulongfang <liulongfang@...wei.com>
To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>,
"alex.williamson@...hat.com" <alex.williamson@...hat.com>, "jgg@...dia.com"
<jgg@...dia.com>, Jonathan Cameron <jonathan.cameron@...wei.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linuxarm@...neuler.org" <linuxarm@...neuler.org>
Subject: Re: [PATCH v5 1/3] migration: update BAR space size
On 2025/7/8 16:02, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: liulongfang <liulongfang@...wei.com>
>> Sent: Monday, June 30, 2025 9:54 AM
>> To: alex.williamson@...hat.com; jgg@...dia.com; Shameerali Kolothum
>> Thodi <shameerali.kolothum.thodi@...wei.com>; Jonathan Cameron
>> <jonathan.cameron@...wei.com>
>> Cc: kvm@...r.kernel.org; linux-kernel@...r.kernel.org;
>> linuxarm@...neuler.org; liulongfang <liulongfang@...wei.com>
>> Subject: [PATCH v5 1/3] migration: update BAR space size
>>
>> On new platforms greater than QM_HW_V3, the live migration
>> configuration
>> region is moved from VF to PF. The VF's own configuration space is
>> restored to the complete 64KB, and there is no need to divide the
>> size of the BAR configuration space equally.
>>
>> Signed-off-by: Longfang Liu <liulongfang@...wei.com>
>> ---
>
> See one minor comment below.
>
> LGTM:
> Reviewed-by: Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>
>
>> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 36 ++++++++++++++-----
>> 1 file changed, 27 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> index 2149f49aeec7..1ddc9dbadb70 100644
>> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> @@ -1250,6 +1250,28 @@ static struct hisi_qm *hisi_acc_get_pf_qm(struct
>> pci_dev *pdev)
>> return !IS_ERR(pf_qm) ? pf_qm : NULL;
>> }
>>
>> +static size_t hisi_acc_get_resource_len(struct vfio_pci_core_device *vdev,
>> + unsigned int index)
>> +{
>> + struct hisi_acc_vf_core_device *hisi_acc_vdev =
>> + hisi_acc_drvdata(vdev->pdev);
>> +
>> + /*
>> + * On the old QM_HW_V3 device, the ACC VF device BAR2
>> + * region encompasses both functional register space
>> + * and migration control register space.
>> + * only the functional region should be report to Guest.
>> + *
>> + * On the new HW device, the migration control register
>> + * has been moved to the PF device BAR2 region.
>> + * The VF device BAR2 is entirely functional register space.
>> + */
>
> Nit: May be move the above comment about new HW to the below
> check is better.
>
OK.
Thanks.
Longfang.
>> + if (hisi_acc_vdev->pf_qm->ver == QM_HW_V3)
>> + return (pci_resource_len(vdev->pdev, index) >> 1);
>> +
>> + return pci_resource_len(vdev->pdev, index);
>> +}
>> +
>> static int hisi_acc_pci_rw_access_check(struct vfio_device *core_vdev,
>> size_t count, loff_t *ppos,
>> size_t *new_count)
>> @@ -1260,8 +1282,9 @@ static int hisi_acc_pci_rw_access_check(struct
>> vfio_device *core_vdev,
>>
>> if (index == VFIO_PCI_BAR2_REGION_INDEX) {
>> loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> - resource_size_t end = pci_resource_len(vdev->pdev, index) /
>> 2;
>> + resource_size_t end;
>>
>> + end = hisi_acc_get_resource_len(vdev, index);
>> /* Check if access is for migration control region */
>> if (pos >= end)
>> return -EINVAL;
>> @@ -1282,8 +1305,9 @@ static int hisi_acc_vfio_pci_mmap(struct
>> vfio_device *core_vdev,
>> index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
>> if (index == VFIO_PCI_BAR2_REGION_INDEX) {
>> u64 req_len, pgoff, req_start;
>> - resource_size_t end = pci_resource_len(vdev->pdev, index) /
>> 2;
>> + resource_size_t end;
>>
>> + end = hisi_acc_get_resource_len(vdev, index);
>> req_len = vma->vm_end - vma->vm_start;
>> pgoff = vma->vm_pgoff &
>> ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
>> @@ -1330,7 +1354,6 @@ static long hisi_acc_vfio_pci_ioctl(struct
>> vfio_device *core_vdev, unsigned int
>> if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
>> struct vfio_pci_core_device *vdev =
>> container_of(core_vdev, struct vfio_pci_core_device,
>> vdev);
>> - struct pci_dev *pdev = vdev->pdev;
>> struct vfio_region_info info;
>> unsigned long minsz;
>>
>> @@ -1345,12 +1368,7 @@ static long hisi_acc_vfio_pci_ioctl(struct
>> vfio_device *core_vdev, unsigned int
>> if (info.index == VFIO_PCI_BAR2_REGION_INDEX) {
>> info.offset =
>> VFIO_PCI_INDEX_TO_OFFSET(info.index);
>>
>> - /*
>> - * ACC VF dev BAR2 region consists of both
>> functional
>> - * register space and migration control register
>> space.
>> - * Report only the functional region to Guest.
>> - */
>> - info.size = pci_resource_len(pdev, info.index) / 2;
>> + info.size = hisi_acc_get_resource_len(vdev,
>> info.index);
>>
>> info.flags = VFIO_REGION_INFO_FLAG_READ |
>> VFIO_REGION_INFO_FLAG_WRITE |
>> --
>> 2.24.0
>
> .
>
Powered by blists - more mailing lists