linux-kernel - Re: [RFC PATCH v4 08/10] i40e/vf_migration: VF live migration

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]

Date:   Thu, 11 Jun 2020 10:27:34 +0800
From:   Xiang Zheng <zhengxiang9@...wei.com>
To:     Yan Zhao <yan.y.zhao@...el.com>
CC:     <kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <alex.williamson@...hat.com>, <cohuck@...hat.com>,
        <zhenyuw@...ux.intel.com>, <zhi.a.wang@...el.com>,
        <kevin.tian@...el.com>, <shaopeng.he@...el.com>,
        <yi.l.liu@...el.com>, <xin.zeng@...el.com>, <hang.yuan@...el.com>,
        "Wang Haibin" <wanghaibin.wang@...wei.com>
Subject: Re: [RFC PATCH v4 08/10] i40e/vf_migration: VF live migration -
 pass-through VF first



On 2020/6/11 8:23, Yan Zhao wrote:
> On Wed, Jun 10, 2020 at 04:59:43PM +0800, Xiang Zheng wrote:
>> Hi Yan,
>>
>> few nits below...
>>
>> On 2020/5/18 10:53, Yan Zhao wrote:
>>> This driver intercepts all device operations as long as it's probed
>>> successfully by vfio-pci driver.
>>>
>>> It will process regions and irqs of its interest and then forward
>>> operations to default handlers exported from vfio pci if it wishes to.
>>>
>>> In this patch, this driver does nothing but pass through VFs to guest
>>> by calling to exported handlers from driver vfio-pci.
>>>
>>> Cc: Shaopeng He <shaopeng.he@...el.com>
>>>
>>> Signed-off-by: Yan Zhao <yan.y.zhao@...el.com>
>>> ---
>>>  drivers/net/ethernet/intel/Kconfig            |  10 ++
>>>  drivers/net/ethernet/intel/i40e/Makefile      |   2 +
>>>  .../ethernet/intel/i40e/i40e_vf_migration.c   | 165 ++++++++++++++++++
>>>  .../ethernet/intel/i40e/i40e_vf_migration.h   |  59 +++++++
>>>  4 files changed, 236 insertions(+)
>>>  create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.c
>>>  create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.h
>>>
>>> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
>>> index ad34e4335df2..31780d9a59f1 100644
>>> --- a/drivers/net/ethernet/intel/Kconfig
>>> +++ b/drivers/net/ethernet/intel/Kconfig
>>> @@ -264,6 +264,16 @@ config I40E_DCB
>>>  
>>>  	  If unsure, say N.
>>>  

[...]

>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_vf_migration.h b/drivers/net/ethernet/intel/i40e/i40e_vf_migration.h
>>> new file mode 100644
>>> index 000000000000..696d40601ec3
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_vf_migration.h
>>> @@ -0,0 +1,59 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/* Copyright(c) 2013 - 2019 Intel Corporation. */
>>> +
>>> +#ifndef I40E_MIG_H
>>> +#define I40E_MIG_H
>>> +
>>> +#include <linux/pci.h>
>>> +#include <linux/vfio.h>
>>> +#include <linux/mdev.h>
>>> +
>>> +#include "i40e.h"
>>> +#include "i40e_txrx.h"
>>> +
>>> +/* helper macros copied from vfio-pci */
>>> +#define VFIO_PCI_OFFSET_SHIFT   40
>>> +#define VFIO_PCI_OFFSET_TO_INDEX(off)   ((off) >> VFIO_PCI_OFFSET_SHIFT)
>>> +#define VFIO_PCI_INDEX_TO_OFFSET(index)	((u64)(index) << VFIO_PCI_OFFSET_SHIFT)
>>> +#define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
>>> +
>>> +/* Single Root I/O Virtualization */
>>> +struct pci_sriov {
>>> +	int		pos;		/* Capability position */
>>> +	int		nres;		/* Number of resources */
>>> +	u32		cap;		/* SR-IOV Capabilities */
>>> +	u16		ctrl;		/* SR-IOV Control */
>>> +	u16		total_VFs;	/* Total VFs associated with the PF */
>>> +	u16		initial_VFs;	/* Initial VFs associated with the PF */
>>> +	u16		num_VFs;	/* Number of VFs available */
>>> +	u16		offset;		/* First VF Routing ID offset */
>>> +	u16		stride;		/* Following VF stride */
>>> +	u16		vf_device;	/* VF device ID */
>>> +	u32		pgsz;		/* Page size for BAR alignment */
>>> +	u8		link;		/* Function Dependency Link */
>>> +	u8		max_VF_buses;	/* Max buses consumed by VFs */
>>> +	u16		driver_max_VFs;	/* Max num VFs driver supports */
>>> +	struct pci_dev	*dev;		/* Lowest numbered PF */
>>> +	struct pci_dev	*self;		/* This PF */
>>> +	u32		cfg_size;	/* VF config space size */
>>> +	u32		class;		/* VF device */
>>> +	u8		hdr_type;	/* VF header type */
>>> +	u16		subsystem_vendor; /* VF subsystem vendor */
>>> +	u16		subsystem_device; /* VF subsystem device */                                                                                   
>>> +	resource_size_t	barsz[PCI_SRIOV_NUM_BARS];	/* VF BAR size */
>>> +	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
>>> +};
>>> +
>>
>> Can "struct pci_sriov" be extracted for common use? This should not be exclusive
>> for "i40e_vf migration support".
>>
> the definition of this structure is actually in driver/pci/pci.h.
> maybe removing the copy here and use below include is better?
> #include "../../../../pci/pci.h"
> 

How about moving the definition from driver/pci/pci.h into include/linux/pci.h? So
we can just include "linux/pci.h" and removing the copy here.

-- 
Thanks,
Xiang