1. Patchew
  2. Xen
  3. View series

[PATCH] xen: Add macro for version number string

Leo Yan posted 1 patch 3 years, 5 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220907120420.387771-1-leo.yan@linaro.org
There is a newer version of this series
xen/arch/arm/acpi/domain_build.c | 3 ++-
xen/arch/arm/domain_build.c      | 2 +-
xen/common/efi/boot.c            | 4 ++--
xen/include/xen/compile.h.in     | 1 +
4 files changed, 6 insertions(+), 4 deletions(-)
[PATCH] xen: Add macro for version number string
Posted by Leo Yan 3 years, 5 months ago 
On Arm64 Linux kernel prints log for Xen version number:

  Xen XEN_VERSION.XEN_SUBVERSION support found

The header file "xen/compile.h" is missed so that XEN_VERSION and
XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".

This patch introduces a string macro XEN_VERSION_STRING, we can directly
use it as version number string, as a result it drops to use of
__stringify() to make the code more readable.

The change has been tested on Ampere AVA Arm64 platform.

Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 xen/arch/arm/acpi/domain_build.c | 3 ++-
 xen/arch/arm/domain_build.c      | 2 +-
 xen/common/efi/boot.c            | 4 ++--
 xen/include/xen/compile.h.in     | 1 +
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/acpi/domain_build.c b/xen/arch/arm/acpi/domain_build.c
index bbdc90f92c..b23c7cad7a 100644
--- a/xen/arch/arm/acpi/domain_build.c
+++ b/xen/arch/arm/acpi/domain_build.c
@@ -9,6 +9,7 @@
  * GNU General Public License for more details.
  */
 
+#include <xen/compile.h>
 #include <xen/mm.h>
 #include <xen/sched.h>
 #include <xen/acpi.h>
@@ -91,7 +92,7 @@ static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
                                             struct membank tbl_add[])
 {
     const char compat[] =
-        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
+        "xen,xen-"XEN_VERSION_STRING"\0"
         "xen,xen";
     int res;
     /* Convenience alias */
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3fd1186b53..62602d2b86 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1367,7 +1367,7 @@ static int __init make_hypervisor_node(struct domain *d,
                                        int addrcells, int sizecells)
 {
     const char compat[] =
-        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
+        "xen,xen-"XEN_VERSION_STRING"\0"
         "xen,xen";
     __be32 *reg, *cells;
     gic_interrupt_t intr;
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index a5b2d6ddb8..db0340c8e2 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1341,8 +1341,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             efi_console_set_mode();
     }
 
-    PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION)
-             XEN_EXTRAVERSION " (c/s " XEN_CHANGESET ") EFI loader\r\n");
+    PrintStr(L"Xen " XEN_VERSION_STRING XEN_EXTRAVERSION
+	     " (c/s " XEN_CHANGESET ") EFI loader\r\n");
 
     efi_arch_relocate_image(0);
 
diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 440ecb25c1..3151d1e7d1 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -7,6 +7,7 @@
 
 #define XEN_VERSION		@@version@@
 #define XEN_SUBVERSION		@@subversion@@
+#define XEN_VERSION_STRING	"@@version@@.@@subversion@@"
 #define XEN_EXTRAVERSION	"@@extraversion@@"
 
 #define XEN_CHANGESET		"@@changeset@@"
-- 
2.34.1
Re: [PATCH] xen: Add macro for version number string
Posted by Bertrand Marquis 3 years, 5 months ago 
Hi Leo,

Thanks a lot for the quick handling here.

