Jason Gunthorpe
2023-Nov-29 00:48 UTC
[Nouveau] [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
The arm-smmu driver can COMPILE_TEST on x86, so expand this to also enable the IORT code so it can be COMPILE_TEST'd too. Signed-off-by: Jason Gunthorpe <jgg at nvidia.com> --- drivers/acpi/Kconfig | 2 -- drivers/acpi/Makefile | 2 +- drivers/acpi/arm64/Kconfig | 1 + drivers/acpi/arm64/Makefile | 2 +- drivers/iommu/Kconfig | 1 + 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index f819e760ff195a..3b7f77b227d13a 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -541,9 +541,7 @@ config ACPI_PFRUT To compile the drivers as modules, choose M here: the modules will be called pfr_update and pfr_telemetry. -if ARM64 source "drivers/acpi/arm64/Kconfig" -endif config ACPI_PPTT bool diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index eaa09bf52f1760..4e77ae37b80726 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -127,7 +127,7 @@ obj-y += pmic/ video-objs += acpi_video.o video_detect.o obj-y += dptf/ -obj-$(CONFIG_ARM64) += arm64/ +obj-y += arm64/ obj-$(CONFIG_ACPI_VIOT) += viot.o diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig index b3ed6212244c1e..537d49d8ace69e 100644 --- a/drivers/acpi/arm64/Kconfig +++ b/drivers/acpi/arm64/Kconfig @@ -11,6 +11,7 @@ config ACPI_GTDT config ACPI_AGDI bool "Arm Generic Diagnostic Dump and Reset Device Interface" + depends on ARM64 depends on ARM_SDE_INTERFACE help Arm Generic Diagnostic Dump and Reset Device Interface (AGDI) is diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile index 143debc1ba4a9d..71d0e635599390 100644 --- a/drivers/acpi/arm64/Makefile +++ b/drivers/acpi/arm64/Makefile @@ -4,4 +4,4 @@ obj-$(CONFIG_ACPI_IORT) += iort.o obj-$(CONFIG_ACPI_GTDT) += gtdt.o obj-$(CONFIG_ACPI_APMT) += apmt.o obj-$(CONFIG_ARM_AMBA) += amba.o -obj-y += dma.o init.o +obj-$(CONFIG_ARM64) += dma.o init.o diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 7673bb82945b6c..309378e76a9bc9 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -318,6 +318,7 @@ config ARM_SMMU select IOMMU_API select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU if ARM + select ACPI_IORT if ACPI help Support for implementations of the ARM System MMU architecture versions 1 and 2. -- 2.42.0
On Tue, Nov 28, 2023 at 08:48:06PM -0400, Jason Gunthorpe wrote:> The arm-smmu driver can COMPILE_TEST on x86, so expand this to also > enable the IORT code so it can be COMPILE_TEST'd too.> Signed-off-by: Jason Gunthorpe <jgg at nvidia.com> > --- > drivers/acpi/Kconfig | 2 -- > drivers/acpi/Makefile | 2 +- > drivers/acpi/arm64/Kconfig | 1 + > drivers/acpi/arm64/Makefile | 2 +- > drivers/iommu/Kconfig | 1 + > 5 files changed, 4 insertions(+), 4 deletions(-)> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index f819e760ff195a..3b7f77b227d13a 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -541,9 +541,7 @@ config ACPI_PFRUT > To compile the drivers as modules, choose M here: > the modules will be called pfr_update and pfr_telemetry.> -if ARM64 > source "drivers/acpi/arm64/Kconfig" > -endif> config ACPI_PPTT > bool > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index eaa09bf52f1760..4e77ae37b80726 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -127,7 +127,7 @@ obj-y += pmic/ > video-objs += acpi_video.o video_detect.o > obj-y += dptf/> -obj-$(CONFIG_ARM64) += arm64/ > +obj-y += arm64/> obj-$(CONFIG_ACPI_VIOT) += viot.o> diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig > index b3ed6212244c1e..537d49d8ace69e 100644 > --- a/drivers/acpi/arm64/Kconfig > +++ b/drivers/acpi/arm64/Kconfig > @@ -11,6 +11,7 @@ config ACPI_GTDT> config ACPI_AGDI > bool "Arm Generic Diagnostic Dump and Reset Device Interface" > + depends on ARM64 > depends on ARM_SDE_INTERFACE > help > Arm Generic Diagnostic Dump and Reset Device Interface (AGDI) is > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 143debc1ba4a9d..71d0e635599390 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -4,4 +4,4 @@ obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > obj-$(CONFIG_ACPI_APMT) += apmt.o > obj-$(CONFIG_ARM_AMBA) += amba.o > -obj-y += dma.o init.o > +obj-$(CONFIG_ARM64) += dma.o init.o > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 7673bb82945b6c..309378e76a9bc9 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -318,6 +318,7 @@ config ARM_SMMU > select IOMMU_API > select IOMMU_IO_PGTABLE_LPAE > select ARM_DMA_USE_IOMMU if ARM > + select ACPI_IORT if ACPI > help > Support for implementations of the ARM System MMU architecture > versions 1 and 2. > -- > 2.42.0Reviewed-by: Moritz Fischer <moritzf at google.com> Ok, now the previous patch makes sense :) Cheers, Moritz
Lorenzo Pieralisi
2023-Nov-29 12:55 UTC
[PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
On Tue, Nov 28, 2023 at 08:48:06PM -0400, Jason Gunthorpe wrote:> The arm-smmu driver can COMPILE_TEST on x86, so expand this to also > enable the IORT code so it can be COMPILE_TEST'd too. > > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com> > --- > drivers/acpi/Kconfig | 2 -- > drivers/acpi/Makefile | 2 +- > drivers/acpi/arm64/Kconfig | 1 + > drivers/acpi/arm64/Makefile | 2 +- > drivers/iommu/Kconfig | 1 + > 5 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index f819e760ff195a..3b7f77b227d13a 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -541,9 +541,7 @@ config ACPI_PFRUT > To compile the drivers as modules, choose M here: > the modules will be called pfr_update and pfr_telemetry. > > -if ARM64 > source "drivers/acpi/arm64/Kconfig" > -endif > > config ACPI_PPTT > bool > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index eaa09bf52f1760..4e77ae37b80726 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -127,7 +127,7 @@ obj-y += pmic/ > video-objs += acpi_video.o video_detect.o > obj-y += dptf/ > > -obj-$(CONFIG_ARM64) += arm64/ > +obj-y += arm64/ > > obj-$(CONFIG_ACPI_VIOT) += viot.o > > diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig > index b3ed6212244c1e..537d49d8ace69e 100644 > --- a/drivers/acpi/arm64/Kconfig > +++ b/drivers/acpi/arm64/Kconfig > @@ -11,6 +11,7 @@ config ACPI_GTDT > > config ACPI_AGDI > bool "Arm Generic Diagnostic Dump and Reset Device Interface" > + depends on ARM64 > depends on ARM_SDE_INTERFACE > help > Arm Generic Diagnostic Dump and Reset Device Interface (AGDI) is > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 143debc1ba4a9d..71d0e635599390 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -4,4 +4,4 @@ obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > obj-$(CONFIG_ACPI_APMT) += apmt.o > obj-$(CONFIG_ARM_AMBA) += amba.o > -obj-y += dma.o init.o > +obj-$(CONFIG_ARM64) += dma.o init.o > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 7673bb82945b6c..309378e76a9bc9 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -318,6 +318,7 @@ config ARM_SMMU > select IOMMU_API > select IOMMU_IO_PGTABLE_LPAE > select ARM_DMA_USE_IOMMU if ARM > + select ACPI_IORT if ACPI > help > Support for implementations of the ARM System MMU architecture > versions 1 and 2. > --I don't think it should be done this way. Is the goal compile testing IORT code ? If so, why are we forcing it through the SMMU (only because it can be compile tested while eg SMMUv3 driver can't ?) menu entry ? This looks a bit artificial (and it is unclear from the Kconfig file why only that driver selects IORT, it looks like eg the SMMUv3 does not have the same dependency - there is also the SMMUv3 perf driver to consider). Maybe we can move IORT code into drivers/acpi and add a silent config option there with a dependency on ARM64 || COMPILE_TEST. Don't know but at least it is clearer. As for the benefits of compile testing IORT code - yes the previous patch is a warning to fix but I am not so sure about the actual benefits. Thanks, Lorenzo
Jason Gunthorpe
2023-Nov-29 19:12 UTC
[Nouveau] [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
On Wed, Nov 29, 2023 at 01:55:04PM +0100, Lorenzo Pieralisi wrote:> I don't think it should be done this way. Is the goal compile testing > IORT code ?Yes> If so, why are we forcing it through the SMMU (only because > it can be compile tested while eg SMMUv3 driver can't ?) menu entry ?Because something needs to select it, and SMMU is one of the places that are implicitly using it. It isn't (and shouldn't be) a user selectable kconfig. Currently the only thing that selects it is the ARM64 master kconfig.> This looks a bit artificial (and it is unclear from the Kconfig > file why only that driver selects IORT, it looks like eg the SMMUv3 > does not have the same dependency - there is also the SMMUv3 perf > driver to consider).SMMUv3 doesn't COMPILE_TEST so it picks up the dependency transitivity through ARM64. I'm not sure why IORT was put as a global ARM64 kconfig dependency and not put in the places that directly need it. "perf driver" ? There is a bunch of GIC stuff that uses this too but I don't know if it compile tests.> Maybe we can move IORT code into drivers/acpi and add a silent config > option there with a dependency on ARM64 || COMPILE_TEST.That seems pretty weird to me, this is the right way to approach it, IMHO. Making an entire directory condition is pretty incompatible with COMPILE_TEST as a philosophy.> Don't know but at least it is clearer. As for the benefits of compile > testing IORT code - yes the previous patch is a warning to fix but > I am not so sure about the actual benefits.IMHO COMPILE_TEST is an inherently good thing. It makes development easier for everyone because you have a less fractured code base to work with. Jason
Robin Murphy
2023-Nov-30 14:10 UTC
[Nouveau] [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
On 29/11/2023 12:48 am, Jason Gunthorpe wrote:> The arm-smmu driver can COMPILE_TEST on x86, so expand this to also > enable the IORT code so it can be COMPILE_TEST'd too. > > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com> > --- > drivers/acpi/Kconfig | 2 -- > drivers/acpi/Makefile | 2 +- > drivers/acpi/arm64/Kconfig | 1 + > drivers/acpi/arm64/Makefile | 2 +- > drivers/iommu/Kconfig | 1 + > 5 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index f819e760ff195a..3b7f77b227d13a 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -541,9 +541,7 @@ config ACPI_PFRUT > To compile the drivers as modules, choose M here: > the modules will be called pfr_update and pfr_telemetry. > > -if ARM64 > source "drivers/acpi/arm64/Kconfig" > -endif > > config ACPI_PPTT > bool > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index eaa09bf52f1760..4e77ae37b80726 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -127,7 +127,7 @@ obj-y += pmic/ > video-objs += acpi_video.o video_detect.o > obj-y += dptf/ > > -obj-$(CONFIG_ARM64) += arm64/ > +obj-y += arm64/ > > obj-$(CONFIG_ACPI_VIOT) += viot.o > > diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig > index b3ed6212244c1e..537d49d8ace69e 100644 > --- a/drivers/acpi/arm64/Kconfig > +++ b/drivers/acpi/arm64/Kconfig > @@ -11,6 +11,7 @@ config ACPI_GTDT > > config ACPI_AGDI > bool "Arm Generic Diagnostic Dump and Reset Device Interface" > + depends on ARM64 > depends on ARM_SDE_INTERFACE > help > Arm Generic Diagnostic Dump and Reset Device Interface (AGDI) is > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > index 143debc1ba4a9d..71d0e635599390 100644 > --- a/drivers/acpi/arm64/Makefile > +++ b/drivers/acpi/arm64/Makefile > @@ -4,4 +4,4 @@ obj-$(CONFIG_ACPI_IORT) += iort.o > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > obj-$(CONFIG_ACPI_APMT) += apmt.o > obj-$(CONFIG_ARM_AMBA) += amba.o > -obj-y += dma.o init.o > +obj-$(CONFIG_ARM64) += dma.o init.o > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 7673bb82945b6c..309378e76a9bc9 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -318,6 +318,7 @@ config ARM_SMMU > select IOMMU_API > select IOMMU_IO_PGTABLE_LPAE > select ARM_DMA_USE_IOMMU if ARM > + select ACPI_IORT if ACPIThis is incomplete. If you want the driver to be responsible for enabling its own probing mechanisms then you need to select OF and ACPI too. And all the other drivers which probe from IORT should surely also select ACPI_IORT, and thus ACPI as well. And maybe the PCI core should as well because there are general properties of PCI host bridges and devices described in there? But of course that's clearly backwards nonsense, because drivers do not and should not do that, so this change is not appropriate either. The IORT code may not be *functionally* arm64-specific, but logically it very much is - it serves a specification which is tied to the Arm architecture and describes Arm-architecture-specific concepts, within the wider context of ACPI on Arm itself only supporting AArch64, and not AArch32. It's also not like it's driver code that someone might use as an example and copy to a similar driver which could then run on different architectures where a latent theoretical bug becomes real. There's really no practical value to be had from compile-testing IORT. Thanks, Robin.
Possibly Parallel Threads
- [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
- [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT
- [RFC PATCH v2 4/5] ACPI/IORT: Support paravirtualized IOMMU
- [RFC 08/13] ACPI/IORT: Add callback to update a device's fwnode
- [RFC 11/13] ACPI: Add VIOT table