Re: [linux-next:master 3581/12910] arch/powerpc/kvm/../../../virt/kvm/vfio.c:89:7: error: attribute declaration must precede definition

[Nick Desaulniers <ndesaulniers@google.com>]

On Fri, Aug 25, 2023 at 12:40 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Fri, Aug 25, 2023 at 11:49:53AM -0700, Nick Desaulniers wrote:
> > I think this example demonstrates a little clearer what's going on:
> > https://godbolt.org/z/9d6scv1hE
> >
> > So based on the warning, it seems like symbol_get() can only be used
> > before the parameter it's passed is defined, otherwise the "weak" and
> > "visibility(hidden)" attributes are ignored.  These attributes are
> > merge-able upon redeclaration up until the first definition.
>
> No, that doesn't make sense. The macro does:
>
> #define symbol_get(x) ({ extern typeof(x) x __attribute__((weak,visibility("hidden"))); &(x); })
>
> x must be defined for typeof(x) to work.

x must be //declared// for typeof(x) to work, but x doesn't need to be
//defined//.
https://godbolt.org/z/qv6zPWf9K

But I think it's kind of moot, given the kconfig discussion below.

>
> I think the issue is that:
>
> # CONFIG_VFIO is not set
> CONFIG_KVM_VFIO=y
>
> And
>
> kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
>
> Which is a combination that doesn't make any sense.
>
> Looks like CONFIG_KVM_VFIO should probably be called CONFIG_KVM_ARCH_VFIO
>
> And then
>
> config KVM_VFIO
>      bool
>      depends on KVM_ARCH_VFIO
>      depends on VFIO
>
> (or similar)
>
> So we don't even attempt to compile kvm/vfio.c if we don't have VFIO
> support turned on.

Do we need to split the Kconfig?

```
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 484d0873061c..e5ebf89de855 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -61,6 +61,7 @@ config HAVE_KVM_CPU_RELAX_INTERCEPT

 config KVM_VFIO
        bool
+       depends on VFIO

 config HAVE_KVM_INVALID_WAKEUPS
        bool
```
$ ./scripts/config -e VFIO
$ make LLVM=1 ARCH=powerpc -j128 olddefconfig
arch/powerpc/kvm/../../../virt/kvm/vfio.o
#
# configuration written to .config
#
  SYNC    include/config/auto.conf.cmd
  CALL    scripts/checksyscalls.sh
  CC      arch/powerpc/kvm/../../../virt/kvm/vfio.o
$
-- 
Thanks,
~Nick Desaulniers