MdeModulePkg/Core/Dxe/DxeMain.h | 18 ++++++++++++++++++ MdeModulePkg/Core/Dxe/Mem/Page.c | 1 + MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 1 - 3 files changed, 19 insertions(+), 1 deletion(-)
Once the paging capabilities were filtered out, there might be some adjacent entries
sharing the same capabilities. It's recommended to merge those entries for the OS
compatibility purpose.
This patch makes use of existing method MergeMemoryMap() to do it. This is done by
simply turning this method from static to extern, and call it after filter code.
This patch is related to an issue described at
https://bugzilla.tianocore.org/show_bug.cgi?id=753
This patch is also passed test of booting follow OSs:
Windows 10
Windows Server 2016
Fedora 26
Fedora 25
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 18 ++++++++++++++++++
MdeModulePkg/Core/Dxe/Mem/Page.c | 1 +
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 1 -
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 1a0babba71..07b86ba696 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
IN UINT64 Length
);
+/**
+ Merge continous memory map entries whose have same attributes.
+
+ @param MemoryMap A pointer to the buffer in which firmware places
+ the current memory map.
+ @param MemoryMapSize A pointer to the size, in bytes, of the
+ MemoryMap buffer. On input, this is the size of
+ the current memory map. On output,
+ it is the size of new memory map after merge.
+ @param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+**/
+VOID
+MergeMemoryMap (
+ IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
+ IN OUT UINTN *MemoryMapSize,
+ IN UINTN DescriptorSize
+ );
+
#endif
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 962ae90d3d..ca4ce69a3f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1915,6 +1915,7 @@ CoreGetMemoryMap (
EFI_MEMORY_XP);
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
}
+ MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
Status = EFI_SUCCESS;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 6cf5edcbe5..75d9b14c1f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -182,7 +182,6 @@ SortMemoryMap (
it is the size of new memory map after merge.
@param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
**/
-STATIC
VOID
MergeMemoryMap (
IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
--
2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: Wang, Jian J
Sent: Monday, November 27, 2017 1:35 PM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
Subject: [PATCH] MdeModulePkg/Core: Merge memory map after filtering paging capability
Once the paging capabilities were filtered out, there might be some adjacent entries sharing the same capabilities. It's recommended to merge those entries for the OS compatibility purpose.
This patch makes use of existing method MergeMemoryMap() to do it. This is done by simply turning this method from static to extern, and call it after filter code.
This patch is related to an issue described at
https://bugzilla.tianocore.org/show_bug.cgi?id=753
This patch is also passed test of booting follow OSs:
Windows 10
Windows Server 2016
Fedora 26
Fedora 25
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 18 ++++++++++++++++++
MdeModulePkg/Core/Dxe/Mem/Page.c | 1 +
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 1 -
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h index 1a0babba71..07b86ba696 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
IN UINT64 Length
);
+/**
+ Merge continous memory map entries whose have same attributes.
+
+ @param MemoryMap A pointer to the buffer in which firmware places
+ the current memory map.
+ @param MemoryMapSize A pointer to the size, in bytes, of the
+ MemoryMap buffer. On input, this is the size of
+ the current memory map. On output,
+ it is the size of new memory map after merge.
+ @param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
+**/
+VOID
+MergeMemoryMap (
+ IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
+ IN OUT UINTN *MemoryMapSize,
+ IN UINTN DescriptorSize
+ );
+
#endif
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 962ae90d3d..ca4ce69a3f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1915,6 +1915,7 @@ CoreGetMemoryMap (
EFI_MEMORY_XP);
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
}
+ MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
Status = EFI_SUCCESS;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 6cf5edcbe5..75d9b14c1f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -182,7 +182,6 @@ SortMemoryMap (
it is the size of new memory map after merge.
@param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
**/
-STATIC
VOID
MergeMemoryMap (
IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
--
2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hello Jian,
On 11/27/17 06:34, Jian J Wang wrote:
> Once the paging capabilities were filtered out, there might be some adjacent entries
> sharing the same capabilities. It's recommended to merge those entries for the OS
> compatibility purpose.
>
> This patch makes use of existing method MergeMemoryMap() to do it. This is done by
> simply turning this method from static to extern, and call it after filter code.
>
> This patch is related to an issue described at
> https://bugzilla.tianocore.org/show_bug.cgi?id=753
>
> This patch is also passed test of booting follow OSs:
> Windows 10
> Windows Server 2016
> Fedora 26
> Fedora 25
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.h | 18 ++++++++++++++++++
> MdeModulePkg/Core/Dxe/Mem/Page.c | 1 +
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 1 -
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 1a0babba71..07b86ba696 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy (
> IN UINT64 Length
> );
>
> +/**
> + Merge continous memory map entries whose have same attributes.
> +
> + @param MemoryMap A pointer to the buffer in which firmware places
> + the current memory map.
> + @param MemoryMapSize A pointer to the size, in bytes, of the
> + MemoryMap buffer. On input, this is the size of
> + the current memory map. On output,
> + it is the size of new memory map after merge.
> + @param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
> +**/
> +VOID
> +MergeMemoryMap (
> + IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
> + IN OUT UINTN *MemoryMapSize,
> + IN UINTN DescriptorSize
> + );
> +
> #endif
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 962ae90d3d..ca4ce69a3f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1915,6 +1915,7 @@ CoreGetMemoryMap (
> EFI_MEMORY_XP);
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size);
> }
> + MergeMemoryMap (MemoryMapStart, &BufferSize, Size);
>
> Status = EFI_SUCCESS;
>
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index 6cf5edcbe5..75d9b14c1f 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -182,7 +182,6 @@ SortMemoryMap (
> it is the size of new memory map after merge.
> @param DescriptorSize Size, in bytes, of an individual EFI_MEMORY_DESCRIPTOR.
> **/
> -STATIC
> VOID
> MergeMemoryMap (
> IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap,
>
This patch looks good to me -- I expect merging the memmap can only
collapse more entries into fewer entries, so the repro / test
instructions that I added to the BZ earlier, and the expected OS
behavior should remain unchanged.
I have one small suggestion: like before, I suggest keeping the local
variables up-to-date after adding the new code, so that further code
need not hunt down invariants as a starting step. Therefore, after the
MergeMemoryMap() call, how about updating MemoryMapEnd, like this:
MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)(
(UINT8 *)MemoryMapStart + BufferSize
);
I think (if Jiewen and Star agree with this suggestion) that you don't
need to post a v2 just for this.
I'm also fine with the patch if the MemoryMapEnd update is rejected.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Can you please add a note to TianoCore BZ#753 when you push this?
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Make sense. Thanks for the comment. > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Tuesday, November 28, 2017 2:21 AM > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg/Core: Merge memory map after > filtering paging capability > > Hello Jian, > > On 11/27/17 06:34, Jian J Wang wrote: > > Once the paging capabilities were filtered out, there might be some adjacent > entries > > sharing the same capabilities. It's recommended to merge those entries for the > OS > > compatibility purpose. > > > > This patch makes use of existing method MergeMemoryMap() to do it. This is > done by > > simply turning this method from static to extern, and call it after filter code. > > > > This patch is related to an issue described at > > https://bugzilla.tianocore.org/show_bug.cgi?id=753 > > > > This patch is also passed test of booting follow OSs: > > Windows 10 > > Windows Server 2016 > > Fedora 26 > > Fedora 25 > > > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Star Zeng <star.zeng@intel.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang <jian.j.wang@intel.com> > > --- > > MdeModulePkg/Core/Dxe/DxeMain.h | 18 ++++++++++++++++++ > > MdeModulePkg/Core/Dxe/Mem/Page.c | 1 + > > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 1 - > > 3 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h > b/MdeModulePkg/Core/Dxe/DxeMain.h > > index 1a0babba71..07b86ba696 100644 > > --- a/MdeModulePkg/Core/Dxe/DxeMain.h > > +++ b/MdeModulePkg/Core/Dxe/DxeMain.h > > @@ -2948,4 +2948,22 @@ ApplyMemoryProtectionPolicy ( > > IN UINT64 Length > > ); > > > > +/** > > + Merge continous memory map entries whose have same attributes. > > + > > + @param MemoryMap A pointer to the buffer in which firmware places > > + the current memory map. > > + @param MemoryMapSize A pointer to the size, in bytes, of the > > + MemoryMap buffer. On input, this is the size of > > + the current memory map. On output, > > + it is the size of new memory map after merge. > > + @param DescriptorSize Size, in bytes, of an individual > EFI_MEMORY_DESCRIPTOR. > > +**/ > > +VOID > > +MergeMemoryMap ( > > + IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap, > > + IN OUT UINTN *MemoryMapSize, > > + IN UINTN DescriptorSize > > + ); > > + > > #endif > > diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c > b/MdeModulePkg/Core/Dxe/Mem/Page.c > > index 962ae90d3d..ca4ce69a3f 100644 > > --- a/MdeModulePkg/Core/Dxe/Mem/Page.c > > +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c > > @@ -1915,6 +1915,7 @@ CoreGetMemoryMap ( > > EFI_MEMORY_XP); > > MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, Size); > > } > > + MergeMemoryMap (MemoryMapStart, &BufferSize, Size); > > > > Status = EFI_SUCCESS; > > > > diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > > index 6cf5edcbe5..75d9b14c1f 100644 > > --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > > +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > > @@ -182,7 +182,6 @@ SortMemoryMap ( > > it is the size of new memory map after merge. > > @param DescriptorSize Size, in bytes, of an individual > EFI_MEMORY_DESCRIPTOR. > > **/ > > -STATIC > > VOID > > MergeMemoryMap ( > > IN OUT EFI_MEMORY_DESCRIPTOR *MemoryMap, > > > > This patch looks good to me -- I expect merging the memmap can only > collapse more entries into fewer entries, so the repro / test > instructions that I added to the BZ earlier, and the expected OS > behavior should remain unchanged. > > I have one small suggestion: like before, I suggest keeping the local > variables up-to-date after adding the new code, so that further code > need not hunt down invariants as a starting step. Therefore, after the > MergeMemoryMap() call, how about updating MemoryMapEnd, like this: > > MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)( > (UINT8 *)MemoryMapStart + BufferSize > ); > > I think (if Jiewen and Star agree with this suggestion) that you don't > need to post a v2 just for this. > > I'm also fine with the patch if the MemoryMapEnd update is rejected. > > Acked-by: Laszlo Ersek <lersek@redhat.com> > > Can you please add a note to TianoCore BZ#753 when you push this? > > Thanks! > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.