> On 7 Sep 2022, at 13:04, Leo Yan <leo.yan@linaro.org> wrote:
> 
> On Arm64 Linux kernel prints log for Xen version number:
> 
>  Xen XEN_VERSION.XEN_SUBVERSION support found
> 
> The header file "xen/compile.h" is missed so that XEN_VERSION and
> XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
> strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".
> 
> This patch introduces a string macro XEN_VERSION_STRING, we can directly
> use it as version number string, as a result it drops to use of
> __stringify() to make the code more readable.
> 
> The change has been tested on Ampere AVA Arm64 platform.
> 
> Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
> Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Regarding the change suggested by Jan to add spaces, I think it is a
good idea so if the commiter agrees to do on it on commit please do,
otherwise we can keep this as is.

Cheers
Bertrand

> ---
> xen/arch/arm/acpi/domain_build.c | 3 ++-
> xen/arch/arm/domain_build.c      | 2 +-
> xen/common/efi/boot.c            | 4 ++--
> xen/include/xen/compile.h.in     | 1 +
> 4 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/acpi/domain_build.c b/xen/arch/arm/acpi/domain_build.c
> index bbdc90f92c..b23c7cad7a 100644
> --- a/xen/arch/arm/acpi/domain_build.c
> +++ b/xen/arch/arm/acpi/domain_build.c
> @@ -9,6 +9,7 @@
>  * GNU General Public License for more details.
>  */
> 
> +#include <xen/compile.h>
> #include <xen/mm.h>
> #include <xen/sched.h>
> #include <xen/acpi.h>
> @@ -91,7 +92,7 @@ static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
>                                             struct membank tbl_add[])
> {
>     const char compat[] =
> -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> +        "xen,xen-"XEN_VERSION_STRING"\0"
>         "xen,xen";
>     int res;
>     /* Convenience alias */
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 3fd1186b53..62602d2b86 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1367,7 +1367,7 @@ static int __init make_hypervisor_node(struct domain *d,
>                                        int addrcells, int sizecells)
> {
>     const char compat[] =
> -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> +        "xen,xen-"XEN_VERSION_STRING"\0"
>         "xen,xen";
>     __be32 *reg, *cells;
>     gic_interrupt_t intr;
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index a5b2d6ddb8..db0340c8e2 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1341,8 +1341,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>             efi_console_set_mode();
>     }
> 
> -    PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION)
> -             XEN_EXTRAVERSION " (c/s " XEN_CHANGESET ") EFI loader\r\n");
> +    PrintStr(L"Xen " XEN_VERSION_STRING XEN_EXTRAVERSION
> +	     " (c/s " XEN_CHANGESET ") EFI loader\r\n");
> 
>     efi_arch_relocate_image(0);
> 
> diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
> index 440ecb25c1..3151d1e7d1 100644
> --- a/xen/include/xen/compile.h.in
> +++ b/xen/include/xen/compile.h.in
> @@ -7,6 +7,7 @@
> 
> #define XEN_VERSION		@@version@@
> #define XEN_SUBVERSION		@@subversion@@
> +#define XEN_VERSION_STRING	"@@version@@.@@subversion@@"
> #define XEN_EXTRAVERSION	"@@extraversion@@"
> 
> #define XEN_CHANGESET		"@@changeset@@"
> -- 
> 2.34.1
>
Re: [PATCH] xen: Add macro for version number string
Posted by Jan Beulich 3 years, 5 months ago 
On 07.09.2022 14:20, Bertrand Marquis wrote:
> Hi Leo,
> 
> Thanks a lot for the quick handling here.
> 
>> On 7 Sep 2022, at 13:04, Leo Yan <leo.yan@linaro.org> wrote:
>>
>> On Arm64 Linux kernel prints log for Xen version number:
>>
>>  Xen XEN_VERSION.XEN_SUBVERSION support found
>>
>> The header file "xen/compile.h" is missed so that XEN_VERSION and
>> XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
>> strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".
>>
>> This patch introduces a string macro XEN_VERSION_STRING, we can directly
>> use it as version number string, as a result it drops to use of
>> __stringify() to make the code more readable.
>>
>> The change has been tested on Ampere AVA Arm64 platform.
>>
>> Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
>> Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> Regarding the change suggested by Jan to add spaces, I think it is a
> good idea so if the commiter agrees to do on it on commit please do,
> otherwise we can keep this as is.

