| Message ID | 20190515034253.79348-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [RESEND] ACPI/IORT: Fix build error when IOMMU_SUPPORT disabled | expand |
On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote: > drivers/acpi/arm64/iort.c: In function iort_iommu_configure: > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > ^~ > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS > undeclared (first use in this function) > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > ^~~~~~~~~~~~~~~~~~~~~~~ > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in > > If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and > IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() > to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Reported-by: Hulk Robot <hulkci@huawei.com> What's this "Hulk Robot"? Does it report things publicly? Anyway, I can pick this up if Lorenzo acks it. Will
[+Joerg, Robin] On Wed, May 15, 2019 at 11:15:34AM +0100, Will Deacon wrote: > On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote: > > drivers/acpi/arm64/iort.c: In function iort_iommu_configure: > > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > ^~ > > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS > > undeclared (first use in this function) > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > ^~~~~~~~~~~~~~~~~~~~~~~ > > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in > > > > If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and > > IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() > > to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. > > > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Cc: Will Deacon <will.deacon@arm.com> > > Reported-by: Hulk Robot <hulkci@huawei.com> > > What's this "Hulk Robot"? Does it report things publicly? AFAIK it is Huawei CI system, I had some doubts about leaving that tag for other patches I dealt with, I am not sure it is really useful (other than giving Hulk some credit). > Anyway, I can pick this up if Lorenzo acks it. Actually I think we should compile out iort_iommu_configure() for !IOMMU_SUPPORT unless I am missing something, so that we are removing this source of errors. If that does not cut it, for this patch: iommu_fwspec_set_ats_flags() should be iommu_fwspec_set_ats_flag() The commit log should be fixed (it is pretty weird to start with the build error and then explain it): "If IOMMU_SUPPORT is not enabled, struct iommu_fwspec is an empty struct and IOMMU_FWSPEC_PCI_RC_ATS is not defined, resulting in the following build error: drivers/acpi/arm64/iort.c: In function iort_iommu_configure: drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; ^~ drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS undeclared (first use in this function) dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; ^~~~~~~~~~~~~~~~~~~~~~~ drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in Add a iommu_fwspec_set_ats_flag() helper function (that is an empty stub on !IOMMU_SUPPORT) to fix it."
On 15/05/2019 13:06, Lorenzo Pieralisi wrote: > [+Joerg, Robin] > > On Wed, May 15, 2019 at 11:15:34AM +0100, Will Deacon wrote: >> On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote: >>> drivers/acpi/arm64/iort.c: In function iort_iommu_configure: >>> drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags >>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>> ^~ >>> drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS >>> undeclared (first use in this function) >>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>> ^~~~~~~~~~~~~~~~~~~~~~~ >>> drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in >>> >>> If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and >>> IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() >>> to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. >>> >>> Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> >>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >>> Cc: Will Deacon <will.deacon@arm.com> >>> Reported-by: Hulk Robot <hulkci@huawei.com> >> >> What's this "Hulk Robot"? Does it report things publicly? > > AFAIK it is Huawei CI system, I had some doubts about leaving > that tag for other patches I dealt with, I am not sure it is > really useful (other than giving Hulk some credit). > >> Anyway, I can pick this up if Lorenzo acks it. > > Actually I think we should compile out iort_iommu_configure() for > !IOMMU_SUPPORT unless I am missing something, so that we are > removing this source of errors. Indeed, much as I like the compile coverage of not having things #ifdefed out in general, this particular case seems like a fairly atypical configuration so I'm not sure all the extra fluff of struct accessors is really worth it. Robin. > If that does not cut it, for this patch: > > iommu_fwspec_set_ats_flags() should be iommu_fwspec_set_ats_flag() > > The commit log should be fixed (it is pretty weird to start with the > build error and then explain it): > > "If IOMMU_SUPPORT is not enabled, struct iommu_fwspec is an empty > struct and IOMMU_FWSPEC_PCI_RC_ATS is not defined, resulting in > the following build error: > > drivers/acpi/arm64/iort.c: In function iort_iommu_configure: > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > ^~ > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS > undeclared (first use in this function) > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > ^~~~~~~~~~~~~~~~~~~~~~~ > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in > > Add a iommu_fwspec_set_ats_flag() helper function (that is an empty > stub on !IOMMU_SUPPORT) to fix it." >
On Wed, May 15, 2019 at 01:46:28PM +0100, Robin Murphy wrote: > On 15/05/2019 13:06, Lorenzo Pieralisi wrote: > > [+Joerg, Robin] > > > > On Wed, May 15, 2019 at 11:15:34AM +0100, Will Deacon wrote: > > > On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote: > > > > drivers/acpi/arm64/iort.c: In function iort_iommu_configure: > > > > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags > > > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > > > ^~ > > > > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS > > > > undeclared (first use in this function) > > > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > > > ^~~~~~~~~~~~~~~~~~~~~~~ > > > > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in > > > > > > > > If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and > > > > IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() > > > > to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. > > > > > > > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> > > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > Cc: Will Deacon <will.deacon@arm.com> > > > > Reported-by: Hulk Robot <hulkci@huawei.com> > > > > > > What's this "Hulk Robot"? Does it report things publicly? > > > > AFAIK it is Huawei CI system, I had some doubts about leaving > > that tag for other patches I dealt with, I am not sure it is > > really useful (other than giving Hulk some credit). > > > > > Anyway, I can pick this up if Lorenzo acks it. > > > > Actually I think we should compile out iort_iommu_configure() for > > !IOMMU_SUPPORT unless I am missing something, so that we are > > removing this source of errors. > > Indeed, much as I like the compile coverage of not having things #ifdefed > out in general, this particular case seems like a fairly atypical > configuration so I'm not sure all the extra fluff of struct accessors is > really worth it. Ok, I will send a patch shortly to address this issue, if Kefeng does not object I will drop the CI bot as reporter and add him instead. Thanks, Lorenzo > > Robin. > > > If that does not cut it, for this patch: > > > > iommu_fwspec_set_ats_flags() should be iommu_fwspec_set_ats_flag() > > > > The commit log should be fixed (it is pretty weird to start with the > > build error and then explain it): > > > > "If IOMMU_SUPPORT is not enabled, struct iommu_fwspec is an empty > > struct and IOMMU_FWSPEC_PCI_RC_ATS is not defined, resulting in > > the following build error: > > > > drivers/acpi/arm64/iort.c: In function iort_iommu_configure: > > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > ^~ > > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS > > undeclared (first use in this function) > > dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; > > ^~~~~~~~~~~~~~~~~~~~~~~ > > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in > > > > Add a iommu_fwspec_set_ats_flag() helper function (that is an empty > > stub on !IOMMU_SUPPORT) to fix it." > >
On 2019/5/16 0:41, Lorenzo Pieralisi wrote: > On Wed, May 15, 2019 at 01:46:28PM +0100, Robin Murphy wrote: >> On 15/05/2019 13:06, Lorenzo Pieralisi wrote: >>> [+Joerg, Robin] >>> >>> On Wed, May 15, 2019 at 11:15:34AM +0100, Will Deacon wrote: >>>> On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote: >>>>> drivers/acpi/arm64/iort.c: In function iort_iommu_configure: >>>>> drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags >>>>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>>>> ^~ >>>>> drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS >>>>> undeclared (first use in this function) >>>>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>>>> ^~~~~~~~~~~~~~~~~~~~~~~ >>>>> drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in >>>>> >>>>> If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and >>>>> IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() >>>>> to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. >>>>> >>>>> Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> >>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >>>>> Cc: Will Deacon <will.deacon@arm.com> >>>>> Reported-by: Hulk Robot <hulkci@huawei.com> >>>> What's this "Hulk Robot"? Does it report things publicly? >>> AFAIK it is Huawei CI system, I had some doubts about leaving >>> that tag for other patches I dealt with, I am not sure it is >>> really useful (other than giving Hulk some credit). As Lorenzo said, it is Huawei CI system to improve the robustness of our inner Linux kernel version, we also use it to test the latest Linux kernel. The system is still being developed, and still need to enhance and improve. >>> >>>> Anyway, I can pick this up if Lorenzo acks it. >>> Actually I think we should compile out iort_iommu_configure() for >>> !IOMMU_SUPPORT unless I am missing something, so that we are >>> removing this source of errors. >> Indeed, much as I like the compile coverage of not having things #ifdefed >> out in general, this particular case seems like a fairly atypical >> configuration so I'm not sure all the extra fluff of struct accessors is >> really worth it. This is founded by randconfig built, it seems that no one use this kind of configuration > Ok, I will send a patch shortly to address this issue, if Kefeng > does not object I will drop the CI bot as reporter and add him > instead. Either way is ok, thanks. > > Thanks, > Lorenzo > >> Robin. >> >>> If that does not cut it, for this patch: >>> >>> iommu_fwspec_set_ats_flags() should be iommu_fwspec_set_ats_flag() >>> >>> The commit log should be fixed (it is pretty weird to start with the >>> build error and then explain it): >>> >>> "If IOMMU_SUPPORT is not enabled, struct iommu_fwspec is an empty >>> struct and IOMMU_FWSPEC_PCI_RC_ATS is not defined, resulting in >>> the following build error: >>> >>> drivers/acpi/arm64/iort.c: In function iort_iommu_configure: >>> drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags >>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>> ^~ >>> drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS >>> undeclared (first use in this function) >>> dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; >>> ^~~~~~~~~~~~~~~~~~~~~~~ >>> drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in >>> >>> Add a iommu_fwspec_set_ats_flag() helper function (that is an empty >>> stub on !IOMMU_SUPPORT) to fix it." >>> > . >
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 9058cb084b91..4ccf0dd99852 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1076,7 +1076,7 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) iort_pci_iommu_init, &info); if (!err && iort_pci_rc_supports_ats(node)) - dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; + iommu_fwspec_set_ats_flags(dev); } else { int i = 0; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index a815cf6f6f47..0a84556c0eb6 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -479,6 +479,11 @@ static inline void dev_iommu_fwspec_set(struct device *dev, dev->iommu_fwspec = fwspec; } +static inline void iommu_fwspec_set_ats_flags(struct device *dev) +{ + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; +} + int iommu_probe_device(struct device *dev); void iommu_release_device(struct device *dev); @@ -770,6 +775,10 @@ static inline void iommu_fwspec_free(struct device *dev) { } +static inline void iommu_fwspec_set_ats_flags(struct device *dev) +{ +} + static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids) {
drivers/acpi/arm64/iort.c: In function iort_iommu_configure: drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; ^~ drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS undeclared (first use in this function) dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; ^~~~~~~~~~~~~~~~~~~~~~~ drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags() to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error. Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Will Deacon <will.deacon@arm.com> Reported-by: Hulk Robot <hulkci@huawei.com> Fixes: 5702ee24182f ("ACPI/IORT: Check ATS capability in root complex nodes") Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- drivers/acpi/arm64/iort.c | 2 +- include/linux/iommu.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)