| Message ID | 20240402032432.41004-3-liulongfang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add debugfs to hisilicon migration driver | expand |
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
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 --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
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(-)