Date   

Re: SPCR / SSDT generation in the DynamicTablesPkg

Sami Mujawar <Sami.Mujawar@...>
 

Hi Irene, Jeff,

 

If I understand correctly, when the HID is PNP0501 the Linux driver ‘drivers\tty\serial\8250\8250_pnp.c’ is setting ‘uart.port.iotype = UPIO_MEM;’

This results in the read/write access size being 1 byte. Is this the problem you are seeing?

 

If so, are you intending to change the HID in your serial port SSDT? Or you are defining a property to specify the access size?

 

Please let me know. I am thinking if this problem can be solved more generically.

 

With regards to SSDT generation I can see a few options here:

1. Implement a SsdtSerialPortFixupLib for your platform.

    This would mean implementing the interfaces in https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/Library/SsdtSerialPortFixupLib.h

 

2. A Feature PCD DisableUartSsdtGeneration could be introduced with the default value being FALSE.

 

I would prefer Option 1 as it would keep the Dynamic Tables Core code SBBR compliant.

 

Regards,

 

Sami Mujawar

 

From: Jeff Brasen <jbrasen@...>
Sent: 18 November 2020 09:36 AM
To: discuss@edk2.groups.io; Irene Park <ipark@...>; Sami Mujawar <Sami.Mujawar@...>; Pierre Gondois <Pierre.Gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

 

To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,

Jeff


From: Irene Park <ipark@...>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To:
discuss@edk2.groups.io <discuss@edk2.groups.io>; Irene Park <ipark@...>; Sami.Mujawar@... <Sami.Mujawar@...>; pierre.gondois@... <pierre.gondois@...>; Alexei Fedorov <Alexei.Fedorov@...>; Jeff Brasen <jbrasen@...>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

 

Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: redundant NULL pointer check

wenyi,xie
 

Hi Laszlo,

Thank you for your detailed explanation, I get it.
I will post a patch later.

Thanks
Wenyi

On 2020/11/25 6:56, Laszlo Ersek wrote:
On 11/24/20 12:05, xiewenyi (A) wrote:
Hi Laszlo,

So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
Well, we can approach this on two levels.

The first level is whether the code transformation (the patch) that you
are describing above would be correct. Yes, removing the comparison in
question is correct. Please feel free to post the patch.

But the deeper (zeroth) level is more nuanced. Namely, it's not
specifically the *non-nullity* of "Info" that guarantees
(Info->VolumeLabel != NULL). Instead, that consequence originates from
the *validity* of Info (namely, that it points to an
EFI_FILE_SYSTEM_VOLUME_LABEL object), and from the fact that
"VolumeLabel" is an *array* in EFI_FILE_SYSTEM_VOLUME_LABEL. The address
of an array object (well, of any object) that *exists* can never be NULL.

I'm not sure if my depiction is understandable. Basically, if Info is
not NULL, then (Info->VolumeLabel != NULL) holds *exactly because*:

CHAR16 VolumeLabel[1];

ASSERT (&VolumeLabel[0] != NULL);

holds as well.

Anyway... feel free to post the patch.

Thanks!
Laszlo

On 2020/11/24 18:52, Laszlo Ersek wrote:
On 11/24/20 10:22, wenyi,xie via groups.io wrote:
Hi all,
When removing -Wno-tautological-compare, there's a warning.

MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare]
Search "no-tautological-compare" (4904 hits in 1 file)

Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place.
So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ?
Assuming Info is a valid pointer, Info->VolumeLabel is an array object.

When evaluated, Info->VolumeLabel is converted to the address of its
first element.

The address of the first element of an array object can never be NULL.
And so (Info->VolumeLabel == NULL) will always evaluate to FALSE.

Thanks
Laszlo

.
.


Re: redundant NULL pointer check

Laszlo Ersek
 

On 11/24/20 12:05, xiewenyi (A) wrote:
Hi Laszlo,

So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.
Well, we can approach this on two levels.

The first level is whether the code transformation (the patch) that you
are describing above would be correct. Yes, removing the comparison in
question is correct. Please feel free to post the patch.

