2023-12-14 18:40:04

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/2] [v2] mei: fix vsc dependency

From: Arnd Bergmann <[email protected]>

CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
but then the driver is not built because Kbuild never enters the
drivers/misc/mei directory for built-in files, leading to a link
failure:

ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko] undefined!

Add an explicit dependency on CONFIG_MEI that was apparently missing,
to ensure the VSC_HW driver cannot be built-in with MEI itself being
a loadable module.

Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/misc/mei/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 858bd701d68c..1e28ca23a74a 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -62,6 +62,7 @@ config INTEL_MEI_GSC

config INTEL_MEI_VSC_HW
tristate "Intel visual sensing controller device transport driver"
+ depends on INTEL_MEI
depends on ACPI && SPI
depends on GPIOLIB || COMPILE_TEST
help
--
2.39.2


2023-12-14 18:40:27

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/2] mei: rework Kconfig dependencies

From: Arnd Bergmann <[email protected]>

The dependencies in the mei framework are inconsistent, with some symbols
using 'select INTEL_MEI' to force it being enabled and others using
'depends on INTEL_MEI'.

In general, one should not select user-visible symbols, so change all
of these to normal dependencies, but change the default on INTEL_MEI to
be enabled when building a kernel for an Intel CPU with ME or a generic
x86 kernel.

Having consistent dependencies makes the 'menuconfig' listing more
readable by using proper indentation.

A large if/endif block is just a simpler syntax than repeating the
dependencies for each symbol.

Signed-off-by: Arnd Bergmann <[email protected]>
---
This does not fix a bug, but seems like a sensible cleanup to me,
making the logic less error-prone for future changes. Feel free
to just take the first patch and ignore this one if I missed
an important reason for the original variant, or if you prefer
a different method.
---
drivers/misc/mei/Kconfig | 14 ++++++--------
drivers/misc/mei/gsc_proxy/Kconfig | 2 +-
drivers/misc/mei/hdcp/Kconfig | 2 +-
drivers/misc/mei/pxp/Kconfig | 2 +-
4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 1e28ca23a74a..67d9391f1855 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -3,6 +3,7 @@
config INTEL_MEI
tristate "Intel Management Engine Interface"
depends on X86 && PCI
+ default GENERIC_CPU || MCORE2 || MATOM || X86_GENERIC
help
The Intel Management Engine (Intel ME) provides Manageability,
Security and Media services for system containing Intel chipsets.
@@ -11,10 +12,11 @@ config INTEL_MEI
For more information see
<https://software.intel.com/en-us/manageability/>

+if INTEL_MEI
+
config INTEL_MEI_ME
tristate "ME Enabled Intel Chipsets"
- select INTEL_MEI
- depends on X86 && PCI
+ default y
help
MEI support for ME Enabled Intel chipsets.

@@ -38,8 +40,6 @@ config INTEL_MEI_ME

config INTEL_MEI_TXE
tristate "Intel Trusted Execution Environment with ME Interface"
- select INTEL_MEI
- depends on X86 && PCI
help
MEI Support for Trusted Execution Environment device on Intel SoCs

@@ -48,9 +48,7 @@ config INTEL_MEI_TXE

config INTEL_MEI_GSC
tristate "Intel MEI GSC embedded device"
- depends on INTEL_MEI
depends on INTEL_MEI_ME
- depends on X86 && PCI
depends on DRM_I915
help
Intel auxiliary driver for GSC devices embedded in Intel graphics devices.
@@ -62,7 +60,6 @@ config INTEL_MEI_GSC

