diff mbox series

[v4,2/4] hisi_acc_vfio_pci: Create subfunction for data reading

Message ID 20240402032432.41004-3-liulongfang@huawei.com (mailing list archive)
State New, archived
Headers show
Series add debugfs to hisilicon migration driver | expand

Commit Message

Longfang Liu April 2, 2024, 3:24 a.m. UTC
During the live migration process. It needs to obtain various status
data of drivers and devices. In order to facilitate calling it in the
debugfs function. For all operations that read data from device registers,
the driver creates a subfunction.
Also fixed the location of address data.

Signed-off-by: Longfang Liu <liulongfang@huawei.com>
---
 .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 56 +++++++++++--------
 .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h    |  3 +
 2 files changed, 37 insertions(+), 22 deletions(-)

Comments

Alex Williamson April 4, 2024, 8:07 p.m. UTC | #1
On Tue, 2 Apr 2024 11:24:30 +0800
Longfang Liu <liulongfang@huawei.com> wrote:

> During the live migration process. It needs to obtain various status
> data of drivers and devices. In order to facilitate calling it in the
> debugfs function. For all operations that read data from device registers,
> the driver creates a subfunction.
> Also fixed the location of address data.

Cédric noted privately and I agree, 1) fixes should be provided in
separate patches with a Fixes: tag rather than subtly included in a
minor refactoring, and 2) what does this imply about the existing
functionality of migration?  This would seem to suggest existing
migration data is bogus if we're offset by a register reading the DMA
address.  The commit log for the Fixes patch should describe this.

