[PATCH] dmaengine: ioat: depends on !UML

[PATCH] dmaengine: ioat: depends on !UML

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Now that UML has PCI support, this driver must depend also on
!UML since it pokes at X86_64 architecture internals that don't
exist on ARCH=um.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/dma/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 39b5b46e880f..dc155f75926d 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -315,7 +315,7 @@ config INTEL_IDXD_PERFMON
 
 config INTEL_IOATDMA
 	tristate "Intel I/OAT DMA support"
-	depends on PCI && X86_64
+	depends on PCI && X86_64 && !UML
 	select DMA_ENGINE
 	select DMA_ENGINE_RAID
 	select DCA
-- 
2.31.1

Re: [PATCH] dmaengine: ioat: depends on !UML

[Dave Jiang]

On 8/9/2021 2:24 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Now that UML has PCI support, this driver must depend also on
> !UML since it pokes at X86_64 architecture internals that don't
> exist on ARCH=um.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/dma/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 39b5b46e880f..dc155f75926d 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -315,7 +315,7 @@ config INTEL_IDXD_PERFMON
>   
>   config INTEL_IOATDMA
>   	tristate "Intel I/OAT DMA support"
> -	depends on PCI && X86_64
> +	depends on PCI && X86_64 && !UML
>   	select DMA_ENGINE
>   	select DMA_ENGINE_RAID
>   	select DCA

Re: [PATCH] dmaengine: ioat: depends on !UML

[Dan Williams]

On Mon, Aug 9, 2021 at 2:25 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>
>
> Now that UML has PCI support, this driver must depend also on
> !UML since it pokes at X86_64 architecture internals that don't
> exist on ARCH=um.
>

Do you really need to disable compilation of the whole driver just
because an arch level helper does not exist on UML builds? Isn't there
already a check for enqcmds on x86_64 to make sure the CPU is
sufficiently feature enabled?


> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  drivers/dma/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 39b5b46e880f..dc155f75926d 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -315,7 +315,7 @@ config INTEL_IDXD_PERFMON
>
>  config INTEL_IOATDMA
>         tristate "Intel I/OAT DMA support"
> -       depends on PCI && X86_64
> +       depends on PCI && X86_64 && !UML
>         select DMA_ENGINE
>         select DMA_ENGINE_RAID
>         select DCA
> --
> 2.31.1
>

Re: [PATCH] dmaengine: ioat: depends on !UML

[Johannes Berg]

On Mon, 2021-08-09 at 10:24 -0700, Dan Williams wrote:
> On Mon, Aug 9, 2021 at 2:25 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > Now that UML has PCI support, this driver must depend also on
> > !UML since it pokes at X86_64 architecture internals that don't
> > exist on ARCH=um.
> > 
> 
> Do you really need to disable compilation of the whole driver just
> because an arch level helper does not exist on UML builds? Isn't there
> already a check for enqcmds on x86_64 to make sure the CPU is
> sufficiently feature enabled?

Hmm?

The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
UML, and that's not really surprising - ARCH=um is after all compiled to
run as a userspace process, not to run on bare metal. I guess
technically we could provide (fake or even sort of real) implementations
of these, but there's very little point?

I don't see why you would ever possibly want to have this driver
compiled on ARCH=um, even if it's compiled for x86-64 "subarch", since
there will be no such device to run against?

johannes

Re: [PATCH] dmaengine: ioat: depends on !UML

[Vinod Koul]

On 09-08-21, 11:24, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Now that UML has PCI support, this driver must depend also on
> !UML since it pokes at X86_64 architecture internals that don't
> exist on ARCH=um.

Applied, thanks

-- 
~Vinod

Re: [PATCH] dmaengine: ioat: depends on !UML

[Dan Williams]

On Mon, Aug 9, 2021 at 10:27 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Mon, 2021-08-09 at 10:24 -0700, Dan Williams wrote:
> > On Mon, Aug 9, 2021 at 2:25 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > >
> > > From: Johannes Berg <johannes.berg@intel.com>
> > >
> > > Now that UML has PCI support, this driver must depend also on
> > > !UML since it pokes at X86_64 architecture internals that don't
> > > exist on ARCH=um.
> > >
> >
> > Do you really need to disable compilation of the whole driver just
> > because an arch level helper does not exist on UML builds? Isn't there
> > already a check for enqcmds on x86_64 to make sure the CPU is
> > sufficiently feature enabled?
>
> Hmm?
>
> The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
> UML, and that's not really surprising - ARCH=um is after all compiled to
> run as a userspace process, not to run on bare metal. I guess
> technically we could provide (fake or even sort of real) implementations
> of these, but there's very little point?
>
> I don't see why you would ever possibly want to have this driver
> compiled on ARCH=um, even if it's compiled for x86-64 "subarch", since
> there will be no such device to run against?

See CONFIG_COMPILE_TEST, i.e. even the "depends on X86_64" should be
reconsidered if you ask me.

Re: [PATCH] dmaengine: ioat: depends on !UML

[Johannes Berg]