If I end up committing this, I'd be happy to add the blanks, and therefore
I'm inclined to say no need for a re-send.

Jan
Re: [PATCH] xen: Add macro for version number string
Posted by Leo Yan 3 years, 5 months ago 
On Wed, Sep 07, 2022 at 02:34:25PM +0200, Jan Beulich wrote:
> On 07.09.2022 14:20, Bertrand Marquis wrote:
> > Hi Leo,
> > 
> > Thanks a lot for the quick handling here.
> > 
> >> On 7 Sep 2022, at 13:04, Leo Yan <leo.yan@linaro.org> wrote:
> >>
> >> On Arm64 Linux kernel prints log for Xen version number:
> >>
> >>  Xen XEN_VERSION.XEN_SUBVERSION support found
> >>
> >> The header file "xen/compile.h" is missed so that XEN_VERSION and
> >> XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
> >> strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".
> >>
> >> This patch introduces a string macro XEN_VERSION_STRING, we can directly
> >> use it as version number string, as a result it drops to use of
> >> __stringify() to make the code more readable.
> >>
> >> The change has been tested on Ampere AVA Arm64 platform.
> >>
> >> Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
> >> Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
> >> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> > 
> > Regarding the change suggested by Jan to add spaces, I think it is a
> > good idea so if the commiter agrees to do on it on commit please do,
> > otherwise we can keep this as is.
> 
> If I end up committing this, I'd be happy to add the blanks, and therefore
> I'm inclined to say no need for a re-send.

For easier for maintainers, have sent patch v2 with adding blank and
adding review tags.  Thanks!

Leo
Re: [PATCH] xen: Add macro for version number string
Posted by Jan Beulich 3 years, 5 months ago 
On 07.09.2022 14:04, Leo Yan wrote:
> On Arm64 Linux kernel prints log for Xen version number:
> 
>   Xen XEN_VERSION.XEN_SUBVERSION support found
> 
> The header file "xen/compile.h" is missed so that XEN_VERSION and
> XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
> strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".
> 
> This patch introduces a string macro XEN_VERSION_STRING, we can directly
> use it as version number string, as a result it drops to use of
> __stringify() to make the code more readable.
> 
> The change has been tested on Ampere AVA Arm64 platform.
> 
> Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
> Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with perhaps a small adjustment (but it'll be the Arm maintainers to judge):

> @@ -91,7 +92,7 @@ static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
>                                              struct membank tbl_add[])
>  {
>      const char compat[] =
> -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> +        "xen,xen-"XEN_VERSION_STRING"\0"

I think readability would benefit here from adding blanks around
XEN_VERSION_STRING here and ...

> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1367,7 +1367,7 @@ static int __init make_hypervisor_node(struct domain *d,
>                                         int addrcells, int sizecells)
>  {
>      const char compat[] =
> -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> +        "xen,xen-"XEN_VERSION_STRING"\0"

... here (as an aside I wonder why these variables aren't static
__initconst), just like ...

> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1341,8 +1341,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>              efi_console_set_mode();
>      }
>  
> -    PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION)
> -             XEN_EXTRAVERSION " (c/s " XEN_CHANGESET ") EFI loader\r\n");
> +    PrintStr(L"Xen " XEN_VERSION_STRING XEN_EXTRAVERSION
> +	     " (c/s " XEN_CHANGESET ") EFI loader\r\n");

... it is here in particular for XEN_CHANGESET.

The other general remark I have: Please follow patch submission guidelines
and send To: the list with maintainers on Cc:.

Jan
Re: [PATCH] xen: Add macro for version number string
Posted by Leo Yan 3 years, 5 months ago 
Hi Jan,

