[PATCH v2] Use EfiACPIReclaimMemory for ESRT

[PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

A previous patch tried to get Linux to use the ESRT under Xen if it is
in memory of type EfiRuntimeServicesData.  However, this turns out to be
a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
winds up fragmenting both the EFI page tables and the direct map, and
that EfiACPIReclaimMemory is a much better choice for this purpose.

Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
Changes since v1:

- Add link to Ard Biesheuvel’s post on xen-devel
- Expand comment to mention buggy firmware that place the ESRT in
  EfiBootServicesData.

 xen/common/efi/boot.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index db0340c8e2628314226c618dda11ede4c62fdf3b..b3de1011ee58a67a82a94da050eb1343f4e37faa 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -601,11 +601,14 @@ static size_t __init get_esrt_size(const EFI_MEMORY_DESCRIPTOR *desc)
     if ( physical_start > esrt || esrt - physical_start >= len )
         return 0;
     /*
-     * The specification requires EfiBootServicesData, but accept
-     * EfiRuntimeServicesData, which is a more logical choice.
+     * The specification requires EfiBootServicesData, but also accept
+     * EfiRuntimeServicesData (for compatibility with buggy firmware)
+     * and EfiACPIReclaimMemory (which will contain the tables after
+     * successful kexec).
      */
     if ( (desc->Type != EfiRuntimeServicesData) &&
-         (desc->Type != EfiBootServicesData) )
+         (desc->Type != EfiBootServicesData) &&
+         (desc->Type != EfiACPIReclaimMemory) )
         return 0;
     available_len = len - (esrt - physical_start);
     if ( available_len <= offsetof(EFI_SYSTEM_RESOURCE_TABLE, Entries) )
