Hi Laszlo,

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

Thanks
Wenyi

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

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

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.

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

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

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

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

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.

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, 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}

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.

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

Thanks
Liming

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!

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

Ard, Liming,

can you please take a look?

Thanks!
Laszlo

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

-Mukesh

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

On 11/10/20 6:01 AM, Laszlo Ersek wrote:

On 11/10/20 05:42, Sandeep Dhanvada wrote:

Hi Laszlo,

I am able to execute PatchCheck.py on a patch generated from git format-patch, but, could not execute this script with git commits i.e., using -1 or -2 as arguments to read git commits.

IIRC an explicit commit range should work. For example,

$ python3 BaseTools/Scripts/PatchCheck.py master..

Similarly, "python3 BaseTools/Scripts/PatchCheck.py HEAD~2" also works.


--
Rebecca Cran

On 11/10/20 05:42, Sandeep Dhanvada wrote:

Hi Laszlo,

I am able to execute PatchCheck.py on a patch generated from git format-patch, but, could not execute this script with git commits i.e., using -1 or -2 as arguments to read git commits.

IIRC an explicit commit range should work. For example,

$ python3 BaseTools/Scripts/PatchCheck.py master..

PatchCheck didn't show any Warnings or Errors on the patch file. so, i executed git send-email with my patches to devel@edk2.groups.io, but, could not see my patch set in edk2 devel mailing list, https://edk2.groups.io/g/devel/topics. do i need to do anything else to send the patch to devel mailing list ?

You need to be both subscribed (up to you) and unmoderated (up to a
moderator) for your postings to go through immediately. A few of us take
turns at moderating the list, there may be delays for first time posters
(even after subscription). Sorry about that.

Your series should now be visible to all list subscribers:
<https://edk2.groups.io/g/devel/message/67177>. (It wasn't me that let
it through, another moderator got to it first.)

Thanks
Laszlo