Re: [PATCH v5 1/3] migration: update BAR space size

[liulongfang <liulongfang@huawei.com>]

On 2025/7/8 16:02, Shameerali Kolothum Thodi wrote:
> 
> 
>> -----Original Message-----
>> From: liulongfang <liulongfang@huawei.com>
>> Sent: Monday, June 30, 2025 9:54 AM
>> To: alex.williamson@redhat.com; jgg@nvidia.com; Shameerali Kolothum
>> Thodi <shameerali.kolothum.thodi@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>
>> Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
>> linuxarm@openeuler.org; liulongfang <liulongfang@huawei.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@huawei.com>
>> ---
> 
> See one minor comment below.
> 
> LGTM:
> Reviewed-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.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
> 
> .
>