All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: ioat: depends on !UML
@ 2021-08-09  9:24 Johannes Berg
  2021-08-09 16:02 ` Dave Jiang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Johannes Berg @ 2021-08-09  9:24 UTC (permalink / raw)
  To: Dave Jiang; +Cc: Dan Williams, dmaengine, Johannes Berg, Geert Uytterhoeven

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-09  9:24 [PATCH] dmaengine: ioat: depends on !UML Johannes Berg
@ 2021-08-09 16:02 ` Dave Jiang
  2021-08-09 17:24 ` Dan Williams
  2021-08-25 11:31 ` Vinod Koul
  2 siblings, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2021-08-09 16:02 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dan Williams, dmaengine, Johannes Berg, Geert Uytterhoeven


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-09  9:24 [PATCH] dmaengine: ioat: depends on !UML Johannes Berg
  2021-08-09 16:02 ` Dave Jiang
@ 2021-08-09 17:24 ` Dan Williams
  2021-08-09 17:26   ` Johannes Berg
  2021-08-25 11:31 ` Vinod Koul
  2 siblings, 1 reply; 10+ messages in thread
From: Dan Williams @ 2021-08-09 17:24 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dave Jiang, dmaengine, Johannes Berg, Geert Uytterhoeven

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-09 17:24 ` Dan Williams
@ 2021-08-09 17:26   ` Johannes Berg
  2021-08-25 13:31     ` Dan Williams
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2021-08-09 17:26 UTC (permalink / raw)
  To: Dan Williams; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-09  9:24 [PATCH] dmaengine: ioat: depends on !UML Johannes Berg
  2021-08-09 16:02 ` Dave Jiang
  2021-08-09 17:24 ` Dan Williams
@ 2021-08-25 11:31 ` Vinod Koul
  2 siblings, 0 replies; 10+ messages in thread
From: Vinod Koul @ 2021-08-25 11:31 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Dave Jiang, Dan Williams, dmaengine, Johannes Berg, Geert Uytterhoeven

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-09 17:26   ` Johannes Berg
@ 2021-08-25 13:31     ` Dan Williams
  2021-08-25 13:33       ` Johannes Berg
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Williams @ 2021-08-25 13:31 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-25 13:31     ` Dan Williams
@ 2021-08-25 13:33       ` Johannes Berg
  2021-08-25 13:36         ` Dan Williams
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2021-08-25 13:33 UTC (permalink / raw)
  To: Dan Williams; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-25 13:33       ` Johannes Berg
@ 2021-08-25 13:36         ` Dan Williams
  2021-08-25 13:41           ` Johannes Berg
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Williams @ 2021-08-25 13:36 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-25 13:36         ` Dan Williams
@ 2021-08-25 13:41           ` Johannes Berg
  2021-08-25 13:53             ` Dan Williams
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2021-08-25 13:41 UTC (permalink / raw)
  To: Dan Williams; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] dmaengine: ioat: depends on !UML
  2021-08-25 13:41           ` Johannes Berg
@ 2021-08-25 13:53             ` Dan Williams
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Williams @ 2021-08-25 13:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dave Jiang, dmaengine, Geert Uytterhoeven

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-08-25 13:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09  9:24 [PATCH] dmaengine: ioat: depends on !UML Johannes Berg
2021-08-09 16:02 ` Dave Jiang
2021-08-09 17:24 ` Dan Williams
2021-08-09 17:26   ` Johannes Berg
2021-08-25 13:31     ` Dan Williams
2021-08-25 13:33       ` Johannes Berg
2021-08-25 13:36         ` Dan Williams
2021-08-25 13:41           ` Johannes Berg
2021-08-25 13:53             ` Dan Williams
2021-08-25 11:31 ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.