Re: [PATCH v9 7/7] xen/asm-generic: fold struct devarch into struct dev

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 19/02/2024 11:26, Jan Beulich wrote:
> On 16.02.2024 13:39, Oleksii Kurochko wrote:
> > The current patch is a follow-up to the patch titled:
> >      xen/asm-generic: introduce generic device.h
> > Also, a prerequisite for this patch is, without which a compilation
> > error will occur:
> >      xen/arm: switch Arm to use asm-generic/device.h
> >
> > The 'struct dev_archdata' is exclusively used within 'struct device',
> > so it could be merged into 'struct device.'
> >
> > After the merger, it is necessary to update the 'dev_archdata()'
> > macros and the comments above 'struct arm_smmu_xen_device' in
> > drivers/passthrough/arm/smmu.c.
> > Additionally, it is required to update instances of
> > "dev->archdata->iommu" to "dev->iommu".
> >
> > Suggested-by: Julien Grall <julien@xxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> >    This patch can be merged with patches 4 and 5 of this patch series.
> > ---
> > Changes in V9:
> >   - newly introduced patch.
> > ---
> >   xen/drivers/passthrough/arm/smmu.c | 12 ++++++------
> >   xen/include/asm-generic/device.h   |  8 +-------
> >   2 files changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/arm/smmu.c 
> > b/xen/drivers/passthrough/arm/smmu.c
> > index 32e2ff279b..4a272c8779 100644
> > --- a/xen/drivers/passthrough/arm/smmu.c
> > +++ b/xen/drivers/passthrough/arm/smmu.c
> > @@ -227,9 +227,9 @@ struct arm_smmu_xen_domain {
> >   };
> >   
> >   /*
> > - * Xen: Information about each device stored in dev->archdata.iommu
> > + * Xen: Information about each device stored in dev->iommu
> >    *
> > - * Initially dev->archdata.iommu only stores the iommu_domain (runtime
> > + * Initially dev->iommu only stores the iommu_domain (runtime
> >    * configuration of the SMMU) but, on Xen, we also have to store the
> >    * iommu_group (list of streamIDs associated to the device).
> >    *
> > @@ -242,7 +242,7 @@ struct arm_smmu_xen_device {
> >         struct iommu_group *group;
> >   };
> >   
> > -#define dev_archdata(dev) ((struct arm_smmu_xen_device *)dev->archdata.iommu)
> > +#define dev_archdata(dev) ((struct arm_smmu_xen_device *)dev->iommu)
> I find in particular the naming here odd. But I'll let Julien judge whether
> this really is along the lines of what he suggested.
It is. The naming is not great, but the SMMU code is intended to stay 
close to Linux. So we want to do the minimum amount of change (at least 
until we decide we should diverge).
Cheers,

--
Julien Grall