* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:01 [PATCH v1 1/1] s390/protvirt: fix compilation issue Claudio Imbrenda
@ 2020-04-23 12:02 ` David Hildenbrand
2020-04-23 12:13 ` Cornelia Huck
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2020-04-23 12:02 UTC (permalink / raw)
To: Claudio Imbrenda, kvm
Cc: linux-kernel, linux-s390, borntraeger, frankja, gor, cohuck,
kbuild test robot, Philipp Rudo
On 23.04.20 14:01, Claudio Imbrenda wrote:
> The kernel fails to compile with CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> set but CONFIG_KVM unset.
>
> This patch fixes the issue by making the needed variable always available.
>
> Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware interface for Ultravisor information")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Philipp Rudo <prudo@linux.ibm.com>
> Suggested-by: Philipp Rudo <prudo@linux.ibm.com>
> CC: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> arch/s390/boot/uv.c | 2 --
> arch/s390/kernel/uv.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
> index 8fde561f1d07..f887a479cdc7 100644
> --- a/arch/s390/boot/uv.c
> +++ b/arch/s390/boot/uv.c
> @@ -7,9 +7,7 @@
> #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> int __bootdata_preserved(prot_virt_guest);
> #endif
> -#if IS_ENABLED(CONFIG_KVM)
> struct uv_info __bootdata_preserved(uv_info);
> -#endif
>
> void uv_query_info(void)
> {
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index c86d654351d1..4c0677fc8904 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -23,10 +23,11 @@
> int __bootdata_preserved(prot_virt_guest);
> #endif
>
> +struct uv_info __bootdata_preserved(uv_info);
> +
> #if IS_ENABLED(CONFIG_KVM)
> int prot_virt_host;
> EXPORT_SYMBOL(prot_virt_host);
> -struct uv_info __bootdata_preserved(uv_info);
> EXPORT_SYMBOL(uv_info);
>
> static int __init prot_virt_setup(char *val)
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:01 [PATCH v1 1/1] s390/protvirt: fix compilation issue Claudio Imbrenda
2020-04-23 12:02 ` David Hildenbrand
@ 2020-04-23 12:13 ` Cornelia Huck
2020-04-23 12:54 ` Vasily Gorbik
2020-04-25 8:23 ` Vasily Gorbik
3 siblings, 0 replies; 8+ messages in thread
From: Cornelia Huck @ 2020-04-23 12:13 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: kvm, linux-kernel, linux-s390, borntraeger, frankja, gor, david,
kbuild test robot, Philipp Rudo
On Thu, 23 Apr 2020 14:01:14 +0200
Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
> The kernel fails to compile with CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> set but CONFIG_KVM unset.
>
> This patch fixes the issue by making the needed variable always available.
>
> Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware interface for Ultravisor information")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Philipp Rudo <prudo@linux.ibm.com>
> Suggested-by: Philipp Rudo <prudo@linux.ibm.com>
> CC: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> arch/s390/boot/uv.c | 2 --
> arch/s390/kernel/uv.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:01 [PATCH v1 1/1] s390/protvirt: fix compilation issue Claudio Imbrenda
2020-04-23 12:02 ` David Hildenbrand
2020-04-23 12:13 ` Cornelia Huck
@ 2020-04-23 12:54 ` Vasily Gorbik
2020-04-23 14:08 ` Claudio Imbrenda
2020-04-24 13:17 ` Christian Borntraeger
2020-04-25 8:23 ` Vasily Gorbik
3 siblings, 2 replies; 8+ messages in thread
From: Vasily Gorbik @ 2020-04-23 12:54 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: kvm, linux-kernel, linux-s390, borntraeger, frankja, cohuck,
david, kbuild test robot, Philipp Rudo
On Thu, Apr 23, 2020 at 02:01:14PM +0200, Claudio Imbrenda wrote:
> The kernel fails to compile with CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> set but CONFIG_KVM unset.
>
> This patch fixes the issue by making the needed variable always available.
This statement confuses me a bit.
It's worth to mention that both arch/s390/boot/uv.c (for the
decompressor) and arch/s390/kernel/uv.c (for the main kernel) are only
built when either CONFIG_PROTECTED_VIRTUALIZATION_GUEST or
CONFIG_KVM is enabled.
Both arch/s390/boot/Makefile and arch/s390/kernel/Makefile contain:
obj-$(findstring y, $(CONFIG_PROTECTED_VIRTUALIZATION_GUEST) $(CONFIG_PGSTE)) += uv.o
So this makes the variable available when
CONFIG_PROTECTED_VIRTUALIZATION_GUEST or CONFIG_KVM (expressed via
CONFIG_PGSTE) is enabled. Hence no need for extra conditions for variable
declaration.
> Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware interface for Ultravisor information")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Philipp Rudo <prudo@linux.ibm.com>
> Suggested-by: Philipp Rudo <prudo@linux.ibm.com>
> CC: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> arch/s390/boot/uv.c | 2 --
> arch/s390/kernel/uv.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
> index 8fde561f1d07..f887a479cdc7 100644
> --- a/arch/s390/boot/uv.c
> +++ b/arch/s390/boot/uv.c
> @@ -7,9 +7,7 @@
> #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> int __bootdata_preserved(prot_virt_guest);
> #endif
> -#if IS_ENABLED(CONFIG_KVM)
> struct uv_info __bootdata_preserved(uv_info);
> -#endif
>
> void uv_query_info(void)
> {
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index c86d654351d1..4c0677fc8904 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -23,10 +23,11 @@
> int __bootdata_preserved(prot_virt_guest);
> #endif
>
> +struct uv_info __bootdata_preserved(uv_info);
> +
> #if IS_ENABLED(CONFIG_KVM)
> int prot_virt_host;
> EXPORT_SYMBOL(prot_virt_host);
> -struct uv_info __bootdata_preserved(uv_info);
> EXPORT_SYMBOL(uv_info);
hm, EXPORT_SYMBOL(uv_info) is not needed without CONFIG_KVM and this saves
1 symbol export, but I'd still made EXPORT_SYMBOL follow the declaration
immediately. Documentation/process/coding-style.rst mentions that only
for function declarations though.
Reviewed-by: Vasily Gorbik <gor@linux.ibm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:54 ` Vasily Gorbik
@ 2020-04-23 14:08 ` Claudio Imbrenda
2020-04-24 13:17 ` Christian Borntraeger
1 sibling, 0 replies; 8+ messages in thread
From: Claudio Imbrenda @ 2020-04-23 14:08 UTC (permalink / raw)
To: Vasily Gorbik
Cc: kvm, linux-kernel, linux-s390, borntraeger, frankja, cohuck,
david, kbuild test robot, Philipp Rudo
On Thu, 23 Apr 2020 14:54:22 +0200
Vasily Gorbik <gor@linux.ibm.com> wrote:
> On Thu, Apr 23, 2020 at 02:01:14PM +0200, Claudio Imbrenda wrote:
> > The kernel fails to compile with
> > CONFIG_PROTECTED_VIRTUALIZATION_GUEST set but CONFIG_KVM unset.
> >
> > This patch fixes the issue by making the needed variable always
> > available.
>
> This statement confuses me a bit.
>
> It's worth to mention that both arch/s390/boot/uv.c (for the
> decompressor) and arch/s390/kernel/uv.c (for the main kernel) are only
> built when either CONFIG_PROTECTED_VIRTUALIZATION_GUEST or
> CONFIG_KVM is enabled.
> Both arch/s390/boot/Makefile and arch/s390/kernel/Makefile contain:
> obj-$(findstring y, $(CONFIG_PROTECTED_VIRTUALIZATION_GUEST)
> $(CONFIG_PGSTE)) += uv.o
>
> So this makes the variable available when
> CONFIG_PROTECTED_VIRTUALIZATION_GUEST or CONFIG_KVM (expressed via
> CONFIG_PGSTE) is enabled. Hence no need for extra conditions for
> variable declaration.
yes, "always available" when the file is compiled at all, obviously.
in fact, there are some probably useless ifdefs in that file too, but
this patch should only address the bug
> > Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware
> > interface for Ultravisor information") Reported-by: kbuild test
> > robot <lkp@intel.com> Reported-by: Philipp Rudo
> > <prudo@linux.ibm.com> Suggested-by: Philipp Rudo
> > <prudo@linux.ibm.com> CC: Vasily Gorbik <gor@linux.ibm.com>
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> > arch/s390/boot/uv.c | 2 --
> > arch/s390/kernel/uv.c | 3 ++-
> > 2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
> > index 8fde561f1d07..f887a479cdc7 100644
> > --- a/arch/s390/boot/uv.c
> > +++ b/arch/s390/boot/uv.c
> > @@ -7,9 +7,7 @@
> > #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> > int __bootdata_preserved(prot_virt_guest);
> > #endif
> > -#if IS_ENABLED(CONFIG_KVM)
> > struct uv_info __bootdata_preserved(uv_info);
> > -#endif
> >
> > void uv_query_info(void)
> > {
> > diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> > index c86d654351d1..4c0677fc8904 100644
> > --- a/arch/s390/kernel/uv.c
> > +++ b/arch/s390/kernel/uv.c
> > @@ -23,10 +23,11 @@
> > int __bootdata_preserved(prot_virt_guest);
> > #endif
> >
> > +struct uv_info __bootdata_preserved(uv_info);
> > +
> > #if IS_ENABLED(CONFIG_KVM)
> > int prot_virt_host;
> > EXPORT_SYMBOL(prot_virt_host);
> > -struct uv_info __bootdata_preserved(uv_info);
> > EXPORT_SYMBOL(uv_info);
>
> hm, EXPORT_SYMBOL(uv_info) is not needed without CONFIG_KVM and this
> saves 1 symbol export, but I'd still made EXPORT_SYMBOL follow the
> declaration immediately. Documentation/process/coding-style.rst
> mentions that only for function declarations though.
I was wondering the same regarding the export.
I don't have any strong opinions, so if anyone has a preference, speak
up and I can fix it.
> Reviewed-by: Vasily Gorbik <gor@linux.ibm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:54 ` Vasily Gorbik
2020-04-23 14:08 ` Claudio Imbrenda
@ 2020-04-24 13:17 ` Christian Borntraeger
2020-04-25 8:22 ` Vasily Gorbik
1 sibling, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2020-04-24 13:17 UTC (permalink / raw)
To: Vasily Gorbik, Claudio Imbrenda
Cc: kvm, linux-kernel, linux-s390, frankja, cohuck, david,
kbuild test robot, Philipp Rudo
On 23.04.20 14:54, Vasily Gorbik wrote:
> On Thu, Apr 23, 2020 at 02:01:14PM +0200, Claudio Imbrenda wrote:
>> The kernel fails to compile with CONFIG_PROTECTED_VIRTUALIZATION_GUEST
>> set but CONFIG_KVM unset.
>>
>> This patch fixes the issue by making the needed variable always available.
>
> This statement confuses me a bit.
>
> It's worth to mention that both arch/s390/boot/uv.c (for the
> decompressor) and arch/s390/kernel/uv.c (for the main kernel) are only
> built when either CONFIG_PROTECTED_VIRTUALIZATION_GUEST or
> CONFIG_KVM is enabled.
> Both arch/s390/boot/Makefile and arch/s390/kernel/Makefile contain:
> obj-$(findstring y, $(CONFIG_PROTECTED_VIRTUALIZATION_GUEST) $(CONFIG_PGSTE)) += uv.o
>
> So this makes the variable available when
> CONFIG_PROTECTED_VIRTUALIZATION_GUEST or CONFIG_KVM (expressed via
> CONFIG_PGSTE) is enabled. Hence no need for extra conditions for variable
> declaration.
>
>> Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware interface for Ultravisor information")
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Reported-by: Philipp Rudo <prudo@linux.ibm.com>
>> Suggested-by: Philipp Rudo <prudo@linux.ibm.com>
>> CC: Vasily Gorbik <gor@linux.ibm.com>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> ---
>> arch/s390/boot/uv.c | 2 --
>> arch/s390/kernel/uv.c | 3 ++-
>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/s390/boot/uv.c b/arch/s390/boot/uv.c
>> index 8fde561f1d07..f887a479cdc7 100644
>> --- a/arch/s390/boot/uv.c
>> +++ b/arch/s390/boot/uv.c
>> @@ -7,9 +7,7 @@
>> #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
>> int __bootdata_preserved(prot_virt_guest);
>> #endif
>> -#if IS_ENABLED(CONFIG_KVM)
>> struct uv_info __bootdata_preserved(uv_info);
>> -#endif
>>
>> void uv_query_info(void)
>> {
>> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
>> index c86d654351d1..4c0677fc8904 100644
>> --- a/arch/s390/kernel/uv.c
>> +++ b/arch/s390/kernel/uv.c
>> @@ -23,10 +23,11 @@
>> int __bootdata_preserved(prot_virt_guest);
>> #endif
>>
>> +struct uv_info __bootdata_preserved(uv_info);
>> +
>> #if IS_ENABLED(CONFIG_KVM)
>> int prot_virt_host;
>> EXPORT_SYMBOL(prot_virt_host);
>> -struct uv_info __bootdata_preserved(uv_info);
>> EXPORT_SYMBOL(uv_info);
>
> hm, EXPORT_SYMBOL(uv_info) is not needed without CONFIG_KVM and this saves
> 1 symbol export, but I'd still made EXPORT_SYMBOL follow the declaration
> immediately. Documentation/process/coding-style.rst mentions that only
> for function declarations though.
>
> Reviewed-by: Vasily Gorbik <gor@linux.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Vasily, I guess you have a pull request soon? Do you want to pick this?
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-24 13:17 ` Christian Borntraeger
@ 2020-04-25 8:22 ` Vasily Gorbik
0 siblings, 0 replies; 8+ messages in thread
From: Vasily Gorbik @ 2020-04-25 8:22 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Claudio Imbrenda, kvm, linux-kernel, linux-s390, frankja, cohuck,
david, kbuild test robot, Philipp Rudo
On Fri, Apr 24, 2020 at 03:17:11PM +0200, Christian Borntraeger wrote:
>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Vasily, I guess you have a pull request soon? Do you want to pick this?
yes, I'll pick this up.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] s390/protvirt: fix compilation issue
2020-04-23 12:01 [PATCH v1 1/1] s390/protvirt: fix compilation issue Claudio Imbrenda
` (2 preceding siblings ...)
2020-04-23 12:54 ` Vasily Gorbik
@ 2020-04-25 8:23 ` Vasily Gorbik
3 siblings, 0 replies; 8+ messages in thread
From: Vasily Gorbik @ 2020-04-25 8:23 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: kvm, linux-kernel, linux-s390, borntraeger, frankja, cohuck,
david, kbuild test robot, Philipp Rudo
On Thu, Apr 23, 2020 at 02:01:14PM +0200, Claudio Imbrenda wrote:
> The kernel fails to compile with CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> set but CONFIG_KVM unset.
>
> This patch fixes the issue by making the needed variable always available.
>
> Fixes: a0f60f8431999bf5 ("s390/protvirt: Add sysfs firmware interface for Ultravisor information")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Philipp Rudo <prudo@linux.ibm.com>
> Suggested-by: Philipp Rudo <prudo@linux.ibm.com>
> CC: Vasily Gorbik <gor@linux.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> arch/s390/boot/uv.c | 2 --
> arch/s390/kernel/uv.c | 3 ++-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
Applied, thanks
^ permalink raw reply [flat|nested] 8+ messages in thread