But the deeper (zeroth) level is more nuanced. Namely, it's not
specifically the *non-nullity* of "Info" that guarantees
(Info->VolumeLabel != NULL). Instead, that consequence originates from
the *validity* of Info (namely, that it points to an
EFI_FILE_SYSTEM_VOLUME_LABEL object), and from the fact that
"VolumeLabel" is an *array* in EFI_FILE_SYSTEM_VOLUME_LABEL. The address
of an array object (well, of any object) that *exists* can never be NULL.

I'm not sure if my depiction is understandable. Basically, if Info is
not NULL, then (Info->VolumeLabel != NULL) holds *exactly because*:

CHAR16 VolumeLabel[1];

ASSERT (&VolumeLabel[0] != NULL);

holds as well.

Anyway... feel free to post the patch.

Thanks!
Laszlo

On 2020/11/24 18:52, Laszlo Ersek wrote:
On 11/24/20 10:22, wenyi,xie via groups.io wrote:
Hi all,
When removing -Wno-tautological-compare, there's a warning.

MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare]
Search "no-tautological-compare" (4904 hits in 1 file)

Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place.
So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ?
Assuming Info is a valid pointer, Info->VolumeLabel is an array object.

When evaluated, Info->VolumeLabel is converted to the address of its
first element.

The address of the first element of an array object can never be NULL.
And so (Info->VolumeLabel == NULL) will always evaluate to FALSE.

Thanks
Laszlo

.


Re: redundant NULL pointer check

wenyi,xie
 

Hi Laszlo,

So if (Info->VolumeLabel == NULL) will always evaluate to FALSE when Info != NULL. Can we just remove it.

Thanks
Wenyi

On 2020/11/24 18:52, Laszlo Ersek wrote:
On 11/24/20 10:22, wenyi,xie via groups.io wrote:
Hi all,
When removing -Wno-tautological-compare, there's a warning.

MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare]
Search "no-tautological-compare" (4904 hits in 1 file)

Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place.
So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ?
Assuming Info is a valid pointer, Info->VolumeLabel is an array object.

When evaluated, Info->VolumeLabel is converted to the address of its
first element.

The address of the first element of an array object can never be NULL.
And so (Info->VolumeLabel == NULL) will always evaluate to FALSE.

Thanks
Laszlo

.


Re: redundant NULL pointer check

Laszlo Ersek
 

On 11/24/20 10:22, wenyi,xie via groups.io wrote:
Hi all,
When removing -Wno-tautological-compare, there's a warning.

MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare]
Search "no-tautological-compare" (4904 hits in 1 file)

Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place.
So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ?
Assuming Info is a valid pointer, Info->VolumeLabel is an array object.

When evaluated, Info->VolumeLabel is converted to the address of its
first element.

The address of the first element of an array object can never be NULL.
And so (Info->VolumeLabel == NULL) will always evaluate to FALSE.

Thanks
Laszlo


redundant NULL pointer check

wenyi,xie
 

Hi all,
When removing -Wno-tautological-compare, there's a warning.

MdeModulePkg/Library/FileExplorerLib/FileExplorer.c:850:19: warning: comparison of array 'Info->VolumeLabel' equal to a null pointer is always false [-Wtautological-pointer-compare]
Search "no-tautological-compare" (4904 hits in 1 file)

Since Info is a pointer of struct EFI_FILE_SYSTEM_VOLUME_LABEL, and this struct has only one member VolumeLabel. So Info and Info->VolumeLabel are point to the same place.
So if Info != NULL, is it necessary to check Info->VolumeLabel == NULL again ?

Thanks
Wenyi


Re: SPCR / SSDT generation in the DynamicTablesPkg

Jeff Brasen
 

Yes, that is the issue we are seeing.

Our COM acpi node has a different HID NVDA0100 which uses drivers\tty\serial\8250\8250_tegra.c in linux. It also has a _DSD node to expose the configured clock rate as that driver needs that as with device tree based bindings it gets that from the clock subsystem.

