Re: [PATCH v2 1/5] hisi_acc_vfio_pci: fix XQE dma address error

Alex Williamson
2025-01-14T13:55:01Z

On Tue, 14 Jan 2025 08:07:43 +0000
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:

> > -----Original Message-----
> > From: liulongfang <liulongfang@huawei.com>
> > Sent: Tuesday, January 14, 2025 3:18 AM
> > To: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>; jgg@nvidia.com; Jonathan
> > Cameron <jonathan.cameron@huawei.com>; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linuxarm@openeuler.org
> > Subject: Re: [PATCH v2 1/5] hisi_acc_vfio_pci: fix XQE dma address error
> >
>
> [...]
>
> > > @@ -418,7 +440,7 @@ static int vf_qm_get_match_data(struct
> > hisi_acc_vf_core_device *hisi_acc_vdev,
> > > int vf_id = hisi_acc_vdev->vf_id;
> > > int ret;
> > >
> > > - vf_data->acc_magic = ACC_DEV_MAGIC;
> > > + vf_data->acc_magic = ACC_DEV_MAGIC_V2;
> > > /* Save device id */
> > > vf_data->dev_id = hisi_acc_vdev->vf_dev->device;
> > >
> > > Thanks,
> > > Alex
> > >
> > >>>
> > >>> As for the compatibility issues between old and new versions in the
> > >>> future, we do not need to reserve version numbers to deal with them
> > >>> now. Because before encountering specific problems, our design may
> > be redundant.
> > >>
> > >> A magic value + version number would prevent the need to replace the
> > >> magic value every time an issue is encountered, which I think was
> > >> also Shameer's commit, which is not addressed by forcing the
> > >> formatting of a portion of the magic value. None of what you're
> > >> trying to do here precludes a new data structure for the new magic
> > >> value or defining the padding fields for different use cases.
> > >> Thanks,
> > >>
> > >> Alex
> > >
> >
> > If we want to use the original magic number, we also need to add the major
> > and minor version numbers. It does not cause compatibility issues and can
> > only reuse the original u64 memory space.
> >
> > This is also Shameerali's previous plan. Do you accept this plan?
>
> The suggestion here is to improve my previous plan.. ie, instead of overloading
> the V2 MAGIC with version info, introduce version(major:minor) separately.
>
> Something like,
>
> Rename old MAGIC as MAGIC_V1
>
> Introduce new MAGIC as MAGIC_V2
>
> Repurpose any padding or reserved fields in struct vf_data for major:minor
> version fields. The idea of introducing these major:minor is for future use
> where instead of coming up with a new MAGIC data every time we can
> increment the version for bug fixes and features if required.

Yes, there's no suggestion to use the original magic value, that magic
number describes a non-extensible data structure. As Shameer
indicates, use a new magic value and define within the data structure
specified by that magic value major:minor number and a scheme for
defining compatible (if any) and incompatible updates. Thanks,

Alex