@@ -1144,18 +1147,19 @@ static void __init efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
     for ( i = 0; i < info_size; i += mdesc_size )
     {
         /*
-         * ESRT needs to be moved to memory of type EfiRuntimeServicesData
+         * ESRT needs to be moved to memory of type EfiACPIReclaimMemory
          * so that the memory it is in will not be used for other purposes.
          */
         void *new_esrt = NULL;
-        size_t esrt_size = get_esrt_size(memory_map + i);
+        const EFI_MEMORY_DESCRIPTOR *desc = memory_map + i;
+        size_t esrt_size = get_esrt_size(desc);
 
         if ( !esrt_size )
             continue;
-        if ( ((EFI_MEMORY_DESCRIPTOR *)(memory_map + i))->Type ==
-             EfiRuntimeServicesData )
+        if ( desc->Type == EfiRuntimeServicesData ||
+             desc->Type == EfiACPIReclaimMemory )
             break; /* ESRT already safe from reuse */
-        status = efi_bs->AllocatePool(EfiRuntimeServicesData, esrt_size,
+        status = efi_bs->AllocatePool(EfiACPIReclaimMemory, esrt_size,
                                       &new_esrt);
         if ( status == EFI_SUCCESS && new_esrt )
         {
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Jan Beulich]

On 11.10.2022 05:42, Demi Marie Obenour wrote:
> A previous patch tried to get Linux to use the ESRT under Xen if it is
> in memory of type EfiRuntimeServicesData.  However, this turns out to be
> a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> winds up fragmenting both the EFI page tables and the direct map,

Can this statement please be made describe Xen, not Linux? Aiui at least
the directmap aspect doesn't apply to Xen.

> and
> that EfiACPIReclaimMemory is a much better choice for this purpose.

I think the "better" wants explaining here, without requiring people to
follow ...

> Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html

... this link. Since the code change looks okay to me, I'd be okay to
replace the description with an adjusted one while committing. However,
if you expect the change to go into 4.17, you will want to resubmit
with Henry on Cc:, so he can consider giving the now necessary release-
ack.

Jan

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Tue, Oct 11, 2022 at 11:59:01AM +0200, Jan Beulich wrote:
> On 11.10.2022 05:42, Demi Marie Obenour wrote:
> > A previous patch tried to get Linux to use the ESRT under Xen if it is
> > in memory of type EfiRuntimeServicesData.  However, this turns out to be
> > a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> > winds up fragmenting both the EFI page tables and the direct map,
> 
> Can this statement please be made describe Xen, not Linux? Aiui at least
> the directmap aspect doesn't apply to Xen.

Should it apply to Xen?  My understanding is that Ard’s statements
regarding mismatched attributes refer to any kernel, not just Linux.
You would be in a better position to judge that, though.

> > and
> > that EfiACPIReclaimMemory is a much better choice for this purpose.
> 
> I think the "better" wants explaining here, without requiring people to
> follow ...

Something like, “EfiACPIReclaimMemory is the correct type for
configuration tables that are only used by the OS.”?

> > Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
> 
> ... this link. Since the code change looks okay to me, I'd be okay to
> replace the description with an adjusted one while committing.

That is fine with me.

> However,
> if you expect the change to go into 4.17, you will want to resubmit
> with Henry on Cc:, so he can consider giving the now necessary release-
> ack.

Will do, with your updated commit message.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Jan Beulich]

On 11.10.2022 18:52, Demi Marie Obenour wrote:
> On Tue, Oct 11, 2022 at 11:59:01AM +0200, Jan Beulich wrote:
>> On 11.10.2022 05:42, Demi Marie Obenour wrote:
>>> A previous patch tried to get Linux to use the ESRT under Xen if it is
>>> in memory of type EfiRuntimeServicesData.  However, this turns out to be
>>> a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
>>> winds up fragmenting both the EFI page tables and the direct map,
>>
>> Can this statement please be made describe Xen, not Linux? Aiui at least
>> the directmap aspect doesn't apply to Xen.
> 
> Should it apply to Xen?  My understanding is that Ard’s statements
> regarding mismatched attributes refer to any kernel, not just Linux.
> You would be in a better position to judge that, though.

We run EFI runtime services functions on their own page tables (with
certain areas copied from the directmap). With EfiACPIReclaimMemory
converted to E820_ACPI we do not insert those ranges into the directmap
(i.e. no difference to EfiRuntimeServices*). At least this latter fact
means fragmentation effects - if they exist - are the same for both
types.

>>> and
>>> that EfiACPIReclaimMemory is a much better choice for this purpose.
>>
>> I think the "better" wants explaining here, without requiring people to
>> follow ...
> 
> Something like, “EfiACPIReclaimMemory is the correct type for
> configuration tables that are only used by the OS.”?

Preferably with "supposedly" inserted, unless you (or Ard) can point
out a place in the spec where this is actually written down.

Jan

[PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

A previous patch tried to get Linux to use the ESRT under Xen if it is
in memory of type EfiRuntimeServicesData.  However, this turns out to be
a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
winds up fragmenting both the EFI page tables and the direct map, and
that EfiACPIReclaimMemory is a much better choice for this purpose.

Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
Should this be included in 4.17?  It is a bug fix for a feature new to
4.17, so I suspect yes, but it is ultimately up to Henry Wang.

 xen/common/efi/boot.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index db0340c8e2628314226c618dda11ede4c62fdf3b..b3de1011ee58a67a82a94da050eb1343f4e37faa 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -601,11 +601,14 @@ static size_t __init get_esrt_size(const EFI_MEMORY_DESCRIPTOR *desc)
     if ( physical_start > esrt || esrt - physical_start >= len )
         return 0;
     /*
-     * The specification requires EfiBootServicesData, but accept
-     * EfiRuntimeServicesData, which is a more logical choice.
+     * The specification requires EfiBootServicesData, but also accept
+     * EfiRuntimeServicesData (for compatibility with buggy firmware)
+     * and EfiACPIReclaimMemory (which will contain the tables after
+     * successful kexec).
      */
     if ( (desc->Type != EfiRuntimeServicesData) &&
-         (desc->Type != EfiBootServicesData) )
+         (desc->Type != EfiBootServicesData) &&
+         (desc->Type != EfiACPIReclaimMemory) )
         return 0;
     available_len = len - (esrt - physical_start);
     if ( available_len <= offsetof(EFI_SYSTEM_RESOURCE_TABLE, Entries) )
@@ -1144,18 +1147,19 @@ static void __init efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
     for ( i = 0; i < info_size; i += mdesc_size )
     {
         /*
-         * ESRT needs to be moved to memory of type EfiRuntimeServicesData
+         * ESRT needs to be moved to memory of type EfiACPIReclaimMemory
          * so that the memory it is in will not be used for other purposes.
          */
         void *new_esrt = NULL;
-        size_t esrt_size = get_esrt_size(memory_map + i);
+        const EFI_MEMORY_DESCRIPTOR *desc = memory_map + i;
+        size_t esrt_size = get_esrt_size(desc);
 
         if ( !esrt_size )
             continue;
-        if ( ((EFI_MEMORY_DESCRIPTOR *)(memory_map + i))->Type ==
-             EfiRuntimeServicesData )
+        if ( desc->Type == EfiRuntimeServicesData ||
+             desc->Type == EfiACPIReclaimMemory )
             break; /* ESRT already safe from reuse */
-        status = efi_bs->AllocatePool(EfiRuntimeServicesData, esrt_size,
+        status = efi_bs->AllocatePool(EfiACPIReclaimMemory, esrt_size,
                                       &new_esrt);
         if ( status == EFI_SUCCESS && new_esrt )
         {
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

RE: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Henry Wang]

Hi Demi,

(+Julien for his info since I am replying below about the 4.17 stuff)

> -----Original Message-----
> From: Demi Marie Obenour <demi@invisiblethingslab.com>
> Subject: [PATCH v2] Use EfiACPIReclaimMemory for ESRT
> 
> A previous patch tried to get Linux to use the ESRT under Xen if it is
> in memory of type EfiRuntimeServicesData.  However, this turns out to be
> a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> winds up fragmenting both the EFI page tables and the direct map, and
> that EfiACPIReclaimMemory is a much better choice for this purpose.
> 
> Link: https://lists.xenproject.org/archives/html/xen-devel/2022-
> 09/msg01365.html
> Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> Should this be included in 4.17?  It is a bug fix for a feature new to
> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.

I am planning to start the process of tagging the 4.17 so we can catch the
release date next Monday before the holiday season in Europe.

That said, if the EFI maintainer (Jan) is happy about including this in 4.17,
I don't object it if this patch can land in the stable-4.17 branch before the
end of Thursday (Dec. 8). Note that OSSTest probably needs ~1 day to do the
sync from the staging-4.17 branch.

Kind regards,
Henry

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

On Wed, Dec 07, 2022 at 02:44:11AM +0000, Henry Wang wrote:
> Hi Demi,
> 
> (+Julien for his info since I am replying below about the 4.17 stuff)
> 
> > -----Original Message-----
> > From: Demi Marie Obenour <demi@invisiblethingslab.com>
> > Subject: [PATCH v2] Use EfiACPIReclaimMemory for ESRT
> > 
> > A previous patch tried to get Linux to use the ESRT under Xen if it is
> > in memory of type EfiRuntimeServicesData.  However, this turns out to be
> > a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> > winds up fragmenting both the EFI page tables and the direct map, and
> > that EfiACPIReclaimMemory is a much better choice for this purpose.
> > 
> > Link: https://lists.xenproject.org/archives/html/xen-devel/2022-
> > 09/msg01365.html
> > Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > ---
> > Should this be included in 4.17?  It is a bug fix for a feature new to
> > 4.17, so I suspect yes, but it is ultimately up to Henry Wang.
> 
> I am planning to start the process of tagging the 4.17 so we can catch the
> release date next Monday before the holiday season in Europe.

> That said, if the EFI maintainer (Jan) is happy about including this in 4.17,
> I don't object it if this patch can land in the stable-4.17 branch before the
> end of Thursday (Dec. 8). Note that OSSTest probably needs ~1 day to do the
> sync from the staging-4.17 branch.

That’s fine, thanks!
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Jan Beulich]

On 07.12.2022 00:27, Demi Marie Obenour wrote:
> A previous patch tried to get Linux to use the ESRT under Xen if it is
> in memory of type EfiRuntimeServicesData.  However, this turns out to be
> a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> winds up fragmenting both the EFI page tables and the direct map, and
> that EfiACPIReclaimMemory is a much better choice for this purpose.
> 
> Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
> Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> Should this be included in 4.17?  It is a bug fix for a feature new to
> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.

First I was surprised this is numbered v2. But it indeed looks to be a
plain re-submission, merely with Henry Cc-ed. You didn't address my
comments; you didn't even incorporate the one adjustment where you
suggested alternative wording and where I did signal my agreement. All
this form of plain re-submission (without any kind of remark in that
direction) did is that I spent (wasted) time checking what the earlier
variant had, what was requested to be changed, and whether any of the
changes were actually carried out.

Jan

Re: [PATCH v2] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

On Wed, Dec 07, 2022 at 11:11:40AM +0100, Jan Beulich wrote:
> On 07.12.2022 00:27, Demi Marie Obenour wrote:
> > A previous patch tried to get Linux to use the ESRT under Xen if it is
> > in memory of type EfiRuntimeServicesData.  However, this turns out to be
> > a bad idea.  Ard Biesheuvel pointed out that EfiRuntimeServices* memory
> > winds up fragmenting both the EFI page tables and the direct map, and
> > that EfiACPIReclaimMemory is a much better choice for this purpose.
> > 
> > Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
> > Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > ---
> > Should this be included in 4.17?  It is a bug fix for a feature new to
> > 4.17, so I suspect yes, but it is ultimately up to Henry Wang.
> 
> First I was surprised this is numbered v2. But it indeed looks to be a
> plain re-submission, merely with Henry Cc-ed. You didn't address my
> comments; you didn't even incorporate the one adjustment where you
> suggested alternative wording and where I did signal my agreement. All
> this form of plain re-submission (without any kind of remark in that
> direction) did is that I spent (wasted) time checking what the earlier
> variant had, what was requested to be changed, and whether any of the
> changes were actually carried out.

Whoops, sorry about that.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

A previous patch tried to get Linux to use the ESRT under Xen if it is
in memory of type EfiRuntimeServicesData.  However, EfiRuntimeServices*
memory needs to be included in the EFI page tables, so it is best to
minimize the amount of memory of this type.  Since EFI runtime services
do not need access to the ESRT, EfiACPIReclaimMemory is a better choice.

Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
Should this be included in 4.17?  It is a bug fix for a feature new to
4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
is identical to v2, but I have improved the commit message.

 xen/common/efi/boot.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index db0340c8e2628314226c618dda11ede4c62fdf3b..b3de1011ee58a67a82a94da050eb1343f4e37faa 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -601,11 +601,14 @@ static size_t __init get_esrt_size(const EFI_MEMORY_DESCRIPTOR *desc)
     if ( physical_start > esrt || esrt - physical_start >= len )
         return 0;
     /*
-     * The specification requires EfiBootServicesData, but accept
-     * EfiRuntimeServicesData, which is a more logical choice.
+     * The specification requires EfiBootServicesData, but also accept
+     * EfiRuntimeServicesData (for compatibility with buggy firmware)
+     * and EfiACPIReclaimMemory (which will contain the tables after
+     * successful kexec).
      */
     if ( (desc->Type != EfiRuntimeServicesData) &&
-         (desc->Type != EfiBootServicesData) )
+         (desc->Type != EfiBootServicesData) &&
+         (desc->Type != EfiACPIReclaimMemory) )
         return 0;
     available_len = len - (esrt - physical_start);
     if ( available_len <= offsetof(EFI_SYSTEM_RESOURCE_TABLE, Entries) )
@@ -1144,18 +1147,19 @@ static void __init efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
     for ( i = 0; i < info_size; i += mdesc_size )
     {
         /*
-         * ESRT needs to be moved to memory of type EfiRuntimeServicesData
+         * ESRT needs to be moved to memory of type EfiACPIReclaimMemory
          * so that the memory it is in will not be used for other purposes.
          */
         void *new_esrt = NULL;
-        size_t esrt_size = get_esrt_size(memory_map + i);
+        const EFI_MEMORY_DESCRIPTOR *desc = memory_map + i;
+        size_t esrt_size = get_esrt_size(desc);
 
         if ( !esrt_size )
             continue;
-        if ( ((EFI_MEMORY_DESCRIPTOR *)(memory_map + i))->Type ==
-             EfiRuntimeServicesData )
+        if ( desc->Type == EfiRuntimeServicesData ||
+             desc->Type == EfiACPIReclaimMemory )
             break; /* ESRT already safe from reuse */
-        status = efi_bs->AllocatePool(EfiRuntimeServicesData, esrt_size,
+        status = efi_bs->AllocatePool(EfiACPIReclaimMemory, esrt_size,
                                       &new_esrt);
         if ( status == EFI_SUCCESS && new_esrt )
         {
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Jan Beulich]

On 07.12.2022 23:42, Demi Marie Obenour wrote:
> A previous patch tried to get Linux to use the ESRT under Xen if it is
> in memory of type EfiRuntimeServicesData.  However, EfiRuntimeServices*
> memory needs to be included in the EFI page tables, so it is best to
> minimize the amount of memory of this type.  Since EFI runtime services
> do not need access to the ESRT, EfiACPIReclaimMemory is a better choice.
> 
> Link: https://lists.xenproject.org/archives/html/xen-devel/2022-09/msg01365.html
> Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

> Should this be included in 4.17?  It is a bug fix for a feature new to
> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> is identical to v2, but I have improved the commit message.

It may be too late now, looking at the state of the tree. Henry, Julien?

Jan

RE: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Henry Wang]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> 
> On 07.12.2022 23:42, Demi Marie Obenour wrote:
> > A previous patch tried to get Linux to use the ESRT under Xen if it is
> > in memory of type EfiRuntimeServicesData.  However, EfiRuntimeServices*
> > memory needs to be included in the EFI page tables, so it is best to
> > minimize the amount of memory of this type.  Since EFI runtime services
> > do not need access to the ESRT, EfiACPIReclaimMemory is a better choice.
> >
> > Link: https://lists.xenproject.org/archives/html/xen-devel/2022-
> 09/msg01365.html
> > Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
> > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> > Should this be included in 4.17?  It is a bug fix for a feature new to
> > 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> > is identical to v2, but I have improved the commit message.
> 
> It may be too late now, looking at the state of the tree. Henry, Julien?

Like I said in v2, I don't object the change if you would like to include this patch
to 4.17, so if you are sure this patch is safe and want to commit it, feel free to add:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Since we also need to commit:
"[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my side
I am no problem. Julien might have different opinion though, if Julien object
the change I would like to respect his opinion and leave this patch uncommitted.

Kind regards,
Henry

> 
> Jan

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Julien Grall]

Hi,

On 08/12/2022 08:53, Henry Wang wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
>>
>> On 07.12.2022 23:42, Demi Marie Obenour wrote:
>>> A previous patch tried to get Linux to use the ESRT under Xen if it is
>>> in memory of type EfiRuntimeServicesData.  However, EfiRuntimeServices*
>>> memory needs to be included in the EFI page tables, so it is best to
>>> minimize the amount of memory of this type.  Since EFI runtime services
>>> do not need access to the ESRT, EfiACPIReclaimMemory is a better choice.
>>>
>>> Link: https://lists.xenproject.org/archives/html/xen-devel/2022-
>> 09/msg01365.html
>>> Fixes: dc7da0874ba4 ("EFI: preserve the System Resource Table for dom0")
>>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>>> Should this be included in 4.17?  It is a bug fix for a feature new to
>>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
>>> is identical to v2, but I have improved the commit message.
>>
>> It may be too late now, looking at the state of the tree. Henry, Julien?
> 
> Like I said in v2, I don't object the change if you would like to include this patch
> to 4.17, so if you are sure this patch is safe and want to commit it, feel free to add:
> 
> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> 
> Since we also need to commit:
> "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my side
> I am no problem. Julien might have different opinion though, if Julien object
> the change I would like to respect his opinion and leave this patch uncommitted.

I have committed it after SUPPORT.md. So if for some reasons we are seen 
any issues with Osstest, then I can tag the tree without this patch 
(that said, I would rather prefer if we have staging-4.17 == stable-4.17).

My plan is to prepare the tarball tomorrow.

Cheers,

-- 
Julien Grall

RE: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Henry Wang]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> 
> Hi,
> 
> >>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> >>
> >> Acked-by: Jan Beulich <jbeulich@suse.com>
> >>
> >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> >>> is identical to v2, but I have improved the commit message.
> >>
> >> It may be too late now, looking at the state of the tree. Henry, Julien?
> >
> > Like I said in v2, I don't object the change if you would like to include this
> patch
> > to 4.17, so if you are sure this patch is safe and want to commit it, feel free
> to add:
> >
> > Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> >
> > Since we also need to commit:
> > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my side
> > I am no problem. Julien might have different opinion though, if Julien
> object
> > the change I would like to respect his opinion and leave this patch
> uncommitted.
> 
> I have committed it after SUPPORT.md. So if for some reasons we are seen
> any issues with Osstest, then I can tag the tree without this patch