I did just test a forked copy of SsdtSerialPortFixupLib for our platfrom and it did work, but it seems like there would be an issue if someone wanted to use what is in DynamicPkg as the SPCR is marked as 4-byte access but the linux driver would use 1-byte accesses. Of course if we change the SPCR generation to be 1-byte that would break our systems table.


Thanks,

Jeff

________________________________
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Thursday, November 19, 2020 10:31 AM
To: Jeff Brasen <jbrasen@nvidia.com>; discuss@edk2.groups.io <discuss@edk2.groups.io>; Irene Park <ipark@nvidia.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; nd <nd@arm.com>
Cc: Thanu Rangarajan <Thanu.Rangarajan@arm.com>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


Hi Irene, Jeff,



If I understand correctly, when the HID is PNP0501 the Linux driver ‘drivers\tty\serial\8250\8250_pnp.c’ is setting ‘uart.port.iotype = UPIO_MEM;’

This results in the read/write access size being 1 byte. Is this the problem you are seeing?



If so, are you intending to change the HID in your serial port SSDT? Or you are defining a property to specify the access size?



Please let me know. I am thinking if this problem can be solved more generically.



With regards to SSDT generation I can see a few options here:

1. Implement a SsdtSerialPortFixupLib for your platform.

This would mean implementing the interfaces in https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Include/Library/SsdtSerialPortFixupLib.h



2. A Feature PCD DisableUartSsdtGeneration could be introduced with the default value being FALSE.



I would prefer Option 1 as it would keep the Dynamic Tables Core code SBBR compliant.



Regards,



Sami Mujawar



From: Jeff Brasen <jbrasen@nvidia.com>
Sent: 18 November 2020 09:36 AM
To: discuss@edk2.groups.io; Irene Park <ipark@nvidia.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>
Subject: Re: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,

Jeff

Get Outlook for Android<https://aka.ms/ghei36>



________________________________

From: Irene Park <ipark@nvidia.com<mailto:ipark@nvidia.com>>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>>; Irene Park <ipark@nvidia.com<mailto:ipark@nvidia.com>>; Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com> <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; pierre.gondois@arm.com<mailto:pierre.gondois@arm.com> <pierre.gondois@arm.com<mailto:pierre.gondois@arm.com>>; Alexei Fedorov <Alexei.Fedorov@arm.com<mailto:Alexei.Fedorov@arm.com>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg



Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io> <discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io<mailto:discuss@edk2.groups.io>
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene





IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-staging] Plan for update Intel UNDI driver source to version 25.1.1

Nhi Pham
 

Hello Maciej Rabeda and All,

I found that the Intel Ethernet Driver 24.3 is public here https://github.com/tianocore/edk2-staging/tree/Intel_UNDI
I see that there is a high demand on Intel UNDI driver source to facilitate the native driver for some Ethernet cards like I210 Network card on ARM64. So, do you have any plan to update IntelUndiPkg to version 25.1.1? And could we merge it into edk2-non-osi repository for use?

Thank you,
Nhi Pham


Re: SPCR / SSDT generation in the DynamicTablesPkg

Jeff Brasen
 

To add a little more detail on what we were seeing our 16550 based serial has 4 byte spacing which the SPCR table is generated with correctly but then the dynamic table code creates a SSDT with the standard pnp hid/cid in the ssdt table which at least from my reading of the Linux driver looks like that only uses 1 byte spacing between registers. It is possible I missed something though.

Thanks,
Jeff

Get Outlook for Android<https://aka.ms/ghei36>

________________________________
From: Irene Park <ipark@nvidia.com>
Sent: Wednesday, November 18, 2020 1:16:10 AM
To: discuss@edk2.groups.io <discuss@edk2.groups.io>; Irene Park <ipark@nvidia.com>; Sami.Mujawar@arm.com <Sami.Mujawar@arm.com>; pierre.gondois@arm.com <pierre.gondois@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; Jeff Brasen <jbrasen@nvidia.com>
Subject: RE: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene


Re: SPCR / SSDT generation in the DynamicTablesPkg

Irene Park
 

