linux-kernel - Re: [Linuxarm] [PATCH 1/2] hisi_acc_vfio_pci: Add debugfs to migration driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7c041671-71bf-d558-d47a-70296fe5c30e@huawei.com>
Date:   Mon, 17 Oct 2022 17:20:34 +0800
From:   liulongfang <liulongfang@...wei.com>
To:     John Garry <john.garry@...wei.com>, <alex.williamson@...hat.com>,
        <jgg@...dia.com>, <shameerali.kolothum.thodi@...wei.com>
CC:     <cohuck@...hat.com>, <linux-kernel@...r.kernel.org>,
        <linuxarm@...neuler.org>
Subject: Re: [Linuxarm] [PATCH 1/2] hisi_acc_vfio_pci: Add debugfs to
 migration driver

On 2022/10/14 17:20, John Garry wrote:
> On 14/10/2022 03:57, Longfang Liu wrote:
>> +static void hisi_acc_vf_debugfs_init(struct hisi_acc_vf_core_device *hisi_acc_vdev)
>> +{
>> +    struct pci_dev *vf_pdev = hisi_acc_vdev->vf_dev;
>> +    struct device *dev = &vf_pdev->dev;
>> +    int ret;
>> +
>> +    if (!atomic_read(&hisi_acc_root_ref))
>> +        hisi_acc_debugfs_root = debugfs_create_dir("hisi_vfio_acc", NULL);
>> +    atomic_inc(&hisi_acc_root_ref);
>> +
> 
> This looks totally racy, such that I wonder why even bother using an atomic for hisi_acc_root_ref.


When enabling VF, it is possible for multiple VMs to enable VF at the same time. The atomic variable
is used to ensure that only one "hisi_vfio_acc" is created. When other VFs are enabled,
it will not be created again, but will be used directly.

 Indeed, why is hisi_acc_debugfs_root not created in the driver module init?
>
Because the normal function of VF is to perform encryption and decryption services, the live migration
function is an auxiliary function, which no need to be used in scenarios where only encryption and
decryption services are performed.

During module init, it can register ops(hisi_acc_vfio_pci_ops) that only perform encryption and
decryption services, and can also register with live migration function ops(hisi_acc_vfio_pci_migrn_ops),
and this debugfs only needs to register it when the the ops is hisi_acc_vfio_pci_migrn_ops.

> Thanks,
> John
> .
>