> 
> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> ---
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 56 +++++++++++--------
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h    |  3 +
>  2 files changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 45351be8e270..bf358ba94b5d 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -486,6 +486,39 @@ static int vf_qm_load_data(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>  	return 0;
>  }
>  
> +static int vf_qm_read_data(struct hisi_qm *vf_qm, struct acc_vf_data *vf_data)
> +{
> +	struct device *dev = &vf_qm->pdev->dev;
> +	int ret;
> +
> +	ret = qm_get_regs(vf_qm, vf_data);
> +	if (ret)
> +		return -EINVAL;
> +
> +	/* Every reg is 32 bit, the dma address is 64 bit. */
> +	vf_data->eqe_dma = vf_data->qm_eqc_dw[QM_XQC_ADDR_HIGH];
> +	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
> +	vf_data->eqe_dma |= vf_data->qm_eqc_dw[QM_XQC_ADDR_LOW];
> +	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[QM_XQC_ADDR_HIGH];
> +	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
> +	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[QM_XQC_ADDR_LOW];
> +
> +	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
> +	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
> +	if (ret) {
> +		dev_err(dev, "failed to read SQC addr!\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
> +	if (ret) {
> +		dev_err(dev, "failed to read CQC addr!\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>  			    struct hisi_acc_vf_migration_file *migf)
>  {
> @@ -511,31 +544,10 @@ static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>  		return ret;
>  	}
>  
> -	ret = qm_get_regs(vf_qm, vf_data);
> +	ret = vf_qm_read_data(vf_qm, vf_data);
>  	if (ret)
>  		return -EINVAL;
>  
> -	/* Every reg is 32 bit, the dma address is 64 bit. */
> -	vf_data->eqe_dma = vf_data->qm_eqc_dw[1];
> -	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
> -	vf_data->eqe_dma |= vf_data->qm_eqc_dw[0];
> -	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[1];
> -	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
> -	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[0];
> -
> -	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
> -	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
> -	if (ret) {
> -		dev_err(dev, "failed to read SQC addr!\n");
> -		return -EINVAL;
> -	}
> -
> -	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
> -	if (ret) {
> -		dev_err(dev, "failed to read CQC addr!\n");
> -		return -EINVAL;
> -	}
> -
>  	migf->total_length = sizeof(struct acc_vf_data);
>  	return 0;
>  }
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
> index 5bab46602fad..7a9dc87627cd 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
> @@ -38,6 +38,9 @@
>  #define QM_REG_ADDR_OFFSET	0x0004
>  
>  #define QM_XQC_ADDR_OFFSET	32U
> +#define QM_XQC_ADDR_LOW		0x1
> +#define QM_XQC_ADDR_HIGH	0x2
> +
>  #define QM_VF_AEQ_INT_MASK	0x0004
>  #define QM_VF_EQ_INT_MASK	0x000c
>  #define QM_IFC_INT_SOURCE_V	0x0020
Longfang Liu April 12, 2024, 3:07 a.m. UTC | #2
On 2024/4/5 4:07, Alex Williamson wrote:
> On Tue, 2 Apr 2024 11:24:30 +0800
> Longfang Liu <liulongfang@huawei.com> wrote:
> 
>> During the live migration process. It needs to obtain various status
>> data of drivers and devices. In order to facilitate calling it in the
>> debugfs function. For all operations that read data from device registers,
>> the driver creates a subfunction.
>> Also fixed the location of address data.
> 
> Cédric noted privately and I agree, 1) fixes should be provided in
> separate patches with a Fixes: tag rather than subtly included in a
> minor refactoring, and 2) what does this imply about the existing
> functionality of migration?  This would seem to suggest existing
> migration data is bogus if we're offset by a register reading the DMA
> address.  The commit log for the Fixes patch should describe this.
>

Okay, the modification of the DMA address offset part is split into
a new patch, and the modification of this part is explained clearly.

Thanks,
Longfang.

>>
>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
>> ---
>>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 56 +++++++++++--------
>>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h    |  3 +
>>  2 files changed, 37 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> index 45351be8e270..bf358ba94b5d 100644
>> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> @@ -486,6 +486,39 @@ static int vf_qm_load_data(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>>  	return 0;
>>  }
>>  
>> +static int vf_qm_read_data(struct hisi_qm *vf_qm, struct acc_vf_data *vf_data)
>> +{
>> +	struct device *dev = &vf_qm->pdev->dev;
>> +	int ret;
>> +
>> +	ret = qm_get_regs(vf_qm, vf_data);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	/* Every reg is 32 bit, the dma address is 64 bit. */
>> +	vf_data->eqe_dma = vf_data->qm_eqc_dw[QM_XQC_ADDR_HIGH];
>> +	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
>> +	vf_data->eqe_dma |= vf_data->qm_eqc_dw[QM_XQC_ADDR_LOW];
>> +	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[QM_XQC_ADDR_HIGH];
>> +	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
>> +	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[QM_XQC_ADDR_LOW];
>> +
>> +	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
>> +	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
>> +	if (ret) {
>> +		dev_err(dev, "failed to read SQC addr!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
>> +	if (ret) {
>> +		dev_err(dev, "failed to read CQC addr!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>>  			    struct hisi_acc_vf_migration_file *migf)
>>  {
>> @@ -511,31 +544,10 @@ static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>>  		return ret;
>>  	}
>>  
>> -	ret = qm_get_regs(vf_qm, vf_data);
>> +	ret = vf_qm_read_data(vf_qm, vf_data);
>>  	if (ret)
>>  		return -EINVAL;
>>  
>> -	/* Every reg is 32 bit, the dma address is 64 bit. */
>> -	vf_data->eqe_dma = vf_data->qm_eqc_dw[1];
>> -	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
>> -	vf_data->eqe_dma |= vf_data->qm_eqc_dw[0];
>> -	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[1];
>> -	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
>> -	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[0];
>> -
>> -	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
>> -	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
>> -	if (ret) {
>> -		dev_err(dev, "failed to read SQC addr!\n");
>> -		return -EINVAL;
>> -	}
>> -
>> -	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
>> -	if (ret) {
>> -		dev_err(dev, "failed to read CQC addr!\n");
>> -		return -EINVAL;
>> -	}
>> -
>>  	migf->total_length = sizeof(struct acc_vf_data);
>>  	return 0;
>>  }
>> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
>> index 5bab46602fad..7a9dc87627cd 100644
>> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
>> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
>> @@ -38,6 +38,9 @@
>>  #define QM_REG_ADDR_OFFSET	0x0004
>>  
>>  #define QM_XQC_ADDR_OFFSET	32U
>> +#define QM_XQC_ADDR_LOW		0x1
>> +#define QM_XQC_ADDR_HIGH	0x2
>> +
>>  #define QM_VF_AEQ_INT_MASK	0x0004
>>  #define QM_VF_EQ_INT_MASK	0x000c
>>  #define QM_IFC_INT_SOURCE_V	0x0020
> 
> .
>
diff mbox series

Patch

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 45351be8e270..bf358ba94b5d 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -486,6 +486,39 @@  static int vf_qm_load_data(struct hisi_acc_vf_core_device *hisi_acc_vdev,
 	return 0;
 }
 
+static int vf_qm_read_data(struct hisi_qm *vf_qm, struct acc_vf_data *vf_data)
+{
+	struct device *dev = &vf_qm->pdev->dev;
+	int ret;
+
+	ret = qm_get_regs(vf_qm, vf_data);
+	if (ret)
+		return -EINVAL;
+
+	/* Every reg is 32 bit, the dma address is 64 bit. */
+	vf_data->eqe_dma = vf_data->qm_eqc_dw[QM_XQC_ADDR_HIGH];
+	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
+	vf_data->eqe_dma |= vf_data->qm_eqc_dw[QM_XQC_ADDR_LOW];
+	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[QM_XQC_ADDR_HIGH];
+	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
+	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[QM_XQC_ADDR_LOW];
+
+	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
+	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
+	if (ret) {
+		dev_err(dev, "failed to read SQC addr!\n");
+		return -EINVAL;
+	}
+
+	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
+	if (ret) {
+		dev_err(dev, "failed to read CQC addr!\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
 			    struct hisi_acc_vf_migration_file *migf)
 {
@@ -511,31 +544,10 @@  static int vf_qm_state_save(struct hisi_acc_vf_core_device *hisi_acc_vdev,
 		return ret;
 	}
 
-	ret = qm_get_regs(vf_qm, vf_data);
+	ret = vf_qm_read_data(vf_qm, vf_data);
 	if (ret)
 		return -EINVAL;
 
-	/* Every reg is 32 bit, the dma address is 64 bit. */
-	vf_data->eqe_dma = vf_data->qm_eqc_dw[1];
-	vf_data->eqe_dma <<= QM_XQC_ADDR_OFFSET;
-	vf_data->eqe_dma |= vf_data->qm_eqc_dw[0];
-	vf_data->aeqe_dma = vf_data->qm_aeqc_dw[1];
-	vf_data->aeqe_dma <<= QM_XQC_ADDR_OFFSET;
-	vf_data->aeqe_dma |= vf_data->qm_aeqc_dw[0];
-
-	/* Through SQC_BT/CQC_BT to get sqc and cqc address */
-	ret = qm_get_sqc(vf_qm, &vf_data->sqc_dma);
-	if (ret) {
-		dev_err(dev, "failed to read SQC addr!\n");
-		return -EINVAL;
-	}
-
-	ret = qm_get_cqc(vf_qm, &vf_data->cqc_dma);
-	if (ret) {
-		dev_err(dev, "failed to read CQC addr!\n");
-		return -EINVAL;
-	}
-
 	migf->total_length = sizeof(struct acc_vf_data);
 	return 0;
 }
diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
index 5bab46602fad..7a9dc87627cd 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h
@@ -38,6 +38,9 @@ 
 #define QM_REG_ADDR_OFFSET	0x0004
 
 #define QM_XQC_ADDR_OFFSET	32U
+#define QM_XQC_ADDR_LOW		0x1
+#define QM_XQC_ADDR_HIGH	0x2
+
 #define QM_VF_AEQ_INT_MASK	0x0004
 #define QM_VF_EQ_INT_MASK	0x000c
 #define QM_IFC_INT_SOURCE_V	0x0020