config INTEL_MEI_VSC_HW
tristate "Intel visual sensing controller device transport driver"
- depends on INTEL_MEI
depends on ACPI && SPI
depends on GPIOLIB || COMPILE_TEST
help
@@ -75,7 +72,6 @@ config INTEL_MEI_VSC_HW
config INTEL_MEI_VSC
tristate "Intel visual sensing controller device with ME interface"
depends on INTEL_MEI_VSC_HW
- depends on INTEL_MEI
help
Intel MEI over SPI driver for Intel visual sensing controller
(IVSC) device embedded in IA platform. It supports camera sharing
@@ -88,3 +84,5 @@ config INTEL_MEI_VSC
source "drivers/misc/mei/hdcp/Kconfig"
source "drivers/misc/mei/pxp/Kconfig"
source "drivers/misc/mei/gsc_proxy/Kconfig"
+
+endif
diff --git a/drivers/misc/mei/gsc_proxy/Kconfig b/drivers/misc/mei/gsc_proxy/Kconfig
index 5f68d9f3d691..ac78b9d1eccd 100644
--- a/drivers/misc/mei/gsc_proxy/Kconfig
+++ b/drivers/misc/mei/gsc_proxy/Kconfig
@@ -3,7 +3,7 @@
#
config INTEL_MEI_GSC_PROXY
tristate "Intel GSC Proxy services of ME Interface"
- select INTEL_MEI_ME
+ depends on INTEL_MEI_ME
depends on DRM_I915
help
MEI Support for GSC Proxy Services on Intel platforms.
diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
index 54e1c9526909..9be312ec798d 100644
--- a/drivers/misc/mei/hdcp/Kconfig
+++ b/drivers/misc/mei/hdcp/Kconfig
@@ -3,7 +3,7 @@
#
config INTEL_MEI_HDCP
tristate "Intel HDCP2.2 services of ME Interface"
- select INTEL_MEI_ME
+ depends on INTEL_MEI_ME
depends on DRM_I915
help
MEI Support for HDCP2.2 Services on Intel platforms.
diff --git a/drivers/misc/mei/pxp/Kconfig b/drivers/misc/mei/pxp/Kconfig
index 4029b96afc04..838eae556dd4 100644
--- a/drivers/misc/mei/pxp/Kconfig
+++ b/drivers/misc/mei/pxp/Kconfig
@@ -4,7 +4,7 @@
#
config INTEL_MEI_PXP
tristate "Intel PXP services of ME Interface"
- select INTEL_MEI_ME
+ depends on INTEL_MEI_ME
depends on DRM_I915
help
MEI Support for PXP Services on Intel platforms.
--
2.39.2

2023-12-15 00:51:59

by Wu, Wentong

[permalink] [raw]
Subject: RE: [PATCH 1/2] [v2] mei: fix vsc dependency

> From: Arnd Bergmann <[email protected]>
>
> CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
> but then the driver is not built because Kbuild never enters the drivers/misc/mei
> directory for built-in files, leading to a link
> failure:
>
> ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko]
> undefined!
>
> Add an explicit dependency on CONFIG_MEI that was apparently missing, to
> ensure the VSC_HW driver cannot be built-in with MEI itself being a loadable
> module.
>
> Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
> Signed-off-by: Arnd Bergmann <[email protected]>

Thanks

Reviewed-by: Wentong Wu <[email protected]>

> ---
> drivers/misc/mei/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> 858bd701d68c..1e28ca23a74a 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -62,6 +62,7 @@ config INTEL_MEI_GSC
>
> config INTEL_MEI_VSC_HW
> tristate "Intel visual sensing controller device transport driver"
> + depends on INTEL_MEI
> depends on ACPI && SPI
> depends on GPIOLIB || COMPILE_TEST
> help
> --
> 2.39.2


2023-12-15 08:04:21

by Sakari Ailus

[permalink] [raw]
Subject: Re: [PATCH 1/2] [v2] mei: fix vsc dependency

Hi Arnd,

On Thu, Dec 14, 2023 at 06:39:31PM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
> but then the driver is not built because Kbuild never enters the
> drivers/misc/mei directory for built-in files, leading to a link
> failure:
>
> ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko] undefined!
>
> Add an explicit dependency on CONFIG_MEI that was apparently missing,
> to ensure the VSC_HW driver cannot be built-in with MEI itself being
> a loadable module.

Well, I don't see why someone would build mei as a module can mei-vsc-hw as
builtin but the actual dependencies don't wouldn't prevent it. How about
instead changing the Makefile in the parent directory so mei directory is
always traversed?

Either way, feel free to add:

Reviewed-by: Sakari Ailus <[email protected]>

--
Regards,

Sakari Ailus

2023-12-15 08:32:45

by Wu, Wentong

[permalink] [raw]
Subject: RE: [PATCH 2/2] mei: rework Kconfig dependencies

> From: Arnd Bergmann <[email protected]> Sent: Friday, December 15, 2023 2:40 AM
>
> From: Arnd Bergmann <[email protected]>
>
> The dependencies in the mei framework are inconsistent, with some symbols
> using 'select INTEL_MEI' to force it being enabled and others using 'depends on
> INTEL_MEI'.
>
> In general, one should not select user-visible symbols, so change all of these to
> normal dependencies, but change the default on INTEL_MEI to be enabled when
> building a kernel for an Intel CPU with ME or a generic
> x86 kernel.
>
> Having consistent dependencies makes the 'menuconfig' listing more readable
> by using proper indentation.
>
> A large if/endif block is just a simpler syntax than repeating the dependencies for
> each symbol.

Yes, I agree it will make future changes more easier.

Reviewed-by: Wentong Wu <[email protected]>