This is a great solution :)

> (that said, I would rather prefer if we have staging-4.17 == stable-4.17).

Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
So we are ready to tag.

> 
> My plan is to prepare the tarball tomorrow.

Thanks very much.

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

On Fri, Dec 09, 2022 at 07:37:53AM +0000, Henry Wang wrote:
> Hi Julien,
> 
> > -----Original Message-----
> > From: Julien Grall <julien@xen.org>
> > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > 
> > Hi,
> > 
> > >>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > >>
> > >> Acked-by: Jan Beulich <jbeulich@suse.com>
> > >>
> > >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> > >>> is identical to v2, but I have improved the commit message.
> > >>
> > >> It may be too late now, looking at the state of the tree. Henry, Julien?
> > >
> > > Like I said in v2, I don't object the change if you would like to include this
> > patch
> > > to 4.17, so if you are sure this patch is safe and want to commit it, feel free
> > to add:
> > >
> > > Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> > >
> > > Since we also need to commit:
> > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my side
> > > I am no problem. Julien might have different opinion though, if Julien
> > object
> > > the change I would like to respect his opinion and leave this patch
> > uncommitted.
> > 
> > I have committed it after SUPPORT.md. So if for some reasons we are seen
> > any issues with Osstest, then I can tag the tree without this patch
> 
> This is a great solution :)
> 
> > (that said, I would rather prefer if we have staging-4.17 == stable-4.17).
> 
> Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> So we are ready to tag.

