| Message ID | 3a5bf394a9d95a28cecac996f6e0decb788c19fd.1708086092.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce generic headers | expand |
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@xen.org> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > 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. Jan
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@xen.org> >> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >> --- >> 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,
Hi, On 16/02/2024 12: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@xen.org> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > This patch can be merged with patches 4 and 5 of this patch series. I am a bit puzzled with this comment. If this is the case, then why was it done in a separate patch? I know I suggested to create the separate patch but this was only in the case you decided to handle it after this series is merged. So this should have been merged when sending. Maybe I should have been clearer. Anyway, regardless that: Reviewed-by: Julien Grall <jgrall@amazon.com> Cheers,
Hi Julien, On Mon, 2024-02-19 at 19:00 +0000, Julien Grall wrote: > Hi, > > On 16/02/2024 12: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@xen.org> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > --- > > This patch can be merged with patches 4 and 5 of this patch > > series. > > I am a bit puzzled with this comment. If this is the case, then why > was > it done in a separate patch? > > I know I suggested to create the separate patch but this was only in > the > case you decided to handle it after this series is merged. So this > should have been merged when sending. Maybe I should have been > clearer. I can submit a new version of the patch series in which this patch will be incorporated into patches 4 and 5, respectively. In this case, should all the "reviewed-by" and "acked-by" tags for patch 4 be removed? > > Anyway, regardless that: > > Reviewed-by: Julien Grall <jgrall@amazon.com> Thanks. ~ Oleksii
Hi Oleksii, On 20/02/2024 10:45, Oleksii wrote: > Hi Julien, > > On Mon, 2024-02-19 at 19:00 +0000, Julien Grall wrote: >> Hi, >> >> On 16/02/2024 12: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@xen.org> >>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>> --- >>> This patch can be merged with patches 4 and 5 of this patch >>> series. >> >> I am a bit puzzled with this comment. If this is the case, then why >> was >> it done in a separate patch? >> >> I know I suggested to create the separate patch but this was only in >> the >> case you decided to handle it after this series is merged. So this >> should have been merged when sending. Maybe I should have been >> clearer. > I can submit a new version of the patch series in which this patch will > be incorporated into patches 4 and 5, respectively. AFAICT the series is fully acked. So no need to send a new version. But please keep the request in mind for future series. Cheers,
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 I've dropped this while committing; it belongs ... > 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@xen.org> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > This patch can be merged with patches 4 and 5 of this patch series. ... somewhere here. I didn't touch patch 5's subject, but I think the duplicate "Arm" in there would better have been avoided. Jan
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) #define dev_iommu_domain(dev) (dev_archdata(dev)->domain) #define dev_iommu_group(dev) (dev_archdata(dev)->group) @@ -2777,9 +2777,9 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn, xen_domain = dom_iommu(d)->arch.priv; - if (!dev->archdata.iommu) { - dev->archdata.iommu = xzalloc(struct arm_smmu_xen_device); - if (!dev->archdata.iommu) + if (!dev->iommu) { + dev->iommu = xzalloc(struct arm_smmu_xen_device); + if (!dev->iommu) return -ENOMEM; } diff --git a/xen/include/asm-generic/device.h b/xen/include/asm-generic/device.h index f91bb7f771..1acd1ba1d8 100644 --- a/xen/include/asm-generic/device.h +++ b/xen/include/asm-generic/device.h @@ -22,12 +22,6 @@ enum device_class DEVICE_UNKNOWN, }; -struct dev_archdata { -#ifdef CONFIG_HAS_PASSTHROUGH - void *iommu; /* IOMMU private data */ -#endif -}; - /* struct device - The basic device structure */ struct device { @@ -35,8 +29,8 @@ struct device #ifdef CONFIG_HAS_DEVICE_TREE struct dt_device_node *of_node; /* Used by drivers imported from Linux */ #endif - struct dev_archdata archdata; #ifdef CONFIG_HAS_PASSTHROUGH + void *iommu; /* IOMMU private data */; struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */ #endif };
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@xen.org> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- 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(-)