Virtualization - Nov 2022 - [PATCH 0/4] ifcvf/vDPA implement features provisioning

? 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 at
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 at 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 at 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.

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
>>>>>>>
>

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 at 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 at 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 at 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
>>>>>>>>
>>
>