linux-kernel - Re: [PATCH v5] vfio error recovery: kernel support

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] [day] [month] [year] [list]

Message-ID: <58D330D7.3090908@cn.fujitsu.com>
Date:   Thu, 23 Mar 2017 10:20:07 +0800
From:   Cao jin <caoj.fnst@...fujitsu.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
CC:     <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
        <qemu-devel@...gnu.org>, <izumi.taku@...fujitsu.com>,
        <alex.williamson@...hat.com>
Subject: Re: [PATCH v5] vfio error recovery: kernel support



On 03/22/2017 09:10 PM, Michael S. Tsirkin wrote:
> Minor comments on commit log below.
> 
> On Wed, Mar 22, 2017 at 06:34:23PM +0800, Cao jin wrote:
>> From: "Michael S. Tsirkin" <mst@...hat.com>
>>

> 
>> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
>> Signed-off-by: Cao jin <caoj.fnst@...fujitsu.com>
>> ---
>>
>> v5 changelog:
>> 1. Add another new eventfd passive_reset_trigger & the boilerplate code,
>>    used in slot_reset. Add comment for slot_reset().
>> 2. Rewrite the commit log.
>>
>>  drivers/vfio/pci/vfio_pci.c         | 49 +++++++++++++++++++++++++++++++++++--
>>  drivers/vfio/pci/vfio_pci_intrs.c   | 38 ++++++++++++++++++++++++++++
>>  drivers/vfio/pci/vfio_pci_private.h |  2 ++
>>  include/uapi/linux/vfio.h           |  2 ++
>>  4 files changed, 89 insertions(+), 2 deletions(-)
>>

> 
> 
>> +static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
>> +{
>> +	struct vfio_pci_device *vdev;
>> +	struct vfio_device *device;
>> +	static pci_ers_result_t err = PCI_ERS_RESULT_NONE;
>> +
>> +	device = vfio_device_get_from_dev(&pdev->dev);
>> +	if (!device)
>> +		goto err_dev;
>> +
>> +	vdev = vfio_device_data(device);
>> +	if (!vdev)
>> +		goto err_data;
>> +
>> +	mutex_lock(&vdev->igate);
>> +
>> +	if (vdev->passive_reset_trigger)
>> +		eventfd_signal(vdev->passive_reset_trigger, 1);
>> +	else if (vdev->err_trigger)
>> +		eventfd_signal(vdev->err_trigger, 1);
> 
> why is this chunk here? why not just do
> 
> 	if (vdev->passive_reset_trigger)
> 		eventfd_signal(vdev->passive_reset_trigger, 1);
> 
> without a fallback?
> 
> 

I thought it is one way of "passing maximum info to userspace and let it
decide."

-- 
Sincerely,
Cao jin