And it turns out that I botched the initial patch, sorry.  (I forgot to
handle the multiboot2 case.)

I understand if it is too late for stable-4.17, but it ought to make
stable 4.17.1 as it was simply omitted from the initial patch series.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Henry Wang]

Hi,

> -----Original Message-----
> From: Demi Marie Obenour <demi@invisiblethingslab.com>
> Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> 
> On Fri, Dec 09, 2022 at 07:37:53AM +0000, Henry Wang wrote:
> > Hi Julien,
> >
> > > -----Original Message-----
> > > From: Julien Grall <julien@xen.org>
> > > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > >
> > > Hi,
> > >
> > > >>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > > >>
> > > >> Acked-by: Jan Beulich <jbeulich@suse.com>
> > > >>
> > > >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> > > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> > > >>> is identical to v2, but I have improved the commit message.
> > > >>
> > > >> It may be too late now, looking at the state of the tree. Henry, Julien?
> > > >
> > > > Like I said in v2, I don't object the change if you would like to include
> this
> > > patch
> > > > to 4.17, so if you are sure this patch is safe and want to commit it, feel
> free
> > > to add:
> > > >
> > > > Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> > > >
> > > > Since we also need to commit:
> > > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my
> side
> > > > I am no problem. Julien might have different opinion though, if Julien
> > > object
> > > > the change I would like to respect his opinion and leave this patch
> > > uncommitted.
> > >
> > > I have committed it after SUPPORT.md. So if for some reasons we are
> seen
> > > any issues with Osstest, then I can tag the tree without this patch
> >
> > This is a great solution :)
> >
> > > (that said, I would rather prefer if we have staging-4.17 == stable-4.17).
> >
> > Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> > So we are ready to tag.
> 
> And it turns out that I botched the initial patch, sorry.  (I forgot to
> handle the multiboot2 case.)
> 
> I understand if it is too late for stable-4.17, but it ought to make
> stable 4.17.1 as it was simply omitted from the initial patch series.

