by Xin Zeng

[permalink] [raw]

Subject: RE: [PATCH v5 09/10] crypto: qat - implement interface for live migration

On Tuesday, April 30, 2024 11:10 AM, liulongfang <[email protected]> wrote:
> To: Zeng, Xin <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Tian, Kevin <[email protected]>
> Cc: [email protected]; [email protected]; qat-linux <qat-
> [email protected]>
> Subject: Re: [PATCH v5 09/10] crypto: qat - implement interface for live
> migration
>
> On 2024/3/6 21:58, Xin Zeng wrote:
> > Add logic to implement the interface for live migration defined in
> > qat/qat_mig_dev.h. This is specific for QAT GEN4 Virtual Functions
> > (VFs).
> >
> > This introduces a migration data manager which is used to handle the
> > device state during migration. The manager ensures that the device state
> > is stored in a format that can be restored in the destination node.
> >
> > The VF state is organized into a hierarchical structure that includes a
> > preamble, a general state section, a MISC bar section and an ETR bar
> > section. The latter contains the state of the 4 ring pairs contained on
> > a VF. Here is a graphical representation of the state:
> >
> > preamble | general state section | leaf state
> > | MISC bar state section| leaf state
> > | ETR bar state section | bank0 state section | leaf state
> > | bank1 state section | leaf state
> > | bank2 state section | leaf state
> > | bank3 state section | leaf state
> >
> > In addition to the implementation of the qat_migdev_ops interface and
> > the state manager framework, add a mutex in pfvf to avoid pf2vf messages
> > during migration.
> >
> > Signed-off-by: Xin Zeng <[email protected]>
> > Reviewed-by: Giovanni Cabiddu <[email protected]>
> > ---
> > .../intel/qat/qat_420xx/adf_420xx_hw_data.c | 2 +
> > .../intel/qat/qat_4xxx/adf_4xxx_hw_data.c | 2 +
> > drivers/crypto/intel/qat/qat_common/Makefile | 2 +
> > .../intel/qat/qat_common/adf_accel_devices.h | 6 +
> > .../intel/qat/qat_common/adf_gen4_hw_data.h | 10 +
> > .../intel/qat/qat_common/adf_gen4_vf_mig.c | 1010 +++++++++++++++++
> > .../intel/qat/qat_common/adf_mstate_mgr.c | 318 ++++++
> > .../intel/qat/qat_common/adf_mstate_mgr.h | 89 ++
> > .../crypto/intel/qat/qat_common/adf_sriov.c | 7 +-
> > 9 files changed, 1445 insertions(+), 1 deletion(-)
> > create mode 100644
> drivers/crypto/intel/qat/qat_common/adf_gen4_vf_mig.c
> > create mode 100644
> drivers/crypto/intel/qat/qat_common/adf_mstate_mgr.c
> > create mode 100644
> drivers/crypto/intel/qat/qat_common/adf_mstate_mgr.h
> >
> > diff --git a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> > index 9ccbf5998d5c..d255cb3ebd9c 100644
> > --- a/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> > +++ b/drivers/crypto/intel/qat/qat_420xx/adf_420xx_hw_data.c
> > @@ -17,6 +17,7 @@
> > +void adf_gen4_init_vf_mig_ops(struct qat_migdev_ops *vfmig_ops)
> > +{
> > + vfmig_ops->init = adf_gen4_vfmig_init_device;
> > + vfmig_ops->cleanup = adf_gen4_vfmig_cleanup_device;
> > + vfmig_ops->reset = adf_gen4_vfmig_reset_device;
> > + vfmig_ops->open = adf_gen4_vfmig_open_device;
> > + vfmig_ops->close = adf_gen4_vfmig_close_device;
> > + vfmig_ops->suspend = adf_gen4_vfmig_suspend_device;
> > + vfmig_ops->resume = adf_gen4_vfmig_resume_device;
> > + vfmig_ops->save_state = adf_gen4_vfmig_save_state;
> > + vfmig_ops->load_state = adf_gen4_vfmig_load_state;
> > + vfmig_ops->load_setup = adf_gen4_vfmig_load_setup;
> > + vfmig_ops->save_setup = adf_gen4_vfmig_save_setup;
> > +}
> > +EXPORT_SYMBOL_GPL(adf_gen4_init_vf_mig_ops);
>
> This GEN4 device supports live migration functionality.
> The above part of the code supports the live migration function and
> has nothing to do with crypto.
>
> Therefore, these should be moved to the vfio/pci/qat directory.
>

Thanks for the suggestion, but
1. The migration operations of QAT VF rely on QAT PF driver sitting in
crypto tree to handle. Some of the states can only be accessed from PF by
PF driver. For each generation of PF, we will have a PF driver. It is obvious
more clear and nature to make these operations part QAT PF driver rather
than the variant VF driver.
2. The interfaces are defined clear enough to understand the dependency
of the variant migration VF driver to QAT PF driver.
3. A device driver sitting in crypto tree usually not only provides crypto stuff, but
also provide helpers to support other functionalities such as the non vfio
use space process access logic, it does make sense to provide migration helpers in
device driver as well.
Those why I prefer to put these helpers into QAT PF driver.

Thanks