Hi Sami, Pierre, Alexei,
I wonder your thought about this topic.
Thank you,
Irene

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Irene Park
Sent: Tuesday, November 17, 2020 3:28 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] SPCR / SSDT generation in the DynamicTablesPkg

External email: Use caution opening links or attachments


The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene


SPCR / SSDT generation in the DynamicTablesPkg

Irene Park
 

The latest patches to the DynamicTablesPkg help an SSDT generated to meet the SBBR requirement when an SPCR generation is desired.
But the auto-generated SSDT might be unable to describe the compatible but custom 16550 device on the non-SBBR compliant platform.
I wonder if an SSDT generation would be manageable when a user doesn't want to.

Thank you,
Irene


SPCR / SSDT generation in the DynamicTablesPkg

Irene Park
 

The latest patches to DynamicTablesPkg help generate an SSDT to meet the SBBR specification when an SPCR generation is desired.
DynamicTablesPkg is useful for the non-SBBR compliant target to support the ACPI table generation, but this auto-generated SSDT might be unable to describe a compatible but custom 16650 device.
I wonder if this SSDT generation can be manageable when a user doesn't want to.


[Question] Using hii to display any size of font string

Liu Xin
 

Hello,



I use HiiAddPackages add a font data to hii, it can display characters by
StringToImage.

But when set the font size by using StringToImage, input
EFI_FONT_DISPLAY_INFO,

the characters are not displayed in the correct size,

is there any sample about changing font size?



EFI_FONT_DISPLAY_INFO *fontDisplayInfo;



fontDisplayInfo = (EFI_FONT_DISPLAY_INFO
*)AllocateZeroPool(sizeof(EFI_FONT_DISPLAY_INFO) + StrLen((const
CHAR16*)L"A") * 2 + 2);

fontDisplayInfo->ForegroundColor = *fontForegroundColor;

fontDisplayInfo->BackgroundColor = *fontBackgroundColor;



fontDisplayInfo->FontInfoMask = EFI_FONT_INFO_SYS_FORE_COLOR |
EFI_FONT_INFO_SYS_BACK_COLOR | EFI_FONT_INFO_RESIZE;

fontDisplayInfo->FontInfo.FontStyle = EFI_HII_FONT_STYLE_NORMAL;

fontDisplayInfo->FontInfo.FontSize = FontSize; //50

CopyMem(fontDisplayInfo->FontInfo.FontName, (const
CHAR16*)L"FZLKSXSJW", StrLen((const CHAR16*)L"A") * 2 + 2);



gSystemFrameBuffer.Width =
(UINT16)gGraphicsOutput->Mode->Info->HorizontalResolution;

gSystemFrameBuffer.Height =
(UINT16)gGraphicsOutput->Mode->Info->VerticalResolution;

gSystemFrameBuffer.Image.Screen = gGraphicsOutput;



Status = gHiiFont->StringToImage(

gHiiFont,

EFI_HII_IGNORE_IF_NO_GLYPH |

EFI_HII_DIRECT_TO_SCREEN | EFI_HII_OUT_FLAG_TRANSPARENT,

String,

fontDisplayInfo,

&pSystemFrameBuffer,

(UINTN)x,

(UINTN)y,

0,

0,

0

);


Using hii to display any size of font string

Liu Xin
 

Using hii, add a new font, it can display string by StringToImage.
When i use EFI_FONT_DISPLAY_INFO to set the font size, the characters are not displayed in the correct size.
Is there any sample?

EFI_FONT_DISPLAY_INFO *fontDisplayInfo;

fontDisplayInfo = (EFI_FONT_DISPLAY_INFO *)AllocateZeroPool(sizeof(EFI_FONT_DISPLAY_INFO) + StrLen((const CHAR16*)L"A") * 2 + 2);
fontDisplayInfo->ForegroundColor = *fontForegroundColor;
fontDisplayInfo->BackgroundColor = *fontBackgroundColor;

