[edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[Sunil V L]

OpenSBI now marks PMP regions with "no-map" attribute.
So, remove the workaround and add the ReservedMemory only
when no-map is set so that it follows DT spec.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Andrei Warkentin <andrei.warkentin@intel.com>

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 OvmfPkg/RiscVVirt/Sec/Memory.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/Sec/Memory.c b/OvmfPkg/RiscVVirt/Sec/Memory.c
index aad71ee5dcbb..164b33cd258f 100644
--- a/OvmfPkg/RiscVVirt/Sec/Memory.c
+++ b/OvmfPkg/RiscVVirt/Sec/Memory.c
@@ -141,22 +141,6 @@ GetNumCells (
 
 /** Mark reserved memory ranges in the EFI memory map
 
-  The M-mode firmware ranges should not be used by the
-  EDK2/OS. These ranges are passed via device tree using reserved
-  memory nodes. Parse the DT and mark those ranges as of
-  type EfiReservedMemoryType.
-
-  NOTE: Device Tree spec section 3.5.4 says reserved memory regions
-  without no-map property should be installed as EfiBootServicesData.
-  As per UEFI spec, memory of type EfiBootServicesData can be used
-  by the OS after ExitBootServices().
-  This is not an issue for DT since OS can parse the DT also along
-  with EFI memory map and avoid using these ranges. But with ACPI,
-  there is no such mechanisms possible.
-  Since EDK2 needs to support both DT and ACPI, we are deviating
-  from the DT spec and marking all reserved memory ranges as
-  EfiReservedMemoryType itself irrespective of no-map.
-
   @param FdtPointer Pointer to FDT
 
 **/
@@ -228,11 +212,19 @@ AddReservedMemoryMap (
           Size
           ));
 
-        BuildMemoryAllocationHob (
-          Addr,
-          Size,
-          EfiReservedMemoryType
-          );
+        if (fdt_getprop (FdtPointer, SubNode, "no-map", &Len)) {
+          BuildMemoryAllocationHob (
+            Addr,
+            Size,
+            EfiReservedMemoryType
+            );
+        } else {
+          BuildMemoryAllocationHob (
+            Addr,
+            Size,
+            EfiBootServicesData
+            );
+        }
       }
     }
   }
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106969): https://edk2.groups.io/g/devel/message/106969
Mute This Topic: https://groups.io/mt/100198972/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[Pedro Falcato]

On Mon, Jul 17, 2023 at 5:59 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> OpenSBI now marks PMP regions with "no-map" attribute.
> So, remove the workaround and add the ReservedMemory only
> when no-map is set so that it follows DT spec.

Isn't there a concern for supporting older OpenSBI versions? Is there
no guarantee that it will work?

This change looks like it should break older OpenSBI versions.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106970): https://edk2.groups.io/g/devel/message/106970
Mute This Topic: https://groups.io/mt/100198972/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[Sunil V L]

On Mon, Jul 17, 2023 at 07:03:28PM +0100, Pedro Falcato wrote:
> On Mon, Jul 17, 2023 at 5:59 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
> >
> > OpenSBI now marks PMP regions with "no-map" attribute.
> > So, remove the workaround and add the ReservedMemory only
> > when no-map is set so that it follows DT spec.
> 
> Isn't there a concern for supporting older OpenSBI versions? Is there
> no guarantee that it will work?
> 
> This change looks like it should break older OpenSBI versions.
> 
Hi Pedro,

You are right, but it is OK because this workaround was mainly required
for RISC-V ACPI. Currently, ACPI is not fully enabled for RISC-V. So,
there are no practical users who get affected.

Thanks,
Sunl


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106977): https://edk2.groups.io/g/devel/message/106977
Mute This Topic: https://groups.io/mt/100198972/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[Ranbir Singh]

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

Better to keep the description wrt DT spec.

otherwise, looks good to me.

Reviewed-by: Ranbir Singh <rsingh@ventanamicro.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107047): https://edk2.groups.io/g/devel/message/107047
Mute This Topic: https://groups.io/mt/100198972/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 937 bytes --]

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

[Sunil V L]

On Tue, Jul 18, 2023 at 10:59:31PM -0700, Ranbir Singh wrote:
> Better to keep the description wrt DT spec.
> 
> otherwise, looks good to me.
> 
> Reviewed-by: Ranbir Singh <rsingh@ventanamicro.com>

Thanks!, Ranbir. Sure, I will keep the comment about DT spec
requirement.

-Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107121): https://edk2.groups.io/g/devel/message/107121
Mute This Topic: https://groups.io/mt/100198972/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-