From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E1692701C4; Tue, 15 Jul 2025 07:15:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.189 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752563751; cv=none; b=gR+mC+0UElsdlbaGe30zw7V6UlRS7O8taKaF5HEeBnq8ykT8/MvifM3o5DS0tPQUCo2nDCIBWRTfgKb/4hfMp9ja+N8dt+D27dFVRiT2UHpiaCfr+aMIDm0BJJs2oESBVFD/XH/yK0oI854HpvXQE3f335fvKi/1SpnF7IFOb4Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752563751; c=relaxed/simple; bh=1IovWaOhiidYGU/HqcJw2Lzr3Uv4x8gXnuT/WGIJ3r8=; h=Subject:To:CC:References:From:Message-ID:Date:MIME-Version: In-Reply-To:Content-Type; b=Zeiv947tzEMxwTLnQUQSAU57MFkaGeXz0hLsYQ23g5MoG6iS4RL97Z6whj6cY6t7nRh+S3VtOuOkTl2zqO3sVxTdp7dZDUYUdgwa38dM1wUkPLgRhBRjUGTQeRU1z+kRQgRSW6tzO2beYnhrVcPeLRXEWUaJxy6WEz3p8DrxCEY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.189 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.162.254]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4bh9N51y9hzHrKN; Tue, 15 Jul 2025 15:11:33 +0800 (CST) Received: from dggpemf500015.china.huawei.com (unknown [7.185.36.143]) by mail.maildlp.com (Postfix) with ESMTPS id 08101180450; Tue, 15 Jul 2025 15:15:40 +0800 (CST) Received: from [10.67.121.110] (10.67.121.110) by dggpemf500015.china.huawei.com (7.185.36.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 15 Jul 2025 15:15:39 +0800 Subject: Re: [PATCH v5 3/3] migration: adapt to new migration configuration To: Shameerali Kolothum Thodi , "alex.williamson@redhat.com" , "jgg@nvidia.com" , Jonathan Cameron CC: "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linuxarm@openeuler.org" References: <20250630085402.7491-1-liulongfang@huawei.com> <20250630085402.7491-4-liulongfang@huawei.com> <7bdf1024bdcd4ba6b0bce352caefdefc@huawei.com> From: liulongfang Message-ID: <38061357-1b9b-1e35-5273-0ebb1d7bcadd@huawei.com> Date: Tue, 15 Jul 2025 15:15:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <7bdf1024bdcd4ba6b0bce352caefdefc@huawei.com> Content-Type: text/plain; charset="gbk" Content-Transfer-Encoding: 7bit X-ClientProxiedBy: kwepems100002.china.huawei.com (7.221.188.206) To dggpemf500015.china.huawei.com (7.185.36.143) On 2025/7/8 16:28, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: liulongfang >> Sent: Monday, June 30, 2025 9:54 AM >> To: alex.williamson@redhat.com; jgg@nvidia.com; Shameerali Kolothum >> Thodi ; Jonathan Cameron >> >> Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org; >> linuxarm@openeuler.org; liulongfang >> Subject: [PATCH v5 3/3] migration: adapt to new migration configuration >> >> On new platforms greater than QM_HW_V3, the migration region has been >> relocated from the VF to the PF. The driver must also be modified >> accordingly to adapt to the new hardware device. >> >> Utilize the PF's I/O base directly on the new hardware platform, >> and no mmap operation is required. If it is on an old platform, >> the driver needs to be compatible with the old solution. >> >> Signed-off-by: Longfang Liu >> --- >> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 166 ++++++++++++------ >> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 7 + >> 2 files changed, 120 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> index 1ddc9dbadb70..3aec3b92787f 100644 >> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> @@ -125,6 +125,72 @@ static int qm_get_cqc(struct hisi_qm *qm, u64 >> *addr) >> return 0; >> } >> >> +static int qm_get_xqc_regs(struct hisi_acc_vf_core_device *hisi_acc_vdev, >> + struct acc_vf_data *vf_data) >> +{ >> + struct hisi_qm *qm = &hisi_acc_vdev->vf_qm; >> + struct device *dev = &qm->pdev->dev; >> + u32 eqc_addr, aeqc_addr; >> + int ret; >> + >> + if (qm->ver == QM_HW_V3) { >> + eqc_addr = QM_EQC_DW0; >> + aeqc_addr = QM_AEQC_DW0; >> + } else { >> + eqc_addr = QM_EQC_PF_DW0; >> + aeqc_addr = QM_AEQC_PF_DW0; >> + } >> + >> + /* QM_EQC_DW has 7 regs */ >> + ret = qm_read_regs(qm, eqc_addr, vf_data->qm_eqc_dw, 7); >> + if (ret) { >> + dev_err(dev, "failed to read QM_EQC_DW\n"); >> + return ret; >> + } >> + >> + /* QM_AEQC_DW has 7 regs */ >> + ret = qm_read_regs(qm, aeqc_addr, vf_data->qm_aeqc_dw, 7); >> + if (ret) { >> + dev_err(dev, "failed to read QM_AEQC_DW\n"); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int qm_set_xqc_regs(struct hisi_acc_vf_core_device *hisi_acc_vdev, >> + struct acc_vf_data *vf_data) >> +{ >> + struct hisi_qm *qm = &hisi_acc_vdev->vf_qm; >> + struct device *dev = &qm->pdev->dev; >> + u32 eqc_addr, aeqc_addr; >> + int ret; >> + >> + if (qm->ver == QM_HW_V3) { >> + eqc_addr = QM_EQC_DW0; >> + aeqc_addr = QM_AEQC_DW0; >> + } else { >> + eqc_addr = QM_EQC_PF_DW0; >> + aeqc_addr = QM_AEQC_PF_DW0; >> + } >> + >> + /* QM_EQC_DW has 7 regs */ >> + ret = qm_write_regs(qm, eqc_addr, vf_data->qm_eqc_dw, 7); >> + if (ret) { >> + dev_err(dev, "failed to write QM_EQC_DW\n"); >> + return ret; >> + } >> + >> + /* QM_AEQC_DW has 7 regs */ >> + ret = qm_write_regs(qm, aeqc_addr, vf_data->qm_aeqc_dw, 7); >> + if (ret) { >> + dev_err(dev, "failed to write QM_AEQC_DW\n"); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> static int qm_get_regs(struct hisi_qm *qm, struct acc_vf_data *vf_data) >> { >> struct device *dev = &qm->pdev->dev; >> @@ -167,20 +233,6 @@ static int qm_get_regs(struct hisi_qm *qm, struct >> acc_vf_data *vf_data) >> return ret; >> } >> >> - /* QM_EQC_DW has 7 regs */ >> - ret = qm_read_regs(qm, QM_EQC_DW0, vf_data->qm_eqc_dw, 7); >> - if (ret) { >> - dev_err(dev, "failed to read QM_EQC_DW\n"); >> - return ret; >> - } >> - >> - /* QM_AEQC_DW has 7 regs */ >> - ret = qm_read_regs(qm, QM_AEQC_DW0, vf_data->qm_aeqc_dw, >> 7); >> - if (ret) { >> - dev_err(dev, "failed to read QM_AEQC_DW\n"); >> - return ret; >> - } >> - >> return 0; >> } >> >> @@ -239,20 +291,6 @@ static int qm_set_regs(struct hisi_qm *qm, struct >> acc_vf_data *vf_data) >> return ret; >> } >> >> - /* QM_EQC_DW has 7 regs */ >> - ret = qm_write_regs(qm, QM_EQC_DW0, vf_data->qm_eqc_dw, 7); >> - if (ret) { >> - dev_err(dev, "failed to write QM_EQC_DW\n"); >> - return ret; >> - } >> - >> - /* QM_AEQC_DW has 7 regs */ >> - ret = qm_write_regs(qm, QM_AEQC_DW0, vf_data->qm_aeqc_dw, >> 7); >> - if (ret) { >> - dev_err(dev, "failed to write QM_AEQC_DW\n"); >> - return ret; >> - } >> - >> return 0; >> } >> >> @@ -522,6 +560,10 @@ static int vf_qm_load_data(struct >> hisi_acc_vf_core_device *hisi_acc_vdev, >> return ret; >> } >> >> + ret = qm_set_xqc_regs(hisi_acc_vdev, vf_data); >> + if (ret) >> + return ret; >> + >> ret = hisi_qm_mb(qm, QM_MB_CMD_SQC_BT, qm->sqc_dma, 0, 0); >> if (ret) { >> dev_err(dev, "set sqc failed\n"); >> @@ -589,6 +631,10 @@ static int vf_qm_state_save(struct >> hisi_acc_vf_core_device *hisi_acc_vdev, >> vf_data->vf_qm_state = QM_READY; >> hisi_acc_vdev->vf_qm_state = vf_data->vf_qm_state; >> >> + ret = qm_get_xqc_regs(hisi_acc_vdev, vf_data); >> + if (ret) >> + return ret; >> + >> ret = vf_qm_read_data(vf_qm, vf_data); >> if (ret) >> return ret; >> @@ -1186,34 +1232,47 @@ static int hisi_acc_vf_qm_init(struct >> hisi_acc_vf_core_device *hisi_acc_vdev) >> { >> struct vfio_pci_core_device *vdev = &hisi_acc_vdev->core_device; >> struct hisi_qm *vf_qm = &hisi_acc_vdev->vf_qm; >> + struct hisi_qm *pf_qm = hisi_acc_vdev->pf_qm; >> struct pci_dev *vf_dev = vdev->pdev; >> >> - /* >> - * ACC VF dev BAR2 region consists of both functional register space >> - * and migration control register space. For migration to work, we >> - * need access to both. Hence, we map the entire BAR2 region here. >> - * But unnecessarily exposing the migration BAR region to the Guest >> - * has the potential to prevent/corrupt the Guest migration. Hence, >> - * we restrict access to the migration control space from >> - * Guest(Please see mmap/ioctl/read/write override functions). >> - * >> - * Please note that it is OK to expose the entire VF BAR if migration >> - * is not supported or required as this cannot affect the ACC PF >> - * configurations. >> - * >> - * Also the HiSilicon ACC VF devices supported by this driver on >> - * HiSilicon hardware platforms are integrated end point devices >> - * and the platform lacks the capability to perform any PCIe P2P >> - * between these devices. >> - */ >> + if (pf_qm->ver == QM_HW_V3) { >> + /* >> + * ACC VF dev BAR2 region consists of both functional >> register space >> + * and migration control register space. For migration to >> work, we >> + * need access to both. Hence, we map the entire BAR2 >> region here. >> + * But unnecessarily exposing the migration BAR region to >> the Guest >> + * has the potential to prevent/corrupt the Guest migration. >> Hence, >> + * we restrict access to the migration control space from >> + * Guest(Please see mmap/ioctl/read/write override >> functions). >> + * >> + * Please note that it is OK to expose the entire VF BAR if >> migration >> + * is not supported or required as this cannot affect the ACC >> PF >> + * configurations. >> + * >> + * Also the HiSilicon ACC VF devices supported by this driver >> on >> + * HiSilicon hardware platforms are integrated end point >> devices >> + * and the platform lacks the capability to perform any PCIe >> P2P >> + * between these devices. >> + */ >> >> - vf_qm->io_base = >> - ioremap(pci_resource_start(vf_dev, >> VFIO_PCI_BAR2_REGION_INDEX), >> - pci_resource_len(vf_dev, >> VFIO_PCI_BAR2_REGION_INDEX)); >> - if (!vf_qm->io_base) >> - return -EIO; >> + vf_qm->io_base = >> + ioremap(pci_resource_start(vf_dev, >> VFIO_PCI_BAR2_REGION_INDEX), >> + pci_resource_len(vf_dev, >> VFIO_PCI_BAR2_REGION_INDEX)); >> + if (!vf_qm->io_base) >> + return -EIO; >> >> - vf_qm->fun_type = QM_HW_VF; >> + vf_qm->fun_type = QM_HW_VF; >> + vf_qm->ver = pf_qm->ver; >> + } else { >> + /* >> + * On hardware platforms greater than QM_HW_V3, the >> migration function >> + * register is placed in the BAR2 configuration region of the >> PF, >> + * and each VF device occupies 8KB of configuration space. >> + */ >> + vf_qm->io_base = pf_qm->io_base + >> QM_MIG_REGION_OFFSET + >> + hisi_acc_vdev->vf_id * >> QM_MIG_REGION_SIZE; >> + vf_qm->fun_type = QM_HW_PF; > > I don't think you can use the QM fun_type to distinguish this, because it is still a > VF dev. Better to have another field to detect this. > The devices that explicitly support live migration have already been identified during probe, and only versions >= QM_HW_V3 are supported. Among these, only QM_HW_V3 uses the VF's BAR2 configuration space. The others use the PF's configuration space. This difference is already distinguishable through the QM fun_type, and both functional verification and testing are OK. > Also I have another question. In the hisi_acc_vfio_pci_probe() we currently > look for pf_qm-> >= QM_HW_V3. This means current driver can get loaded > on this new hardware, right? If so, I think we need to prevent that. And also > we need to block any migration attempt from existing host kernels to new > ones. > The current driver can be loaded on both old QM_HW_V3 hardware and new hardware platforms. This is because they only differ in the io_base, while other functionalities are the same. Additionally, the compatibility issue for live migration between old and new devices is handled. Compatibility checks are performed during vf_qm_check_match --> vf_qm_version_check to ensure normal functionality after migration. Thanks, Longfang. > Thanks, > Shameer > >> + } >> vf_qm->pdev = vf_dev; >> mutex_init(&vf_qm->mailbox_lock); >> >> @@ -1539,7 +1598,8 @@ static void hisi_acc_vfio_pci_close_device(struct >> vfio_device *core_vdev) >> hisi_acc_vf_disable_fds(hisi_acc_vdev); >> mutex_lock(&hisi_acc_vdev->open_mutex); >> hisi_acc_vdev->dev_opened = false; >> - iounmap(vf_qm->io_base); >> + if (vf_qm->ver == QM_HW_V3) >> + iounmap(vf_qm->io_base); >> mutex_unlock(&hisi_acc_vdev->open_mutex); >> vfio_pci_core_close_device(core_vdev); >> } >> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h >> b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h >> index 91002ceeebc1..348f8bb5b42c 100644 >> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h >> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.h >> @@ -59,6 +59,13 @@ >> #define ACC_DEV_MAGIC_V1 0XCDCDCDCDFEEDAACC >> #define ACC_DEV_MAGIC_V2 0xAACCFEEDDECADEDE >> >> +#define QM_MIG_REGION_OFFSET 0x180000 >> +#define QM_MIG_REGION_SIZE 0x2000 >> + >> +#define QM_SUB_VERSION_ID 0x100210 >> +#define QM_EQC_PF_DW0 0x1c00 >> +#define QM_AEQC_PF_DW0 0x1c20 >> + >> struct acc_vf_data { >> #define QM_MATCH_SIZE offsetofend(struct acc_vf_data, qm_rsv_state) >> /* QM match information */ >> -- >> 2.24.0 > > . >