fontDisplayInfo->FontInfoMask = EFI_FONT_INFO_SYS_FORE_COLOR | EFI_FONT_INFO_SYS_BACK_COLOR | EFI_FONT_INFO_RESIZE;
fontDisplayInfo->FontInfo.FontStyle = EFI_HII_FONT_STYLE_NORMAL;
fontDisplayInfo->FontInfo.FontSize = 50;
CopyMem(fontDisplayInfo->FontInfo.FontName, (const CHAR16*)L"A", StrLen((const CHAR16*)L"A") * 2 + 2);


Status = gHiiFont->StringToImage(
gHiiFont,
EFI_HII_IGNORE_IF_NO_GLYPH |
EFI_HII_DIRECT_TO_SCREEN | EFI_HII_OUT_FLAG_TRANSPARENT,
String,
fontDisplayInfo,
&pSystemFrameBuffer,
(UINTN)x,
(UINTN)y,
0,
0,
0}


Re: [edk2-devel] : Query regarding IsTextShdr inside Basetools

Ard Biesheuvel <ard.biesheuvel@...>
 

On 11/11/20 11:41 PM, Laszlo Ersek wrote:
On 11/11/20 23:40, Laszlo Ersek wrote:
Ard, Liming,

can you please take a look?

Thanks!
Laszlo
Darn, I used Liming's old email address. Correcting it now. Sorry!
Laszlo


On 11/10/20 14:07, Mukesh Ojha wrote:
Hi All,

I have a doubt about the check we have put inside IsTextShdr() .

STATIC
BOOLEAN
IsTextShdr (
  Elf_Shdr *Shdr
  )
{
  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
}


We are observing one issue where while generate EFI using GenFW in EDK2
because test/data section offset is different than calculated
mCoffSectionsOffset when scanning sections.
I run GenFW with a failure dll in my local after adding some logs into
GenFW. and found that “mCoffSectionsOffset” for data section seems not
to have expected value due to
“.note.gnu.property” size. Because compiled dll has “.note.gnu.property”
section with alloc flag and GenFW thinks that it’s a text section if
alloc flag is set.
So its size is added to the mCoffSectionsOffset.

Could you please give us an advice whether we can fix IsTextShdr()
function like below ?