On Wed, Sep 07, 2022 at 02:12:25PM +0200, Jan Beulich wrote:
> On 07.09.2022 14:04, Leo Yan wrote:
> > On Arm64 Linux kernel prints log for Xen version number:
> > 
> >   Xen XEN_VERSION.XEN_SUBVERSION support found
> > 
> > The header file "xen/compile.h" is missed so that XEN_VERSION and
> > XEN_SUBVERSION are not defined, __stringify() wrongly converts them as
> > strings and concatenate to string "XEN_VERSION.XEN_SUBVERSION".
> > 
> > This patch introduces a string macro XEN_VERSION_STRING, we can directly
> > use it as version number string, as a result it drops to use of
> > __stringify() to make the code more readable.
> > 
> > The change has been tested on Ampere AVA Arm64 platform.
> > 
> > Fixes: 5d797ee199b3 ("xen/arm: split domain_build.c")
> > Suggested-by: Bertrand Marquis <bertrand.marquis@arm.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with perhaps a small adjustment (but it'll be the Arm maintainers to judge):
> 
> > @@ -91,7 +92,7 @@ static int __init acpi_make_hypervisor_node(const struct kernel_info *kinfo,
> >                                              struct membank tbl_add[])
> >  {
> >      const char compat[] =
> > -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> > +        "xen,xen-"XEN_VERSION_STRING"\0"
> 
> I think readability would benefit here from adding blanks around
> XEN_VERSION_STRING here and ...

Agree that adding blanks is better.  Will do.

> 
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1367,7 +1367,7 @@ static int __init make_hypervisor_node(struct domain *d,
> >                                         int addrcells, int sizecells)
> >  {
> >      const char compat[] =
> > -        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> > +        "xen,xen-"XEN_VERSION_STRING"\0"
> 
> ... here (as an aside I wonder why these variables aren't static
> __initconst), just like ...

Will add blanks.

> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -1341,8 +1341,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> >              efi_console_set_mode();
> >      }
> >  
> > -    PrintStr(L"Xen " __stringify(XEN_VERSION) "." __stringify(XEN_SUBVERSION)
> > -             XEN_EXTRAVERSION " (c/s " XEN_CHANGESET ") EFI loader\r\n");
> > +    PrintStr(L"Xen " XEN_VERSION_STRING XEN_EXTRAVERSION
> > +	     " (c/s " XEN_CHANGESET ") EFI loader\r\n");
> 
> ... it is here in particular for XEN_CHANGESET.
> 
> The other general remark I have: Please follow patch submission guidelines
> and send To: the list with maintainers on Cc:.

Ah, just now quickly went through docs/process/sending-patches.pandoc,
thanks for reminding.

A question, since commit 5d797ee199b3 was merged in 4.11.0-rc6, for
fixing it, should I explictly add backport tag as below?

  Backport: 4.11+

Thanks,
Leo
Re: [PATCH] xen: Add macro for version number string
Posted by Jan Beulich 3 years, 5 months ago 
On 07.09.2022 14:28, Leo Yan wrote:
> A question, since commit 5d797ee199b3 was merged in 4.11.0-rc6, for
> fixing it, should I explictly add backport tag as below?
> 
>   Backport: 4.11+

That's up to you, I would say. We don't really use that tag all that much,
the Fixes: tag is more relevant at least based on recent observations.

Jan
Re: [PATCH] xen: Add macro for version number string
Posted by Leo Yan 3 years, 5 months ago 
On Wed, Sep 07, 2022 at 02:31:52PM +0200, Jan Beulich wrote:
> On 07.09.2022 14:28, Leo Yan wrote:
> > A question, since commit 5d797ee199b3 was merged in 4.11.0-rc6, for
> > fixing it, should I explictly add backport tag as below?
> > 
> >   Backport: 4.11+
> 
> That's up to you, I would say. We don't really use that tag all that much,
> the Fixes: tag is more relevant at least based on recent observations.

Thanks a lot for confirmation.  If so, I will just use Fixes tag.

Leo

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.