From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CB2BC4332F for ; Sat, 17 Dec 2022 06:44:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230377AbiLQGoC (ORCPT ); Sat, 17 Dec 2022 01:44:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52168 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230286AbiLQGnh (ORCPT ); Sat, 17 Dec 2022 01:43:37 -0500 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 65C53680A9 for ; Fri, 16 Dec 2022 22:43:13 -0800 (PST) Received: from kwepemm600005.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4NYxGG0GqdzmWbK; Sat, 17 Dec 2022 14:41:58 +0800 (CST) Received: from [10.67.103.158] (10.67.103.158) by kwepemm600005.china.huawei.com (7.193.23.191) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Sat, 17 Dec 2022 14:43:00 +0800 Subject: Re: [PATCH v4 3/5] hisi_acc_vfio_pci: register debugfs for hisilicon migration driver To: Jason Gunthorpe CC: , , , , , References: <20221202092625.35075-1-liulongfang@huawei.com> <20221202092625.35075-4-liulongfang@huawei.com> From: liulongfang Message-ID: <9a84fd20-720e-2bc0-7646-6c344befbecd@huawei.com> Date: Sat, 17 Dec 2022 14:42:59 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="gbk" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.103.158] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600005.china.huawei.com (7.193.23.191) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022/12/2 23:21, Jason Gunthorpe Wrote: > On Fri, Dec 02, 2022 at 05:26:23PM +0800, Longfang Liu wrote: >> On the debugfs framework of VFIO, register the debug function >> for the live migration driver of the hisilicon accelerator device. >> >> On the basis of the original public debug function, a private debug >> function is added, so that the single-step debugging function >> during live migration can be realized. >> >> Signed-off-by: Longfang Liu >> --- >> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 250 ++++++++++++++++++ >> .../vfio/pci/hisilicon/hisi_acc_vfio_pci.h | 9 + >> drivers/vfio/pci/vfio_pci_debugfs.c | 10 +- >> 3 files changed, 268 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> index eb18edffbd5f..0f35cde6e8ec 100644 >> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c >> @@ -15,6 +15,7 @@ >> #include >> >> #include "hisi_acc_vfio_pci.h" >> +#include "../vfio_pci_debugfs.h" >> >> /* Return 0 on VM acc device ready, -ETIMEDOUT hardware timeout */ >> static int qm_wait_dev_not_ready(struct hisi_qm *qm) >> @@ -609,6 +610,18 @@ hisi_acc_check_int_state(struct hisi_acc_vf_core_device *hisi_acc_vdev) >> } >> } >> >> +static void hisi_acc_vf_migf_save(struct hisi_acc_vf_migration_file *src_migf, >> + struct hisi_acc_vf_migration_file *dst_migf) >> +{ >> + if (!dst_migf) >> + return; >> + >> + dst_migf->disabled = false; >> + dst_migf->total_length = src_migf->total_length; >> + memcpy(&dst_migf->vf_data, &src_migf->vf_data, >> + sizeof(struct acc_vf_data)); >> +} >> + >> static void hisi_acc_vf_disable_fd(struct hisi_acc_vf_migration_file *migf) >> { >> mutex_lock(&migf->lock); >> @@ -621,12 +634,16 @@ static void hisi_acc_vf_disable_fd(struct hisi_acc_vf_migration_file *migf) >> static void hisi_acc_vf_disable_fds(struct hisi_acc_vf_core_device *hisi_acc_vdev) >> { >> if (hisi_acc_vdev->resuming_migf) { >> + hisi_acc_vf_migf_save(hisi_acc_vdev->resuming_migf, >> + hisi_acc_vdev->debug_migf); >> hisi_acc_vf_disable_fd(hisi_acc_vdev->resuming_migf); >> fput(hisi_acc_vdev->resuming_migf->filp); >> hisi_acc_vdev->resuming_migf = NULL; >> } >> >> if (hisi_acc_vdev->saving_migf) { >> + hisi_acc_vf_migf_save(hisi_acc_vdev->saving_migf, >> + hisi_acc_vdev->debug_migf); >> hisi_acc_vf_disable_fd(hisi_acc_vdev->saving_migf); >> fput(hisi_acc_vdev->saving_migf->filp); >> hisi_acc_vdev->saving_migf = NULL; >> @@ -1188,6 +1205,231 @@ static long hisi_acc_vfio_pci_ioctl(struct vfio_device *core_vdev, unsigned int >> return vfio_pci_core_ioctl(core_vdev, cmd, arg); >> } >> >> +static int hisi_acc_vf_debug_io(struct vfio_device *vdev) >> +{ >> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev); >> + struct hisi_qm *vf_qm = &hisi_acc_vdev->vf_qm; >> + struct device *dev = vdev->dev; >> + u64 data; >> + int ret; >> + >> + data = readl(vf_qm->io_base + QM_MB_CMD_SEND_BASE); >> + dev_info(dev, "debug mailbox val: 0x%llx\n", data); >> + >> + ret = qm_wait_dev_not_ready(vf_qm); >> + if (ret) >> + dev_err(dev, "VF device not ready!\n"); > > debugfs has helpers for this, you should use debugfs_print_regs32() > I analyzed it, and there is no need to replace it here, because it is a subtype of the "debug" file operation, not an independent debugfs file operation. Thanks, Longfang. >> +static int hisi_acc_vf_debug_resume(struct vfio_device *vdev) >> +{ >> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev); >> + struct hisi_acc_vf_migration_file *migf = hisi_acc_vdev->debug_migf; >> + struct device *dev = vdev->dev; >> + int ret; >> + >> + ret = vf_qm_state_save(hisi_acc_vdev, migf); >> + if (ret) { >> + dev_err(dev, "failed to save device data!\n"); >> + return -EINVAL; >> + } >> + >> + ret = vf_qm_check_match(hisi_acc_vdev, migf); >> + if (ret) { >> + dev_err(dev, "failed to match the VF!\n"); >> + return -EINVAL; >> + } >> + >> + ret = vf_qm_load_data(hisi_acc_vdev, migf); >> + if (ret) { >> + dev_err(dev, "failed to recover the VF!\n"); >> + return -EINVAL; >> + } >> + >> + vf_qm_fun_reset(&hisi_acc_vdev->vf_qm); >> + dev_info(dev, "successful to resume device data!\n"); >> + >> + return 0; >> +} > > This doesn't seem like it belongs in debugfs at all, please just write > a test progam using normal vfio ioctls to do these steps and put in > the kernel selftests directory. > >> +static int hisi_acc_vf_debug_save(struct vfio_device *vdev) >> +{ >> + struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_get_vf_dev(vdev); >> + struct hisi_acc_vf_migration_file *migf = hisi_acc_vdev->debug_migf; >> + struct device *dev = vdev->dev; >> + int ret; >> + >> + ret = vf_qm_state_save(hisi_acc_vdev, migf); >> + if (ret) { >> + dev_err(dev, "failed to save device data!\n"); >> + return -EINVAL; >> + } >> + dev_info(dev, "successful to save device data!\n"); >> + >> + return 0; >> +} > > Ditto, I don't think debugfs should be accessing the device state > independently of the main FSM. > > Jason > . >