On Wed, 2021-08-25 at 06:31 -0700, Dan Williams wrote:
> On Mon, Aug 9, 2021 at 10:27 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > On Mon, 2021-08-09 at 10:24 -0700, Dan Williams wrote:
> > > On Mon, Aug 9, 2021 at 2:25 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > > > 
> > > > From: Johannes Berg <johannes.berg@intel.com>
> > > > 
> > > > Now that UML has PCI support, this driver must depend also on
> > > > !UML since it pokes at X86_64 architecture internals that don't
> > > > exist on ARCH=um.
> > > > 
> > > 
> > > Do you really need to disable compilation of the whole driver just
> > > because an arch level helper does not exist on UML builds? Isn't there
> > > already a check for enqcmds on x86_64 to make sure the CPU is
> > > sufficiently feature enabled?
> > 
> > Hmm?
> > 
> > The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
> > UML, and that's not really surprising - ARCH=um is after all compiled to
> > run as a userspace process, not to run on bare metal. I guess
> > technically we could provide (fake or even sort of real) implementations
> > of these, but there's very little point?
> > 
> > I don't see why you would ever possibly want to have this driver
> > compiled on ARCH=um, even if it's compiled for x86-64 "subarch", since
> > there will be no such device to run against?
> 
> See CONFIG_COMPILE_TEST, i.e. even the "depends on X86_64" should be
> reconsidered if you ask me.
> 
But CONFIG_COMPILE_TEST is for stuff that can *compile*, just not *work*
independent of the platform - e.g. if I have a driver that compiles
fine, but I know there's never going to be such a PCI device on non-
Intel platforms (happens a lot, after all)

But here it's the other way around - this cannot *compile* even anywhere
other than "X86_64 && !UML", let alone *work*.

johannes

Re: [PATCH] dmaengine: ioat: depends on !UML

[Dan Williams]

On Wed, Aug 25, 2021 at 6:34 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Wed, 2021-08-25 at 06:31 -0700, Dan Williams wrote:
> > On Mon, Aug 9, 2021 at 10:27 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > >
> > > On Mon, 2021-08-09 at 10:24 -0700, Dan Williams wrote:
> > > > On Mon, Aug 9, 2021 at 2:25 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > > > >
> > > > > From: Johannes Berg <johannes.berg@intel.com>
> > > > >
> > > > > Now that UML has PCI support, this driver must depend also on
> > > > > !UML since it pokes at X86_64 architecture internals that don't
> > > > > exist on ARCH=um.
> > > > >
> > > >
> > > > Do you really need to disable compilation of the whole driver just
> > > > because an arch level helper does not exist on UML builds? Isn't there
> > > > already a check for enqcmds on x86_64 to make sure the CPU is
> > > > sufficiently feature enabled?
> > >
> > > Hmm?
> > >
> > > The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
> > > UML, and that's not really surprising - ARCH=um is after all compiled to
> > > run as a userspace process, not to run on bare metal. I guess
> > > technically we could provide (fake or even sort of real) implementations
> > > of these, but there's very little point?
> > >
> > > I don't see why you would ever possibly want to have this driver
> > > compiled on ARCH=um, even if it's compiled for x86-64 "subarch", since
> > > there will be no such device to run against?
> >
> > See CONFIG_COMPILE_TEST, i.e. even the "depends on X86_64" should be
> > reconsidered if you ask me.
> >
> But CONFIG_COMPILE_TEST is for stuff that can *compile*, just not *work*
> independent of the platform - e.g. if I have a driver that compiles
> fine, but I know there's never going to be such a PCI device on non-
> Intel platforms (happens a lot, after all)
>
> But here it's the other way around - this cannot *compile* even anywhere
> other than "X86_64 && !UML", let alone *work*.

It can't compile because the arch dependencies are not stubbed out
like other arch specific helpers. I think this is something to revisit
if / when concepts similar to the "enqcmd" instruction appear on other
archs.

Re: [PATCH] dmaengine: ioat: depends on !UML

[Johannes Berg]

On Wed, 2021-08-25 at 06:36 -0700, Dan Williams wrote:
> > > > 
> > > > The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
> 
[snip]

> It can't compile because the arch dependencies are not stubbed out
> like other arch specific helpers. I think this is something to revisit
> if / when concepts similar to the "enqcmd" instruction appear on other
> archs.

I think you're confusing ioat and idxd? :)

The ioat driver (which this patch is against) uses cpuid_eax() and
cpuid_ebx() above, and really if a driver uses that then it can only
possibly run on IA :)

I'm not sure it makes sense to abstract out things such as
dca_enabled_in_bios() to a general architecture thing either?

johannes

Re: [PATCH] dmaengine: ioat: depends on !UML

[Dan Williams]

On Wed, Aug 25, 2021 at 6:41 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Wed, 2021-08-25 at 06:36 -0700, Dan Williams wrote:
> > > > >
> > > > > The problem here is that cpuid_eax() and cpuid_ebx() don't even exist on
> >
> [snip]
>
> > It can't compile because the arch dependencies are not stubbed out
> > like other arch specific helpers. I think this is something to revisit
> > if / when concepts similar to the "enqcmd" instruction appear on other
> > archs.
>
> I think you're confusing ioat and idxd? :)
>

Whoops, indeed I am, sorry for that noise.

> The ioat driver (which this patch is against) uses cpuid_eax() and
> cpuid_ebx() above, and really if a driver uses that then it can only
> possibly run on IA :)

True.

> I'm not sure it makes sense to abstract out things such as
> dca_enabled_in_bios() to a general architecture thing either?

Not general arch in this case, but a preference to keep x86'isms out
of drivers. That helper can move behind some local header ifdef'ery
and return false when needed. That said IOAT is mature, Vinod has
already taken this as is, and the commit that added it pre-dated
CONFIG_COMPILE_TEST by 6 years, so it can be forgiven.