>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> This does not fix a bug, but seems like a sensible cleanup to me, making the logic
> less error-prone for future changes. Feel free to just take the first patch and
> ignore this one if I missed an important reason for the original variant, or if you
> prefer a different method.
> ---
> drivers/misc/mei/Kconfig | 14 ++++++--------
> drivers/misc/mei/gsc_proxy/Kconfig | 2 +-
> drivers/misc/mei/hdcp/Kconfig | 2 +-
> drivers/misc/mei/pxp/Kconfig | 2 +-
> 4 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> 1e28ca23a74a..67d9391f1855 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -3,6 +3,7 @@
> config INTEL_MEI
> tristate "Intel Management Engine Interface"
> depends on X86 && PCI
> + default GENERIC_CPU || MCORE2 || MATOM || X86_GENERIC
> help
> The Intel Management Engine (Intel ME) provides Manageability,
> Security and Media services for system containing Intel chipsets.
> @@ -11,10 +12,11 @@ config INTEL_MEI
> For more information see
> <https://software.intel.com/en-us/manageability/>
>
> +if INTEL_MEI
> +
> config INTEL_MEI_ME
> tristate "ME Enabled Intel Chipsets"
> - select INTEL_MEI
> - depends on X86 && PCI
> + default y
> help
> MEI support for ME Enabled Intel chipsets.
>
> @@ -38,8 +40,6 @@ config INTEL_MEI_ME
>
> config INTEL_MEI_TXE
> tristate "Intel Trusted Execution Environment with ME Interface"
> - select INTEL_MEI
> - depends on X86 && PCI
> help
> MEI Support for Trusted Execution Environment device on Intel SoCs
>
> @@ -48,9 +48,7 @@ config INTEL_MEI_TXE
>
> config INTEL_MEI_GSC
> tristate "Intel MEI GSC embedded device"
> - depends on INTEL_MEI
> depends on INTEL_MEI_ME
> - depends on X86 && PCI
> depends on DRM_I915
> help
> Intel auxiliary driver for GSC devices embedded in Intel graphics devices.
> @@ -62,7 +60,6 @@ config INTEL_MEI_GSC
>
> config INTEL_MEI_VSC_HW
> tristate "Intel visual sensing controller device transport driver"
> - depends on INTEL_MEI
> depends on ACPI && SPI
> depends on GPIOLIB || COMPILE_TEST
> help
> @@ -75,7 +72,6 @@ config INTEL_MEI_VSC_HW config INTEL_MEI_VSC
> tristate "Intel visual sensing controller device with ME interface"
> depends on INTEL_MEI_VSC_HW
> - depends on INTEL_MEI
> help
> Intel MEI over SPI driver for Intel visual sensing controller
> (IVSC) device embedded in IA platform. It supports camera sharing @@
> -88,3 +84,5 @@ config INTEL_MEI_VSC source
> "drivers/misc/mei/hdcp/Kconfig"
> source "drivers/misc/mei/pxp/Kconfig"
> source "drivers/misc/mei/gsc_proxy/Kconfig"
> +
> +endif
> diff --git a/drivers/misc/mei/gsc_proxy/Kconfig
> b/drivers/misc/mei/gsc_proxy/Kconfig
> index 5f68d9f3d691..ac78b9d1eccd 100644
> --- a/drivers/misc/mei/gsc_proxy/Kconfig
> +++ b/drivers/misc/mei/gsc_proxy/Kconfig
> @@ -3,7 +3,7 @@
> #
> config INTEL_MEI_GSC_PROXY
> tristate "Intel GSC Proxy services of ME Interface"
> - select INTEL_MEI_ME
> + depends on INTEL_MEI_ME
> depends on DRM_I915
> help
> MEI Support for GSC Proxy Services on Intel platforms.
> diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
> index 54e1c9526909..9be312ec798d 100644
> --- a/drivers/misc/mei/hdcp/Kconfig
> +++ b/drivers/misc/mei/hdcp/Kconfig
> @@ -3,7 +3,7 @@
> #
> config INTEL_MEI_HDCP
> tristate "Intel HDCP2.2 services of ME Interface"
> - select INTEL_MEI_ME
> + depends on INTEL_MEI_ME
> depends on DRM_I915
> help
> MEI Support for HDCP2.2 Services on Intel platforms.
> diff --git a/drivers/misc/mei/pxp/Kconfig b/drivers/misc/mei/pxp/Kconfig index
> 4029b96afc04..838eae556dd4 100644
> --- a/drivers/misc/mei/pxp/Kconfig
> +++ b/drivers/misc/mei/pxp/Kconfig
> @@ -4,7 +4,7 @@
> #
> config INTEL_MEI_PXP
> tristate "Intel PXP services of ME Interface"
> - select INTEL_MEI_ME
> + depends on INTEL_MEI_ME
> depends on DRM_I915
> help
> MEI Support for PXP Services on Intel platforms.
> --
> 2.39.2

Thanks
Wentong