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(-)
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
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 >
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
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
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
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
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
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
© 2016 - 2024 Red Hat, Inc.