I don't think this patch will make it today so I would suggest we still follow
what Julien planned yesterday. Also I think this is also consistent with the
release management guideline.

Kind regards,
Henry


> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]

On Fri, Dec 09, 2022 at 02:54:15PM +0000, Henry Wang wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Demi Marie Obenour <demi@invisiblethingslab.com>
> > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > 
> > On Fri, Dec 09, 2022 at 07:37:53AM +0000, Henry Wang wrote:
> > > Hi Julien,
> > >
> > > > -----Original Message-----
> > > > From: Julien Grall <julien@xen.org>
> > > > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > > >
> > > > Hi,
> > > >
> > > > >>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > > > >>
> > > > >> Acked-by: Jan Beulich <jbeulich@suse.com>
> > > > >>
> > > > >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> > > > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> > > > >>> is identical to v2, but I have improved the commit message.
> > > > >>
> > > > >> It may be too late now, looking at the state of the tree. Henry, Julien?
> > > > >
> > > > > Like I said in v2, I don't object the change if you would like to include
> > this
> > > > patch
> > > > > to 4.17, so if you are sure this patch is safe and want to commit it, feel
> > free
> > > > to add:
> > > > >
> > > > > Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> > > > >
> > > > > Since we also need to commit:
> > > > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my
> > side
> > > > > I am no problem. Julien might have different opinion though, if Julien
> > > > object
> > > > > the change I would like to respect his opinion and leave this patch
> > > > uncommitted.
> > > >
> > > > I have committed it after SUPPORT.md. So if for some reasons we are
> > seen
> > > > any issues with Osstest, then I can tag the tree without this patch
> > >
> > > This is a great solution :)
> > >
> > > > (that said, I would rather prefer if we have staging-4.17 == stable-4.17).
> > >
> > > Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> > > So we are ready to tag.
> > 
> > And it turns out that I botched the initial patch, sorry.  (I forgot to
> > handle the multiboot2 case.)
> > 
> > I understand if it is too late for stable-4.17, but it ought to make
> > stable 4.17.1 as it was simply omitted from the initial patch series.
> 
> I don't think this patch will make it today so I would suggest we still follow
> what Julien planned yesterday. Also I think this is also consistent with the
> release management guideline.

That’s okay.  Qubes can take this as an out of tree patch until it is
merged.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]