Re: [linux-next:master 5053/7963] virtio.c:undefined reference to `virtio_check_driver_offered_feature'

[Cristian Marussi <cristian.marussi@arm.com>]

On Sun, Aug 15, 2021 at 12:54:38PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   4b358aabb93a2c654cd1dcab1a25a589f6e2b153
> commit: 46abe13b5e3db187e52cd0de06c07bbce010726c [5053/7963] firmware: arm_scmi: Add virtio transport
> config: ia64-randconfig-r032-20210815 (attached as .config)
> compiler: ia64-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=46abe13b5e3db187e52cd0de06c07bbce010726c
>         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>         git fetch --no-tags linux-next master
>         git checkout 46abe13b5e3db187e52cd0de06c07bbce010726c
>         # save the attached .config to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 

Hi,

>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `virtio_chan_available':
> >> virtio.c:(.text+0x4b2): undefined reference to `virtio_check_driver_offered_feature'
>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `scmi_vio_probe':
>    virtio.c:(.text+0x622): undefined reference to `virtio_check_driver_offered_feature'
> >> ia64-linux-ld: virtio.c:(.text+0x792): undefined reference to `virtqueue_get_vring_size'
>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `scmi_vio_feed_vq_rx.isra.0':
> >> virtio.c:(.text+0xa12): undefined reference to `virtqueue_add_inbuf'
> >> ia64-linux-ld: virtio.c:(.text+0xb22): undefined reference to `virtqueue_kick'
>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `virtio_send_message':
> >> virtio.c:(.text+0x1132): undefined reference to `virtqueue_add_sgs'
>    ia64-linux-ld: virtio.c:(.text+0x1252): undefined reference to `virtqueue_kick'
>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `scmi_vio_complete_cb':
> >> virtio.c:(.text+0x1bc2): undefined reference to `virtqueue_disable_cb'
> >> ia64-linux-ld: virtio.c:(.text+0x1bd2): undefined reference to `virtqueue_get_buf'
> >> ia64-linux-ld: virtio.c:(.text+0x1e92): undefined reference to `virtqueue_enable_cb'
>    ia64-linux-ld: virtio.c:(.text+0x2182): undefined reference to `virtqueue_enable_cb'
>    ia64-linux-ld: drivers/firmware/arm_scmi/virtio.o: in function `virtio_scmi_init':
> >> virtio.c:(.init.text+0x22): undefined reference to `register_virtio_driver'
>    `virtio_scmi_exit' referenced in section `.rodata' of drivers/firmware/arm_scmi/virtio.o: defined in discarded section `.exit.text' of drivers/firmware/arm_scmi/virtio.o

This is due to the attached random config which sports:

ARM_SCMI_PROTOCOL=y
ARM_SCMI_TRANSPORT_VIRTIO=y

while

VIRTIO=m.

It is really due to how Kconfig has been restructured previously by me
in the scmi-virtio series in:

e8419c24bace firmware: arm_scmi: Make SCMI transports configurable

Since that commit, ARM_SCMI_TRANSPORT_* carry needed per-transport
dependencies (like MAILBOX/VIRTIO), but ARM_SCMI_TRANSPORT_* are "bool" not
"tristate", since they are meant to indicate if you want or not some
specific transports supports to be built into the SCMI stack identified by
ARM_SCMI_PROTOCOL: this latter, a tristate, is the only recognized entity
at built time which includes (as a builtin or as a scmi-module.ko) all
configured transports

This intermediate ARM_SCMI_TRANSPORT_* bool cause the dependency on =m to
be lost in ARM_SCMI_PROTOCOL (so you can have the SCMI stack with virtio
transport as builtin (ARM_SCMI_PROTOCOL=y) while core VIRTIO=m...and
everything breaks)

I'm testing the snippet below, which seems to solve and support all the
cases.

A similar one would be needed for ARM_SCMI_TRANSPORT_MAILBOX to be sure.

ARM_SCMI_TRANSPORT_SMC instead does not need, and should not use, this kind
of fix being based on HAVE_ARM_SMCCC_DISCOVERY.

Not sure if this is the proper way, but it's the only way I've found to
make the above configs possible (or impossible) without drowning in
circular dependencies.

If nobody shout about this I'll post 2 patches along this lines to fix
scmi virtio/mbox transport kconfig dependencies.

Thanks,
Cristian

-----8<

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index 7f4d2435503b..daa349615e91 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -4,6 +4,7 @@ menu "ARM System Control and Management Interface Protocol"
 config ARM_SCMI_PROTOCOL
        tristate "ARM System Control and Management Interface (SCMI) Message Protocol"
        depends on ARM || ARM64 || COMPILE_TEST
+       select VIRTIO if ARM_SCMI_TRANSPORT_VIRTIO
        help
          ARM System Control and Management Interface (SCMI) protocol is a
          set of operating system-independent software interfaces that are
@@ -68,7 +69,6 @@ config ARM_SCMI_TRANSPORT_SMC
 
 config ARM_SCMI_TRANSPORT_VIRTIO
        bool "SCMI transport based on VirtIO"
-       depends on VIRTIO
        select ARM_SCMI_HAVE_TRANSPORT
        select ARM_SCMI_HAVE_MSG
        help