--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -229,7 +229,7 @@ IsTextShdr (
   Elf_Shdr *Shdr
   )
{
-  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
+  return (BOOLEAN) ((Shdr->sh_flags & (SHF_EXECINSTR | SHF_WRITE |
SHF_ALLOC)) == (SHF_ALLOC | SHF_EXECINSTR));^

Was this ELF executable built using the GccBase.lds linker script? If so, we should fix it to disregard .note sections.

If you are not using GccBase.lds, I'm afraid you are simply in unsupported territory - there are too many assumptions in GenFw that are not guaranteed to hold for arbitrary ELF executables.

I don't think changing IsTextShdr() is the right approach here.


回复: [edk2-discuss] where can I get the record of Monthly Tianocore Community Meeting

gaoliming
 

TianoCore Community Meeting Minutes - November is sent to devel and announce mail thread.

Thanks
Liming

-----邮件原件-----
发件人: bounce+34241+444+4905953+8764802@groups.io
<bounce+34241+444+4905953+8764802@groups.io> 代表 wenyi,xie via
groups.io
发送时间: 2020年11月13日 14:51
收件人: discuss@edk2.groups.io
主题: [edk2-discuss] where can I get the record of Monthly Tianocore
Community Meeting

Hi,all
May I ask whether there is Community meeting recording or summary. If I
miss the meeting, where can I get the content of the meeting? Thank you!




where can I get the record of Monthly Tianocore Community Meeting

wenyi,xie
 

Hi,all
May I ask whether there is Community meeting recording or summary. If I miss the meeting, where can I get the content of the meeting? Thank you!


Re: [edk2-devel] : Query regarding IsTextShdr inside Basetools

Laszlo Ersek
 

On 11/11/20 23:40, Laszlo Ersek wrote:
Ard, Liming,

can you please take a look?

Thanks!
Laszlo
Darn, I used Liming's old email address. Correcting it now. Sorry!

Laszlo


On 11/10/20 14:07, Mukesh Ojha wrote:
Hi All,

I have a doubt about the check we have put inside IsTextShdr() .

STATIC
BOOLEAN
IsTextShdr (
  Elf_Shdr *Shdr
  )
{
  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
}


We are observing one issue where while generate EFI using GenFW in EDK2
because test/data section offset is different than calculated
mCoffSectionsOffset when scanning sections.
I run GenFW with a failure dll in my local after adding some logs into
GenFW. and found that “mCoffSectionsOffset” for data section seems not
to have expected value due to
“.note.gnu.property” size. Because compiled dll has “.note.gnu.property”
section with alloc flag and GenFW thinks that it’s a text section if
alloc flag is set.
So its size is added to the mCoffSectionsOffset.

Could you please give us an advice whether we can fix IsTextShdr()
function like below ?


--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -229,7 +229,7 @@ IsTextShdr (
   Elf_Shdr *Shdr
   )
{
-  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
+  return (BOOLEAN) ((Shdr->sh_flags & (SHF_EXECINSTR | SHF_WRITE |
SHF_ALLOC)) == (SHF_ALLOC | SHF_EXECINSTR));^


Thanks,
Mukesh





Re: [edk2-devel] : Query regarding IsTextShdr inside Basetools

Laszlo Ersek
 

Ard, Liming,

can you please take a look?

Thanks!
Laszlo

On 11/10/20 14:07, Mukesh Ojha wrote:
Hi All,

I have a doubt about the check we have put inside IsTextShdr() .

STATIC
BOOLEAN
IsTextShdr (
  Elf_Shdr *Shdr
  )
{
  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
}


We are observing one issue where while generate EFI using GenFW in EDK2
because test/data section offset is different than calculated
mCoffSectionsOffset when scanning sections.
I run GenFW with a failure dll in my local after adding some logs into
GenFW. and found that “mCoffSectionsOffset” for data section seems not
to have expected value due to
“.note.gnu.property” size. Because compiled dll has “.note.gnu.property”
section with alloc flag and GenFW thinks that it’s a text section if
alloc flag is set.
So its size is added to the mCoffSectionsOffset.

Could you please give us an advice whether we can fix IsTextShdr()
function like below ?


--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -229,7 +229,7 @@ IsTextShdr (
   Elf_Shdr *Shdr
   )
{
-  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) ==
SHF_ALLOC);
+  return (BOOLEAN) ((Shdr->sh_flags & (SHF_EXECINSTR | SHF_WRITE |
SHF_ALLOC)) == (SHF_ALLOC | SHF_EXECINSTR));^


Thanks,
Mukesh





Re: [edk2-devel] : Query regarding IsTextShdr inside Basetools

Mukesh Ojha
 

Apology for rushing into this.
Looking for a quick input on this.

-Mukesh

On 11/10/2020 6:37 PM, Mukesh Ojha wrote:
Hi All,

I have a doubt about the check we have put inside IsTextShdr() .

STATIC
BOOLEAN
IsTextShdr (
  Elf_Shdr *Shdr
  )
{
  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) == SHF_ALLOC);
}


We are observing one issue where while generate EFI using GenFW in EDK2 because test/data section offset is different than calculated mCoffSectionsOffset when scanning sections.
I run GenFW with a failure dll in my local after adding some logs into GenFW. and found that “mCoffSectionsOffset” for data ection seems not to have expected value due to
“.note.gnu.property” size. Because compiled dll has “.note.gnu.property” section with alloc flag and GenFW thinks that it’s a text section if alloc flag is set.
So its size is added to the mCoffSectionsOffset.

Could you please give us an advice whether we can fix IsTextShdr() function like below ?


--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -229,7 +229,7 @@ IsTextShdr (
   Elf_Shdr *Shdr
   )
{
-  return (BOOLEAN) ((Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) == SHF_ALLOC);
+  return (BOOLEAN) ((Shdr->sh_flags & (SHF_EXECINSTR | SHF_WRITE | SHF_ALLOC)) == (SHF_ALLOC | SHF_EXECINSTR));^


Thanks,
Mukesh



381 - 400 of 834