On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores),
the physical memory regions are:
DRAM memory regions:
Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff
Node[0] Region[1]: 0x080000000000 - 0x08007fffffff
Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff
The UEFI loads Xen hypervisor and DTB into the high memory, the kernel
and ramdisk images are loaded into the low memory space:
(XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen
(XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree
(XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk
(XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel
In this case, the Xen binary is loaded above 8TB, which exceeds the
maximum supported identity map space of 2TB in Xen. Consequently, the
system fails to boot.
This patch enlarges identity map space to 10TB, allowing module loading
within the range of [0x0 .. 0x000009ff_ffff_ffff].
Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable")
Reported-by: Alexey Klimov <alexey.klimov@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
xen/arch/arm/arm64/mm.c | 6 ++++--
xen/arch/arm/include/asm/mmu/layout.h | 8 ++++----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/arm64/mm.c b/xen/arch/arm/arm64/mm.c
index 78b7c7eb00..cb69df0661 100644
--- a/xen/arch/arm/arm64/mm.c
+++ b/xen/arch/arm/arm64/mm.c
@@ -41,7 +41,8 @@ static void __init prepare_boot_identity_mapping(void)
clear_page(boot_third_id);
if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
- panic("Cannot handle ID mapping above 2TB\n");
+ panic("Cannot handle ID mapping above %uTB\n",
+ IDENTITY_MAPPING_AREA_NR_L0 >> 1);
/* Link first ID table */
pte = mfn_to_xen_entry(virt_to_mfn(boot_first_id), MT_NORMAL);
@@ -74,7 +75,8 @@ static void __init prepare_runtime_identity_mapping(void)
DECLARE_OFFSETS(id_offsets, id_addr);
if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
- panic("Cannot handle ID mapping above 2TB\n");
+ panic("Cannot handle ID mapping above %uTB\n",
+ IDENTITY_MAPPING_AREA_NR_L0 >> 1);
/* Link first ID table */
pte = pte_of_xenaddr((vaddr_t)xen_first_id);
diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h
index 2cb2382fbf..eac7eef885 100644
--- a/xen/arch/arm/include/asm/mmu/layout.h
+++ b/xen/arch/arm/include/asm/mmu/layout.h
@@ -19,11 +19,11 @@
* 2G - 4G Domheap: on-demand-mapped
*
* ARM64 layout:
- * 0x0000000000000000 - 0x000001ffffffffff (2TB, L0 slots [0..3])
+ * 0x0000000000000000 - 0x000009ffffffffff (10TB, L0 slots [0..19])
*
* Reserved to identity map Xen
*
- * 0x0000020000000000 - 0x0000027fffffffff (512GB, L0 slot [4])
+ * 0x00000a0000000000 - 0x00000a7fffffffff (512GB, L0 slot [20])
* (Relative offsets)
* 0 - 2M Unmapped
* 2M - 10M Xen text, data, bss
@@ -35,7 +35,7 @@
*
* 32G - 64G Frametable: 56 bytes per page for 2TB of RAM
*
- * 0x0000028000000000 - 0x00007fffffffffff (125TB, L0 slots [5..255])
+ * 0x00000a8000000000 - 0x00007fffffffffff (512GB+117TB, L0 slots [21..255])
* Unused
*
* 0x0000800000000000 - 0x000084ffffffffff (5TB, L0 slots [256..265])
@@ -49,7 +49,7 @@
#define XEN_VIRT_START _AT(vaddr_t, MB(2))
#else
-#define IDENTITY_MAPPING_AREA_NR_L0 4
+#define IDENTITY_MAPPING_AREA_NR_L0 20
#define XEN_VM_MAPPING SLOT0(IDENTITY_MAPPING_AREA_NR_L0)
#define SLOT0_ENTRY_BITS 39
--
2.39.2
Hi, > On 13 Oct 2023, at 14:26, Leo Yan <leo.yan@linaro.org> wrote: > > On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), > the physical memory regions are: > > DRAM memory regions: > Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff > Node[0] Region[1]: 0x080000000000 - 0x08007fffffff > Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff > > The UEFI loads Xen hypervisor and DTB into the high memory, the kernel > and ramdisk images are loaded into the low memory space: > > (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen > (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree > (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk > (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel > > In this case, the Xen binary is loaded above 8TB, which exceeds the > maximum supported identity map space of 2TB in Xen. Consequently, the > system fails to boot. > > This patch enlarges identity map space to 10TB, allowing module loading > within the range of [0x0 .. 0x000009ff_ffff_ffff]. > > Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") I agree with Michal here, this is not a fix so this should be removed (can be done on commit). > Reported-by: Alexey Klimov <alexey.klimov@linaro.org> > Signed-off-by: Leo Yan <leo.yan@linaro.org> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > xen/arch/arm/arm64/mm.c | 6 ++++-- > xen/arch/arm/include/asm/mmu/layout.h | 8 ++++---- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/arm64/mm.c b/xen/arch/arm/arm64/mm.c > index 78b7c7eb00..cb69df0661 100644 > --- a/xen/arch/arm/arm64/mm.c > +++ b/xen/arch/arm/arm64/mm.c > @@ -41,7 +41,8 @@ static void __init prepare_boot_identity_mapping(void) > clear_page(boot_third_id); > > if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 ) > - panic("Cannot handle ID mapping above 2TB\n"); > + panic("Cannot handle ID mapping above %uTB\n", > + IDENTITY_MAPPING_AREA_NR_L0 >> 1); > > /* Link first ID table */ > pte = mfn_to_xen_entry(virt_to_mfn(boot_first_id), MT_NORMAL); > @@ -74,7 +75,8 @@ static void __init prepare_runtime_identity_mapping(void) > DECLARE_OFFSETS(id_offsets, id_addr); > > if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 ) > - panic("Cannot handle ID mapping above 2TB\n"); > + panic("Cannot handle ID mapping above %uTB\n", > + IDENTITY_MAPPING_AREA_NR_L0 >> 1); > > /* Link first ID table */ > pte = pte_of_xenaddr((vaddr_t)xen_first_id); > diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h > index 2cb2382fbf..eac7eef885 100644 > --- a/xen/arch/arm/include/asm/mmu/layout.h > +++ b/xen/arch/arm/include/asm/mmu/layout.h > @@ -19,11 +19,11 @@ > * 2G - 4G Domheap: on-demand-mapped > * > * ARM64 layout: > - * 0x0000000000000000 - 0x000001ffffffffff (2TB, L0 slots [0..3]) > + * 0x0000000000000000 - 0x000009ffffffffff (10TB, L0 slots [0..19]) > * > * Reserved to identity map Xen > * > - * 0x0000020000000000 - 0x0000027fffffffff (512GB, L0 slot [4]) > + * 0x00000a0000000000 - 0x00000a7fffffffff (512GB, L0 slot [20]) > * (Relative offsets) > * 0 - 2M Unmapped > * 2M - 10M Xen text, data, bss > @@ -35,7 +35,7 @@ > * > * 32G - 64G Frametable: 56 bytes per page for 2TB of RAM > * > - * 0x0000028000000000 - 0x00007fffffffffff (125TB, L0 slots [5..255]) > + * 0x00000a8000000000 - 0x00007fffffffffff (512GB+117TB, L0 slots [21..255]) > * Unused > * > * 0x0000800000000000 - 0x000084ffffffffff (5TB, L0 slots [256..265]) > @@ -49,7 +49,7 @@ > #define XEN_VIRT_START _AT(vaddr_t, MB(2)) > #else > > -#define IDENTITY_MAPPING_AREA_NR_L0 4 > +#define IDENTITY_MAPPING_AREA_NR_L0 20 > #define XEN_VM_MAPPING SLOT0(IDENTITY_MAPPING_AREA_NR_L0) > > #define SLOT0_ENTRY_BITS 39 > -- > 2.39.2 >
On Mon, Oct 16, 2023 at 01:40:26PM +0000, Bertrand Marquis wrote: [...] > > This patch enlarges identity map space to 10TB, allowing module loading > > within the range of [0x0 .. 0x000009ff_ffff_ffff]. > > > > Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") > > I agree with Michal here, this is not a fix so this should be removed (can be done > on commit). This is fine for me. I'd like to confirm with maintainers that should I spin a new patch set to remove the fix tag? Or maintainers could help to remove it when pick up this patch set. And thanks for review, Michal and Bertrand. Leo > > Reported-by: Alexey Klimov <alexey.klimov@linaro.org> > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> > > Cheers > Bertrand
Hi Leo, On 16/10/2023 14:54, Leo Yan wrote: > On Mon, Oct 16, 2023 at 01:40:26PM +0000, Bertrand Marquis wrote: > > [...] > >>> This patch enlarges identity map space to 10TB, allowing module loading >>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>> >>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >> >> I agree with Michal here, this is not a fix so this should be removed (can be done >> on commit). > > This is fine for me. > > I'd like to confirm with maintainers that should I spin a new patch > set to remove the fix tag? Or maintainers could help to remove it > when pick up this patch set. I can do small changes while committing (updating the fixes tag is one). That said, we are in the middle of the code freeze for Xen 4.18. Any patch requires a release-ack from the release manager (Henry). I am a bit split at the moment whether we want this patch for Xen 4.18. So I have asked opinions from Henry (others can also provide some). If it doesn't go for Xen 4.18, then Stefano might be able to queue it in his for-4.19. Otherwise, it will be picked up when the tree re-open hopefully in a couple of weeks time. Cheers, > > And thanks for review, Michal and Bertrand. > > Leo > >>> Reported-by: Alexey Klimov <alexey.klimov@linaro.org> >>> Signed-off-by: Leo Yan <leo.yan@linaro.org> >> >> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> >> >> Cheers >> Bertrand -- Julien Grall
Hi, On 13/10/2023 14:26, Leo Yan wrote: > > > On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), > the physical memory regions are: > > DRAM memory regions: > Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff > Node[0] Region[1]: 0x080000000000 - 0x08007fffffff > Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff > > The UEFI loads Xen hypervisor and DTB into the high memory, the kernel > and ramdisk images are loaded into the low memory space: > > (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen > (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree > (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk > (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel > > In this case, the Xen binary is loaded above 8TB, which exceeds the > maximum supported identity map space of 2TB in Xen. Consequently, the > system fails to boot. > > This patch enlarges identity map space to 10TB, allowing module loading > within the range of [0x0 .. 0x000009ff_ffff_ffff]. > > Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") I don't think a fixes tag applies here given that 2TB was just a number we believed is enough and all of this is platform dependent. This can be dropped on commit if committer agrees. > Reported-by: Alexey Klimov <alexey.klimov@linaro.org> > Signed-off-by: Leo Yan <leo.yan@linaro.org> Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
On 16/10/2023 09:44, Michal Orzel wrote: > Hi, Hi, > On 13/10/2023 14:26, Leo Yan wrote: >> >> >> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >> the physical memory regions are: >> >> DRAM memory regions: >> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >> >> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >> and ramdisk images are loaded into the low memory space: >> >> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >> >> In this case, the Xen binary is loaded above 8TB, which exceeds the >> maximum supported identity map space of 2TB in Xen. Consequently, the >> system fails to boot. >> >> This patch enlarges identity map space to 10TB, allowing module loading >> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >> >> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") > I don't think a fixes tag applies here given that 2TB was just a number we believed is enough > and all of this is platform dependent. > This can be dropped on commit if committer agrees Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be 1c78d76b67e1 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping"). We would also need to consider it as a candidate for Xen 4.18 because we would regress boot on ADLink. Cheers, -- Julien Grall
On Mon, 16 Oct 2023 at 14:54, Julien Grall <julien@xen.org> wrote: > > > > On 16/10/2023 09:44, Michal Orzel wrote: > > Hi, > > Hi, > > > On 13/10/2023 14:26, Leo Yan wrote: > >> > >> > >> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), > >> the physical memory regions are: > >> > >> DRAM memory regions: > >> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff > >> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff > >> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff > >> > >> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel > >> and ramdisk images are loaded into the low memory space: > >> > >> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen > >> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree > >> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk > >> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel > >> > >> In this case, the Xen binary is loaded above 8TB, which exceeds the > >> maximum supported identity map space of 2TB in Xen. Consequently, the > >> system fails to boot. > >> > >> This patch enlarges identity map space to 10TB, allowing module loading > >> within the range of [0x0 .. 0x000009ff_ffff_ffff]. > >> > >> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") > > I don't think a fixes tag applies here given that 2TB was just a number we believed is enough > > and all of this is platform dependent. > > This can be dropped on commit if committer agrees > Xen may have booted on that platform before hand. So this would be > considered a regression and therefore a tag would be warrant. > > AFAICT, the commit is only present on the upcoming 4.18. So the question > is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is > yes, then we need to add a Fixes tag. But the correct one would be Yes. The upstream xen 4.17 booted fine and the mentioned commit was found during bisect, so it is introduced regression. I'd personally say that "Fixes" tag is needed here. Thanks, Alexey
Hi +Luca and Rahul > On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: > > > > On 16/10/2023 09:44, Michal Orzel wrote: >> Hi, > > Hi, > >> On 13/10/2023 14:26, Leo Yan wrote: >>> >>> >>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>> the physical memory regions are: >>> >>> DRAM memory regions: >>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>> >>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>> and ramdisk images are loaded into the low memory space: >>> >>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>> >>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>> maximum supported identity map space of 2TB in Xen. Consequently, the >>> system fails to boot. >>> >>> This patch enlarges identity map space to 10TB, allowing module loading >>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>> >>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >> and all of this is platform dependent. >> This can be dropped on commit if committer agrees > Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. > > AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be > @Rahul or Luca: could you give an answer here ? I know you used Xen on an AVA platform but was it booting out of the box ? > 1c78d76b67e1 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping"). > > We would also need to consider it as a candidate for Xen 4.18 because we would regress boot on ADLink. Ack Cheers Bertrand > > Cheers, > > -- > Julien Grall
> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: > > Hi > > +Luca and Rahul > >> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >> >> >> >> On 16/10/2023 09:44, Michal Orzel wrote: >>> Hi, >> >> Hi, >> >>> On 13/10/2023 14:26, Leo Yan wrote: >>>> >>>> >>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>> the physical memory regions are: >>>> >>>> DRAM memory regions: >>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>> >>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>> and ramdisk images are loaded into the low memory space: >>>> >>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>> >>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>> system fails to boot. >>>> >>>> This patch enlarges identity map space to 10TB, allowing module loading >>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>> >>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>> and all of this is platform dependent. >>> This can be dropped on commit if committer agrees >> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >> >> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >> > > @Rahul or Luca: could you give an answer here ? > I know you used Xen on an AVA platform but was it booting out of the box ? I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 (docs/misra: add deviations.rst to document additional deviations.) We are not applying any patch for it to run on AVA. > >> 1c78d76b67e1 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping"). >> >> We would also need to consider it as a candidate for Xen 4.18 because we would regress boot on ADLink. > > Ack > > Cheers > Bertrand > >> >> Cheers, >> >> -- >> Julien Grall > >
On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: > > > > > On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: > > > > Hi > > > > +Luca and Rahul > > > >> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: > >> > >> > >> > >> On 16/10/2023 09:44, Michal Orzel wrote: > >>> Hi, > >> > >> Hi, > >> > >>> On 13/10/2023 14:26, Leo Yan wrote: > >>>> > >>>> > >>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), > >>>> the physical memory regions are: > >>>> > >>>> DRAM memory regions: > >>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff > >>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff > >>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff > >>>> > >>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel > >>>> and ramdisk images are loaded into the low memory space: > >>>> > >>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen > >>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree > >>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk > >>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel > >>>> > >>>> In this case, the Xen binary is loaded above 8TB, which exceeds the > >>>> maximum supported identity map space of 2TB in Xen. Consequently, the > >>>> system fails to boot. > >>>> > >>>> This patch enlarges identity map space to 10TB, allowing module loading > >>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. > >>>> > >>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") > >>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough > >>> and all of this is platform dependent. > >>> This can be dropped on commit if committer agrees > >> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. > >> > >> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be > >> > > > > @Rahul or Luca: could you give an answer here ? > > I know you used Xen on an AVA platform but was it booting out of the box ? > > I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 > (docs/misra: add deviations.rst to document additional deviations.) > > We are not applying any patch for it to run on AVA. Most likely it is because your UEFI/BIOS firmware is 2.x, for instance 2.04.100.07. This fix if for AVA machine with older UEFI firmware 1.07.300.03. Best regards, Alexey
CC Henry Hi Alexey, On 16/10/2023 15:24, Alexey Klimov wrote: > On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >> >> >> >>> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >>> >>> Hi >>> >>> +Luca and Rahul >>> >>>> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >>>> >>>> >>>> >>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>> Hi, >>>> >>>> Hi, >>>> >>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>> >>>>>> >>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>> the physical memory regions are: >>>>>> >>>>>> DRAM memory regions: >>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>> >>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>> and ramdisk images are loaded into the low memory space: >>>>>> >>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>> >>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>> system fails to boot. >>>>>> >>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>> >>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>> and all of this is platform dependent. >>>>> This can be dropped on commit if committer agrees >>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>> >>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>> >>> >>> @Rahul or Luca: could you give an answer here ? >>> I know you used Xen on an AVA platform but was it booting out of the box ? >> >> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >> (docs/misra: add deviations.rst to document additional deviations.) >> >> We are not applying any patch for it to run on AVA. > > Most likely it is because your UEFI/BIOS firmware is 2.x, for instance > 2.04.100.07. > This fix if for AVA machine with older UEFI firmware 1.07.300.03. OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). Henry (as the release manager) and others, any opinions? Cheers, -- Julien Grall
Hi Julien, > On Oct 16, 2023, at 22:46, Julien Grall <julien@xen.org> wrote: > > CC Henry Thanks. > Hi Alexey, > > On 16/10/2023 15:24, Alexey Klimov wrote: >> On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote >>> >>>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>>> >>>>>>> >>>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>>> the physical memory regions are: >>>>>>> >>>>>>> DRAM memory regions: >>>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>>> >>>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>>> and ramdisk images are loaded into the low memory space: >>>>>>> >>>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>>> >>>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>>> system fails to boot. >>>>>>> >>>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>>> >>>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>>> and all of this is platform dependent. >>>>>> This can be dropped on commit if committer agrees >>>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>>> >>>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>>> >>>> >>>> @Rahul or Luca: could you give an answer here ? >>>> I know you used Xen on an AVA platform but was it booting out of the box ? >>> >>> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >>> (docs/misra: add deviations.rst to document additional deviations.) >>> >>> We are not applying any patch for it to run on AVA. >> Most likely it is because your UEFI/BIOS firmware is 2.x, for instance >> 2.04.100.07. >> This fix if for AVA machine with older UEFI firmware 1.07.300.03. > > OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. Not sure what is the reason from Alexey’s side, but from my experience of updating AVA firmware, it is sometimes tricky as I remember I did 5 boards using the same set of upgrade instructions but one board was bricked magically during this progress (Funny that the reason why we upgrade the firmware was to fix this issue…). Once this happens, the only method to recover is to have a dediprog or something similar to program the flash. If there is no handy tools, then there would be a problem…So I understand people may don’t really want to do the upgrading, but indeed I also recommend to upgrade the firmware. > > With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). > > Henry (as the release manager) and others, any opinions? I tested this series on top of today’s staging in our CI and it seems that this series is not breaking any boards/emulators we used for testing, so: Tested-by: Henry Wang <Henry.Wang@arm.com> I saw you and Bertrand had a slightly different opinion on this and probably we need Stefano’s input too, Any ideas Stefano? But I personally won’t block this patch, so if the discussion is settled to merge this to 4.18, feel free: Release-acked-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry > > Cheers, > > -- > Julien Grall
Hi, > On 16 Oct 2023, at 16:46, Julien Grall <julien@xen.org> wrote: > > CC Henry > > Hi Alexey, > > On 16/10/2023 15:24, Alexey Klimov wrote: >> On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>> >>> >>> >>>> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >>>> >>>> Hi >>>> >>>> +Luca and Rahul >>>> >>>>> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >>>>> >>>>> >>>>> >>>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>>> Hi, >>>>> >>>>> Hi, >>>>> >>>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>>> >>>>>>> >>>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>>> the physical memory regions are: >>>>>>> >>>>>>> DRAM memory regions: >>>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>>> >>>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>>> and ramdisk images are loaded into the low memory space: >>>>>>> >>>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>>> >>>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>>> system fails to boot. >>>>>>> >>>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>>> >>>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>>> and all of this is platform dependent. >>>>>> This can be dropped on commit if committer agrees >>>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>>> >>>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>>> >>>> >>>> @Rahul or Luca: could you give an answer here ? >>>> I know you used Xen on an AVA platform but was it booting out of the box ? >>> >>> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >>> (docs/misra: add deviations.rst to document additional deviations.) >>> >>> We are not applying any patch for it to run on AVA. >> Most likely it is because your UEFI/BIOS firmware is 2.x, for instance >> 2.04.100.07. >> This fix if for AVA machine with older UEFI firmware 1.07.300.03. > > OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. > > With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). > > Henry (as the release manager) and others, any opinions? With the new information, I think it should be stated that it is fixing an issue on AVA platforms using an old firmware and platforms with and up-to-date firmware are not impacted. It is an important information to keep in mind that this is not a fix to be backported for example (and for me the fixes tag should not be kept). On whether or not it should go in the release, I am not quite sure that making a change in the layout at that stage is a good idea unless it is fixing a critical issue (which is not the case here). So i would vote no but not go against if someone argue to have it in. Cheers Bertrand > > Cheers, > > -- > Julien Grall
On 16/10/2023 16:07, Bertrand Marquis wrote: > Hi, > >> On 16 Oct 2023, at 16:46, Julien Grall <julien@xen.org> wrote: >> >> CC Henry >> >> Hi Alexey, >> >> On 16/10/2023 15:24, Alexey Klimov wrote: >>> On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>>> >>>> >>>> >>>>> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >>>>> >>>>> Hi >>>>> >>>>> +Luca and Rahul >>>>> >>>>>> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>>>> Hi, >>>>>> >>>>>> Hi, >>>>>> >>>>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>>>> >>>>>>>> >>>>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>>>> the physical memory regions are: >>>>>>>> >>>>>>>> DRAM memory regions: >>>>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>>>> >>>>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>>>> and ramdisk images are loaded into the low memory space: >>>>>>>> >>>>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>>>> >>>>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>>>> system fails to boot. >>>>>>>> >>>>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>>>> >>>>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>>>> and all of this is platform dependent. >>>>>>> This can be dropped on commit if committer agrees >>>>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>>>> >>>>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>>>> >>>>> >>>>> @Rahul or Luca: could you give an answer here ? >>>>> I know you used Xen on an AVA platform but was it booting out of the box ? >>>> >>>> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >>>> (docs/misra: add deviations.rst to document additional deviations.) >>>> >>>> We are not applying any patch for it to run on AVA. >>> Most likely it is because your UEFI/BIOS firmware is 2.x, for instance >>> 2.04.100.07. >>> This fix if for AVA machine with older UEFI firmware 1.07.300.03. >> >> OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. >> >> With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). >> >> Henry (as the release manager) and others, any opinions? > > With the new information, I think it should be stated that it is fixing an issue on AVA platforms using an old firmware and platforms with and up-to-date firmware are not impacted. > It is an important information to keep in mind that this is not a fix to be backported for example (and for me the fixes tag should not be kept). IMHO, the fixes tag should not be based solely on the AVA platform. It should be based on whether this could sensibly affect others. Right now, there is nothing that would indicate either way. And therefore a Fixes tag is sensible. This doesn't mean I would want to backport it right now (note that only 4.18 is affected). But this could change in the future if we get another report (post-4.18) on a platform where there are no other workaround. Stefano any opinions? > > On whether or not it should go in the release, I am not quite sure that making a change in the layout at that stage is a good idea unless it is fixing a critical issue (which is not the case here). > So i would vote no but not go against if someone argue to have it in. I agree with your statement with the information we have today. But it could become a critical issue if another platform is hit by the same issue and have no other workaround. Cheers, -- Julien Grall
On Mon, 15 Oct 2023, Julien Grall wrote: > On 16/10/2023 16:07, Bertrand Marquis wrote: > > > On 16 Oct 2023, at 16:46, Julien Grall <julien@xen.org> wrote: > > > CC Henry > > > > > > Hi Alexey, > > > > > > On 16/10/2023 15:24, Alexey Klimov wrote: > > > > On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> > > > > wrote: > > > > > > > > > > > > > > > > > > > > > On 16 Oct 2023, at 15:00, Bertrand Marquis > > > > > > <Bertrand.Marquis@arm.com> wrote: > > > > > > > > > > > > Hi > > > > > > > > > > > > +Luca and Rahul > > > > > > > > > > > > > On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 16/10/2023 09:44, Michal Orzel wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > On 13/10/2023 14:26, Leo Yan wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse > > > > > > > > > N1 cores), > > > > > > > > > the physical memory regions are: > > > > > > > > > > > > > > > > > > DRAM memory regions: > > > > > > > > > Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff > > > > > > > > > Node[0] Region[1]: 0x080000000000 - 0x08007fffffff > > > > > > > > > Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff > > > > > > > > > > > > > > > > > > The UEFI loads Xen hypervisor and DTB into the high memory, > > > > > > > > > the kernel > > > > > > > > > and ramdisk images are loaded into the low memory space: > > > > > > > > > > > > > > > > > > (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen > > > > > > > > > (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device > > > > > > > > > Tree > > > > > > > > > (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk > > > > > > > > > (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel > > > > > > > > > > > > > > > > > > In this case, the Xen binary is loaded above 8TB, which > > > > > > > > > exceeds the > > > > > > > > > maximum supported identity map space of 2TB in Xen. > > > > > > > > > Consequently, the > > > > > > > > > system fails to boot. > > > > > > > > > > > > > > > > > > This patch enlarges identity map space to 10TB, allowing > > > > > > > > > module loading > > > > > > > > > within the range of [0x0 .. 0x000009ff_ffff_ffff]. > > > > > > > > > > > > > > > > > > Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to > > > > > > > > > prepare/enable/disable") > > > > > > > > I don't think a fixes tag applies here given that 2TB was just a > > > > > > > > number we believed is enough > > > > > > > > and all of this is platform dependent. > > > > > > > > This can be dropped on commit if committer agrees > > > > > > > Xen may have booted on that platform before hand. So this would be > > > > > > > considered a regression and therefore a tag would be warrant. > > > > > > > > > > > > > > AFAICT, the commit is only present on the upcoming 4.18. So the > > > > > > > question is whether Xen 4.17 booted out-of-the-box on ADLink? If > > > > > > > the answer is yes, then we need to add a Fixes tag. But the > > > > > > > correct one would be > > > > > > > > > > > > > > > > > > > @Rahul or Luca: could you give an answer here ? > > > > > > I know you used Xen on an AVA platform but was it booting out of the > > > > > > box ? > > > > > > > > > > I can’t say for Xen 4.17, but our nightly job has run successfully on > > > > > AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 > > > > > (docs/misra: add deviations.rst to document additional deviations.) > > > > > > > > > > We are not applying any patch for it to run on AVA. > > > > Most likely it is because your UEFI/BIOS firmware is 2.x, for instance > > > > 2.04.100.07. > > > > This fix if for AVA machine with older UEFI firmware 1.07.300.03. > > > > > > OOI, why not updating your firmware? I was expecting that it would also > > > contain other critical fixes. > > > > > > With this in mind, I am now more in two mind to ask to merge this patch in > > > Xen 4.18. On one hand, I understand it will help to boot on AVA machine > > > with an older firmware. But on the other hand this is changing the memory > > > layout quite late in the release. The risk seems limited because Xen is > > > not loaded at the top of the virtual address space (there is the directmap > > > afterwards). > > > > > > Henry (as the release manager) and others, any opinions? > > > > With the new information, I think it should be stated that it is fixing an > > issue on AVA platforms using an old firmware and platforms with and > > up-to-date firmware are not impacted. > > It is an important information to keep in mind that this is not a fix to be > > backported for example (and for me the fixes tag should not be kept). > > IMHO, the fixes tag should not be based solely on the AVA platform. It should > be based on whether this could sensibly affect others. Right now, there is > nothing that would indicate either way. > > And therefore a Fixes tag is sensible. This doesn't mean I would want to > backport it right now (note that only 4.18 is affected). But this could change > in the future if we get another report (post-4.18) on a platform where there > are no other workaround. > > Stefano any opinions? The Fixes tag carries useful information but the problem is that it is typically used for identifying backports and this is not a backport (at least today we would not consider it a backport). So I would provide the same information but without using the Fixes tag. For instance: "This commit fixes an issue that was introduced by XXX because of YYY and only affects the AVA platform with not up-to-date firmware". That way, we avoid the risk of someone taking all the applicable commits with a Fixes tag and backporting them without thinking twice about it. But we still have the information in the git log.
Hi Stefano, On 17/10/2023 20:58, Stefano Stabellini wrote: >> And therefore a Fixes tag is sensible. This doesn't mean I would want to >> backport it right now (note that only 4.18 is affected). But this could change >> in the future if we get another report (post-4.18) on a platform where there >> are no other workaround. >> >> Stefano any opinions? > > The Fixes tag carries useful information but the problem is that it is > typically used for identifying backports and this is not a backport (at > least today we would not consider it a backport). Below the definition of Fixes tag (from process/sending-patches.pandoc): If your patch fixes a bug in a specific commit, e.g. you found an issue using ``git bisect``, please use the `Fixes:` tag with the first 12 characters of the commit id, and the one line summary. This doesn't say anything about backport. In fact, we introduced a separate tag for that (see Backport). > > So I would provide the same information but without using the Fixes tag. > For instance: "This commit fixes an issue that was introduced by XXX > because of YYY and only affects the AVA platform with not up-to-date > firmware". The part after 'and' is misleading. So far we had a report only on AVA platform but this doesn't mean it couldn't happen on other HW. The same is true with the new limit. So this wants to be: "and so far we only had a report for the AVA platform when using UEFI older than vX." > > That way, we avoid the risk of someone taking all the applicable commits > with a Fixes tag and backporting them without thinking twice about it. > But we still have the information in the git log. I don't really see the problem for someone to mistakenly backport this patch. In fact, this could potentially save them a lot of debugging if it happens that Xen is loaded above 2TB. Anyway, both Bertrand and you seems to be against the Fixes tag here. So I can compromise with the "This commit fixes...". However, can Bertrand or you update process/send-patches.pandoc so it is clear for a contributor when they should add Fixes tag (which BTW I still disagree with but if the majority agrees, then I will not nack)? Regarding this patch itself, Stefano can you queue to your branch for-4.19? Cheers, -- Julien Grall
Hi, On 18/10/2023 11:59, Julien Grall wrote: > On 17/10/2023 20:58, Stefano Stabellini wrote: >>> And therefore a Fixes tag is sensible. This doesn't mean I would want to >>> backport it right now (note that only 4.18 is affected). But this >>> could change >>> in the future if we get another report (post-4.18) on a platform >>> where there >>> are no other workaround. >>> >>> Stefano any opinions? >> >> The Fixes tag carries useful information but the problem is that it is >> typically used for identifying backports and this is not a backport (at >> least today we would not consider it a backport). > > Below the definition of Fixes tag (from process/sending-patches.pandoc): > > If your patch fixes a bug in a specific commit, e.g. you found an issue > using ``git bisect``, please use the `Fixes:` tag with the first 12 > characters of the commit id, and the one line summary. > > This doesn't say anything about backport. In fact, we introduced a > separate tag for that (see Backport). > >> >> So I would provide the same information but without using the Fixes tag. >> For instance: "This commit fixes an issue that was introduced by XXX >> because of YYY and only affects the AVA platform with not up-to-date >> firmware". > > The part after 'and' is misleading. So far we had a report only on AVA > platform but this doesn't mean it couldn't happen on other HW. The same > is true with the new limit. > > So this wants to be: > > "and so far we only had a report for the AVA platform when using UEFI > older than vX." > >> >> That way, we avoid the risk of someone taking all the applicable commits >> with a Fixes tag and backporting them without thinking twice about it. >> But we still have the information in the git log. > > I don't really see the problem for someone to mistakenly backport this > patch. In fact, this could potentially save them a lot of debugging if > it happens that Xen is loaded above 2TB. > > Anyway, both Bertrand and you seems to be against the Fixes tag here. So > I can compromise with the "This commit fixes...". However, can Bertrand > or you update process/send-patches.pandoc so it is clear for a > contributor when they should add Fixes tag (which BTW I still disagree > with but if the majority agrees, then I will not nack)? We had a chat about this during the Arm maintainer calls. The disagreement boiled down to the fact that SUPPORT.md (or the documentation) doesn't say anything about whether loading Xen above 2TB was supported or not. Depending on the view, one could consider a bug or not. Looking through the documentation, the best place to document might actually be misc/arm/booting.txt where we already have some requirements to boot Xen (such the binary must be entered in NS EL2 mode). I will prepare a patch and send one. Cheers, -- Julien Grall
Hi all, On Wed, Oct 18, 2023 at 07:11:11PM +0100, Julien Grall wrote: [...] > > Anyway, both Bertrand and you seems to be against the Fixes tag here. So > > I can compromise with the "This commit fixes...". However, can Bertrand > > or you update process/send-patches.pandoc so it is clear for a > > contributor when they should add Fixes tag (which BTW I still disagree > > with but if the majority agrees, then I will not nack)? > > We had a chat about this during the Arm maintainer calls. The disagreement > boiled down to the fact that SUPPORT.md (or the documentation) doesn't say > anything about whether loading Xen above 2TB was supported or not. Depending > on the view, one could consider a bug or not. > > Looking through the documentation, the best place to document might actually > be misc/arm/booting.txt where we already have some requirements to boot Xen > (such the binary must be entered in NS EL2 mode). > > I will prepare a patch and send one. I saw Julien's patch "docs/arm: Document where Xen should be loaded in memory" has landed on the staging branch [1]. Should I respin this patch set and update the document to 10TB limitation? I need maintainer's sugguestion, otherwise I don't know how to proceed for this patch set. Thanks, Leo [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=5415b2b2118bd78d8a04f276a8312f7f0cb1a466
Hi Julien, On Wed, Oct 18, 2023 at 07:11:11PM +0100, Julien Grall wrote: > On 18/10/2023 11:59, Julien Grall wrote: > > On 17/10/2023 20:58, Stefano Stabellini wrote: [...] > > I don't really see the problem for someone to mistakenly backport this > > patch. In fact, this could potentially save them a lot of debugging if > > it happens that Xen is loaded above 2TB. > > > > Anyway, both Bertrand and you seems to be against the Fixes tag here. So > > I can compromise with the "This commit fixes...". However, can Bertrand > > or you update process/send-patches.pandoc so it is clear for a > > contributor when they should add Fixes tag (which BTW I still disagree > > with but if the majority agrees, then I will not nack)? > > We had a chat about this during the Arm maintainer calls. The disagreement > boiled down to the fact that SUPPORT.md (or the documentation) doesn't say > anything about whether loading Xen above 2TB was supported or not. Depending > on the view, one could consider a bug or not. > > Looking through the documentation, the best place to document might actually > be misc/arm/booting.txt where we already have some requirements to boot Xen > (such the binary must be entered in NS EL2 mode). > > I will prepare a patch and send one. I would like to check if here is anything specific I should follow up on. Based on the discussion in this thread, I've come to the following conclusions: - Remove the fixes tags; - Add a description in commit log, something like: "Since commit 1c78d76b67e1 ('xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping'), Xen will fail to boot up if it's loaded in memory above 2TB. This commit fixes the regression introduced by that commit." - Add tages: A review tag from Michal Orzel A review tag from Bertrand Marquis A test tag from Henry Wang Should I repin a new patch set to address the items mentioned above? Another question is for the 'Release-acked-by' tag. Henry gave this tag, but I don't know how to handle it if I need to respin this patch. Seems to me this is a special tag only for release process, so I don't need to include it in the new patch, right? Thanks all of you for the efforts on this patch set! Leo
On 19/10/2023 10:35, Leo Yan wrote: > Hi Julien, Hi Leo, > On Wed, Oct 18, 2023 at 07:11:11PM +0100, Julien Grall wrote: >> On 18/10/2023 11:59, Julien Grall wrote: >>> On 17/10/2023 20:58, Stefano Stabellini wrote: > > [...] > >>> I don't really see the problem for someone to mistakenly backport this >>> patch. In fact, this could potentially save them a lot of debugging if >>> it happens that Xen is loaded above 2TB. >>> >>> Anyway, both Bertrand and you seems to be against the Fixes tag here. So >>> I can compromise with the "This commit fixes...". However, can Bertrand >>> or you update process/send-patches.pandoc so it is clear for a >>> contributor when they should add Fixes tag (which BTW I still disagree >>> with but if the majority agrees, then I will not nack)? >> >> We had a chat about this during the Arm maintainer calls. The disagreement >> boiled down to the fact that SUPPORT.md (or the documentation) doesn't say >> anything about whether loading Xen above 2TB was supported or not. Depending >> on the view, one could consider a bug or not. >> >> Looking through the documentation, the best place to document might actually >> be misc/arm/booting.txt where we already have some requirements to boot Xen >> (such the binary must be entered in NS EL2 mode). >> >> I will prepare a patch and send one. > > > I would like to check if here is anything specific I should follow up > on. Based on the discussion in this thread, I've come to the following > conclusions: > > - Remove the fixes tags; > - Add a description in commit log, something like: > "Since commit 1c78d76b67e1 ('xen/arm64: mm: Introduce helpers to > prepare/enable/disable the identity mapping'), Xen will fail to boot > up if it's loaded in memory above 2TB. This commit fixes the > regression introduced by that commit." > - Add tages: > A review tag from Michal Orzel > A review tag from Bertrand Marquis > A test tag from Henry Wang > > Should I repin a new patch set to address the items mentioned above? You will also want to update the documentation after "docs/arm: Document where Xen should be loaded in memory" > > Another question is for the 'Release-acked-by' tag. Henry gave this > tag, but I don't know how to handle it if I need to respin this patch. > Seems to me this is a special tag only for release process, so I don't > need to include it in the new patch, right? The release-acked-by tag is only necessary during freeze period if the patch will land in the next release (i.e. 4.18). In this case, your patch will be part of the 4.19, so you can remove the release-acked-by. If your patch was targeting 4.19, then it is usually fine to keep the release-acked-by even for the respin. It means that the release manager is happy for the patch to go assuming the patch has all the necessary reviews. Cheers, -- Julien Grall
Hi Julien, On Mon, Nov 06, 2023 at 09:39:24AM +0000, Julien Grall wrote: [...] > > I would like to check if here is anything specific I should follow up > > on. Based on the discussion in this thread, I've come to the following > > conclusions: > > > > - Remove the fixes tags; > > - Add a description in commit log, something like: > > "Since commit 1c78d76b67e1 ('xen/arm64: mm: Introduce helpers to > > prepare/enable/disable the identity mapping'), Xen will fail to boot > > up if it's loaded in memory above 2TB. This commit fixes the > > regression introduced by that commit." > > - Add tages: > > A review tag from Michal Orzel > > A review tag from Bertrand Marquis > > A test tag from Henry Wang > > > > Should I repin a new patch set to address the items mentioned above? > > You will also want to update the documentation after > "docs/arm: Document where Xen should be loaded in > memory" Will do. > > Another question is for the 'Release-acked-by' tag. Henry gave this > > tag, but I don't know how to handle it if I need to respin this patch. > > Seems to me this is a special tag only for release process, so I don't > > need to include it in the new patch, right? > > The release-acked-by tag is only necessary during freeze period if the patch > will land in the next release (i.e. 4.18). In this case, your patch will be > part of the 4.19, so you can remove the release-acked-by. Okay, I will _not_ include release-acked-by tag in the respin. > If your patch was targeting 4.19, then it is usually fine to keep the > release-acked-by even for the respin. It means that the release manager is > happy for the patch to go assuming the patch has all the necessary reviews. Thanks for explaination. Leo
Hi Leo, On 06/11/2023 09:45, Leo Yan wrote: > Hi Julien, > > On Mon, Nov 06, 2023 at 09:39:24AM +0000, Julien Grall wrote: > > [...] > >>> I would like to check if here is anything specific I should follow up >>> on. Based on the discussion in this thread, I've come to the following >>> conclusions: >>> >>> - Remove the fixes tags; >>> - Add a description in commit log, something like: >>> "Since commit 1c78d76b67e1 ('xen/arm64: mm: Introduce helpers to >>> prepare/enable/disable the identity mapping'), Xen will fail to boot >>> up if it's loaded in memory above 2TB. This commit fixes the >>> regression introduced by that commit." >>> - Add tages: >>> A review tag from Michal Orzel >>> A review tag from Bertrand Marquis >>> A test tag from Henry Wang >>> >>> Should I repin a new patch set to address the items mentioned above? >> >> You will also want to update the documentation after >> "docs/arm: Document where Xen should be loaded in >> memory" > > Will do. > >>> Another question is for the 'Release-acked-by' tag. Henry gave this >>> tag, but I don't know how to handle it if I need to respin this patch. >>> Seems to me this is a special tag only for release process, so I don't >>> need to include it in the new patch, right? >> >> The release-acked-by tag is only necessary during freeze period if the patch >> will land in the next release (i.e. 4.18). In this case, your patch will be >> part of the 4.19, so you can remove the release-acked-by. > > Okay, I will _not_ include release-acked-by tag in the respin. I have done the changes and directly committed the series. So no need to respin. Cheers, -- Julien Grall
On Thu, Nov 16, 2023 at 02:21:18PM +0000, Julien Grall wrote: [...] > I have done the changes and directly committed the series. So no need to > respin. Thanks a lot, Julien! Leo
> On 16 Oct 2023, at 17:31, Julien Grall <julien@xen.org> wrote: > > > > On 16/10/2023 16:07, Bertrand Marquis wrote: >> Hi, >>> On 16 Oct 2023, at 16:46, Julien Grall <julien@xen.org> wrote: >>> >>> CC Henry >>> >>> Hi Alexey, >>> >>> On 16/10/2023 15:24, Alexey Klimov wrote: >>>> On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>>>> >>>>> >>>>> >>>>>> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >>>>>> >>>>>> Hi >>>>>> >>>>>> +Luca and Rahul >>>>>> >>>>>>> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>>>>> Hi, >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>>>>> the physical memory regions are: >>>>>>>>> >>>>>>>>> DRAM memory regions: >>>>>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>>>>> >>>>>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>>>>> and ramdisk images are loaded into the low memory space: >>>>>>>>> >>>>>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>>>>> >>>>>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>>>>> system fails to boot. >>>>>>>>> >>>>>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>>>>> >>>>>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>>>>> and all of this is platform dependent. >>>>>>>> This can be dropped on commit if committer agrees >>>>>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>>>>> >>>>>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>>>>> >>>>>> >>>>>> @Rahul or Luca: could you give an answer here ? >>>>>> I know you used Xen on an AVA platform but was it booting out of the box ? >>>>> >>>>> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >>>>> (docs/misra: add deviations.rst to document additional deviations.) >>>>> >>>>> We are not applying any patch for it to run on AVA. >>>> Most likely it is because your UEFI/BIOS firmware is 2.x, for instance >>>> 2.04.100.07. >>>> This fix if for AVA machine with older UEFI firmware 1.07.300.03. >>> >>> OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. >>> >>> With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). >>> >>> Henry (as the release manager) and others, any opinions? >> With the new information, I think it should be stated that it is fixing an issue on AVA platforms using an old firmware and platforms with and up-to-date firmware are not impacted. >> It is an important information to keep in mind that this is not a fix to be backported for example (and for me the fixes tag should not be kept). > > IMHO, the fixes tag should not be based solely on the AVA platform. It should be based on whether this could sensibly affect others. Right now, there is nothing that would indicate either way. > > And therefore a Fixes tag is sensible. This doesn't mean I would want to backport it right now (note that only 4.18 is affected). But this could change in the future if we get another report (post-4.18) on a platform where there are no other workaround. > > Stefano any opinions? > >> On whether or not it should go in the release, I am not quite sure that making a change in the layout at that stage is a good idea unless it is fixing a critical issue (which is not the case here). >> So i would vote no but not go against if someone argue to have it in. > > I agree with your statement with the information we have today. But it could become a critical issue if another platform is hit by the same issue and have no other workaround. I am more seeing this as adding support for platforms with a topology that was not supported until now (because it was not encountered) rather than fixing a bug which is why i find it a bit odd to say that it is fixing some issue. But definitely this is an opinion and nothing that i could argue on :-) Cheers Bertrand > > Cheers, > > -- > Julien Grall
On 16/10/2023 16:39, Bertrand Marquis wrote: > > >> On 16 Oct 2023, at 17:31, Julien Grall <julien@xen.org> wrote: >> >> >> >> On 16/10/2023 16:07, Bertrand Marquis wrote: >>> Hi, >>>> On 16 Oct 2023, at 16:46, Julien Grall <julien@xen.org> wrote: >>>> >>>> CC Henry >>>> >>>> Hi Alexey, >>>> >>>> On 16/10/2023 15:24, Alexey Klimov wrote: >>>>> On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@arm.com> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: >>>>>>> >>>>>>> Hi >>>>>>> >>>>>>> +Luca and Rahul >>>>>>> >>>>>>>> On 16 Oct 2023, at 15:54, Julien Grall <julien@xen.org> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 16/10/2023 09:44, Michal Orzel wrote: >>>>>>>>> Hi, >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>>> On 13/10/2023 14:26, Leo Yan wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores), >>>>>>>>>> the physical memory regions are: >>>>>>>>>> >>>>>>>>>> DRAM memory regions: >>>>>>>>>> Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff >>>>>>>>>> Node[0] Region[1]: 0x080000000000 - 0x08007fffffff >>>>>>>>>> Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff >>>>>>>>>> >>>>>>>>>> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel >>>>>>>>>> and ramdisk images are loaded into the low memory space: >>>>>>>>>> >>>>>>>>>> (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen >>>>>>>>>> (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree >>>>>>>>>> (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk >>>>>>>>>> (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel >>>>>>>>>> >>>>>>>>>> In this case, the Xen binary is loaded above 8TB, which exceeds the >>>>>>>>>> maximum supported identity map space of 2TB in Xen. Consequently, the >>>>>>>>>> system fails to boot. >>>>>>>>>> >>>>>>>>>> This patch enlarges identity map space to 10TB, allowing module loading >>>>>>>>>> within the range of [0x0 .. 0x000009ff_ffff_ffff]. >>>>>>>>>> >>>>>>>>>> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable") >>>>>>>>> I don't think a fixes tag applies here given that 2TB was just a number we believed is enough >>>>>>>>> and all of this is platform dependent. >>>>>>>>> This can be dropped on commit if committer agrees >>>>>>>> Xen may have booted on that platform before hand. So this would be considered a regression and therefore a tag would be warrant. >>>>>>>> >>>>>>>> AFAICT, the commit is only present on the upcoming 4.18. So the question is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is yes, then we need to add a Fixes tag. But the correct one would be >>>>>>>> >>>>>>> >>>>>>> @Rahul or Luca: could you give an answer here ? >>>>>>> I know you used Xen on an AVA platform but was it booting out of the box ? >>>>>> >>>>>> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >>>>>> (docs/misra: add deviations.rst to document additional deviations.) >>>>>> >>>>>> We are not applying any patch for it to run on AVA. >>>>> Most likely it is because your UEFI/BIOS firmware is 2.x, for instance >>>>> 2.04.100.07. >>>>> This fix if for AVA machine with older UEFI firmware 1.07.300.03. >>>> >>>> OOI, why not updating your firmware? I was expecting that it would also contain other critical fixes. >>>> >>>> With this in mind, I am now more in two mind to ask to merge this patch in Xen 4.18. On one hand, I understand it will help to boot on AVA machine with an older firmware. But on the other hand this is changing the memory layout quite late in the release. The risk seems limited because Xen is not loaded at the top of the virtual address space (there is the directmap afterwards). >>>> >>>> Henry (as the release manager) and others, any opinions? >>> With the new information, I think it should be stated that it is fixing an issue on AVA platforms using an old firmware and platforms with and up-to-date firmware are not impacted. >>> It is an important information to keep in mind that this is not a fix to be backported for example (and for me the fixes tag should not be kept). >> >> IMHO, the fixes tag should not be based solely on the AVA platform. It should be based on whether this could sensibly affect others. Right now, there is nothing that would indicate either way. >> >> And therefore a Fixes tag is sensible. This doesn't mean I would want to backport it right now (note that only 4.18 is affected). But this could change in the future if we get another report (post-4.18) on a platform where there are no other workaround. >> >> Stefano any opinions? >> >>> On whether or not it should go in the release, I am not quite sure that making a change in the layout at that stage is a good idea unless it is fixing a critical issue (which is not the case here). >>> So i would vote no but not go against if someone argue to have it in. >> >> I agree with your statement with the information we have today. But it could become a critical issue if another platform is hit by the same issue and have no other workaround. > > I am more seeing this as adding support for platforms with a topology that was not supported until now (because it was not encountered) rather than fixing a bug which is why i find it a bit odd to say that it is fixing some issue. But definitely this is an opinion and nothing that i could argue on :-) This is a valid point. The problem is SUPPORT.md doesn't say anything about new HW. So in theory, if it works out-of-the-box on Xen 4.17, then one could expect that it should work on 4.18. I can see why the owner of the platform may consider it critical. But I can see why the community may say it is not important. This is not an easy one to solve because we don't exactly know all the use-cases where Xen is used. We only know the public ones. Cheers, -- Julien Grall
>>> >>> @Rahul or Luca: could you give an answer here ? >>> I know you used Xen on an AVA platform but was it booting out of the box ? >> >> I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3 >> (docs/misra: add deviations.rst to document additional deviations.) >> >> We are not applying any patch for it to run on AVA. > > Most likely it is because your UEFI/BIOS firmware is 2.x, for instance > 2.04.100.07. > This fix if for AVA machine with older UEFI firmware 1.07.300.03. Yes indeed, all our AVA have a newer firmware, so I’m afraid we can’t check Cheers, Luca
© 2016 - 2024 Red Hat, Inc.