| Message ID | 20221107093345.121648-1-lingshan.zhu@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ifcvf/vDPA implement features provisioning | expand |
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. 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/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. 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 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. 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/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. 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 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. AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add() and it seems to work? 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 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 > > 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. 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 >>>>>>
在 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. 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@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 >>>>>>>> >> >
On Thu, Nov 10, 2022 at 4:59 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote: > > > > 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? Then please reorder the patches, stable-kernel-rules.rst said: - It cannot be bigger than 100 lines, with context. Let's see. Thanks > > 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 > >>>>>>>> > >> > > >
On 11/10/2022 5:13 PM, Jason Wang wrote: > On Thu, Nov 10, 2022 at 4:59 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote: >> >> >> 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? > Then please reorder the patches, stable-kernel-rules.rst said: > > - It cannot be bigger than 100 lines, with context. > > Let's see. It is over 180 lines, so maybe re-ordering can not help here, I will try to split patch 3. Thanks, Zhu Lingshan > > Thanks > >> 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 >>>>>>>>>>