Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 11/10/2022 2:29 PM, Jason Wang wrote:
>
> 在 2022/11/10 14:20, Zhu, Lingshan 写道:
>>
>>
>> On 11/10/2022 11:49 AM, Jason Wang wrote:
>>> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan 
>>> <lingshan.zhu@intel.com> wrote:
>>>>
>>>>
>>>> On 11/9/2022 4:59 PM, Jason Wang wrote:
>>>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan 
>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>
>>>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan 
>>>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>>> This series implements features provisioning for ifcvf.
>>>>>>>> By applying this series, we allow userspace to create
>>>>>>>> a vDPA device with selected (management device supported)
>>>>>>>> feature bits and mask out others.
>>>>>>> I don't see a direct relationship between the first 3 and the last.
>>>>>>> Maybe you can state the reason why the restructure is a must for 
>>>>>>> the
>>>>>>> feature provisioning. Otherwise, we'd better split the series.
>>>>>> When introducing features provisioning ability to ifcvf, there is 
>>>>>> a need
>>>>>> to re-create vDPA devices
>>>>>> on a VF with different feature bits.
>>>>> This seems a requirement even without feature provisioning? Device
>>>>> could be deleted from the management device anyhow.
>>>> Yes, we need this to delete and re-create a vDPA device.
>>> I wonder if we need something that works for -stable.
>> I can add a fix tag, so these three patches could apply to stable
>
>
> It's too huge for -stable.
>
>
>>>
>>> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
>>> and it seems to work?
>> Yes and this is done in this series and that's why we need these
>> refactoring code.
>
>
> I meant there's probably no need to change the association of existing 
> structure but just do the allocation in dev_add(), then we will have a 
> patch with much more small changeset that fit for -stable.
Patch 1(ifcvf_base only work on ifcvf_hw) and patch 2(irq functions only 
work on ifcvf_hw) are not needed for stable.
I have already done this allocation of ifcvf_adapter which is the 
container of struct vdpa_device in dev_add() in Patch 3, this should be 
merged to stable.
Patch 3 is huge but necessary, not only allocate ifcvf_adapter in 
dev_add(), it also refactors the structures of ifcvf_mgmt_dev and 
ifcvf_adapter,
because we need to initialize the VF's hw structure ifcvf_hw(which was a 
member of ifcvf_adapter but now should be a member of ifcvf_mgmt_dev) in 
probe.

Is it still huge?

Thanks
>
> Thanks
>
>
>>
>> By the way, do you have any comments to the patches?
>>
>> Thanks,
>> Zhu Lingshan
>>>
>>> Thanks
>>>
>>>> We create vDPA device from a VF, so without features provisioning
>>>> requirements,
>>>> we don't need to re-create the vDPA device. But with features 
>>>> provisioning,
>>>> it is a must now.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> Thakns
>>>>>
>>>>>> When remove a vDPA device, the container of struct vdpa_device 
>>>>>> (here is
>>>>>> ifcvf_adapter) is free-ed in
>>>>>> dev_del() interface, so we need to allocate ifcvf_adapter in 
>>>>>> dev_add()
>>>>>> than in probe(). That's
>>>>>> why I have re-factored the adapter/mgmt_dev code.
>>>>>>
>>>>>> For re-factoring the irq related code and ifcvf_base, let them 
>>>>>> work on
>>>>>> struct ifcvf_hw, the
>>>>>> reason is that the adapter is allocated in dev_add(), if we want 
>>>>>> theses
>>>>>> functions to work
>>>>>> before dev_add(), like in probe, we need them work on ifcvf_hw 
>>>>>> than the
>>>>>> adapter.
>>>>>>
>>>>>> Thanks
>>>>>> Zhu Lingshan
>>>>>>> Thanks
>>>>>>>
>>>>>>>> Please help review
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> Zhu Lingshan (4):
>>>>>>>>      vDPA/ifcvf: ifcvf base layer interfaces work on struct 
>>>>>>>> ifcvf_hw
>>>>>>>>      vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>>>>>      vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>>>>>      vDPA/ifcvf: implement features provisioning
>>>>>>>>
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_main.c | 156 
>>>>>>>> +++++++++++++++-----------------
>>>>>>>>     3 files changed, 89 insertions(+), 109 deletions(-)
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.31.1
>>>>>>>>
>>
>