Date   

Re: 回复: [edk2-devel] A proposal to reduce incompatible case

Zhiguang Liu
 

Hi all,

 

I write a python script to do the work, including classify all the library instance, generating the including lib file.

In the attachment file, the first sheet includes all "Single-instance", which means in edk2 and edk2-platforms repo, only one inf files has the specified library name.

The second sheet includes all "Multi-Single-instance", which means in edk2 and edk2-platforms repo, for a specified module type and arch, this inf is a "Single-instance".

I think the library instance in the first two sheet can be extracted as a XXXLibs.dsc.inc

 

Here is the draft code

https://github.com/LiuZhiguang001/edk2/commits/extract_lib

 

Here is the source code of this tool

https://github.com/LiuZhiguang001/MyTool/tree/extract_lib

 

Please review the excel and the draft code.

If no comments, I will send out a formal patch next week.

 

Thanks

Zhiguang

 

> -----Original Message-----

> From: Liu, Zhiguang

> Sent: Saturday, November 21, 2020 2:08 PM

> To: Laszlo Ersek <lersek@...>; Ard Biesheuvel

> <ard.biesheuvel@...>; gaoliming <gaoliming@...>;

> devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>; Feng, Bob C

> <bob.c.feng@...>; Tian, Hot <hot.tian@...>; 'Bret Barkelew'

> <bret@...>

> Cc: Bi, Dandan <dandan.bi@...>; 'Chao Zhang'

> <chao.b.zhang@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao A

> <hao.a.wu@...>; 'Liming Gao' <liming.gao@...>; Justen, Jordan L

> <jordan.l.justen@...>; 'Andrew Fish' <afish@...>; Ni, Ray

> <ray.ni@...>; 'Bret Barkelew' <brbarkel@...>; Kinney,

> Michael D <michael.d.kinney@...>; debtech@...;

> awarkentin@...; michael.kubacki@...

> Subject: RE: 回复: [edk2-devel] A proposal to reduce incompatible case

>

> Hi all,

> Thanks all for the comments.

>

> Hi Jiewen:

> I think we are thinking from the different aspects.

> I want the XXPkgLib.dsc.inc to specify the default library instance that the

> package will consumes.

> You may want it to specify the default library instance that the package will

> produce.

> I now think your way makes more sense, and also align with the implement in

> NetworkPkg.

> I will follow your way when working on the demo code.

>

> Hi Bret:

> I see you point about that platform should be like platform and should only

> change in the stable tag.

> I partly agree with you, but in fact there are some projects need to keep the

> Edk2 updated frequently.

> And my proposal should also be an optional way to add the library instance.

> Platform dsc can also choose the original way. I think it is at least harmless if

> some platforms choose not to use this way.

>

> Hi Lazlo:

> I agree with you that this proposal should only extract "Single-instance".

> After all, this proposal is meant to reduce incompatible case like this

> VaribalePolicyLib.

> I want to the platform to be "Seamless update" in such case.

> However, if this lib has two instances, it is a platform's choice and platform

> owner should be aware the change.

> Therefore, "Single-instance" and "Multiple-instance" should be totally different

> case, and we should only enable this proposal to "Single-instance".

>

> Hi Liming and Ard,

> Thanks for liking this idea.

> I plan to work on the demo code next week and send it here for more

> comments.

>

> BTW, about the file name, I want to it to follow the existing rule of NetworkPkg

> and ArmVirtPkg.

> I think for MdeModulePkg, "MdeModuleLibs.dsc.inc" will be better.

>

> Thanks

> Zhiguang

>

> > -----Original Message-----

> > From: Laszlo Ersek <lersek@...>

> > Sent: Friday, November 20, 2020 7:18 PM

> > To: Ard Biesheuvel <ard.biesheuvel@...>; gaoliming

> > <gaoliming@...>; devel@edk2.groups.io; Yao, Jiewen

> > <jiewen.yao@...>; Liu, Zhiguang <zhiguang.liu@...>;

> > michael.kubacki@...; awarkentin@...;

> debtech@...;

> > Feng, Bob C <bob.c.feng@...>; Tian, Hot <hot.tian@...>

> > Cc: 'Bret Barkelew' <bret@...>; Bi, Dandan

> > <dandan.bi@...>; 'Chao Zhang' <chao.b.zhang@...>; Wang,

> > Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>;

> > 'Liming Gao' <liming.gao@...>; Justen, Jordan L

> > <jordan.l.justen@...>; 'Andrew Fish' <afish@...>; Ni, Ray

> > <ray.ni@...>; 'Bret Barkelew' <brbarkel@...>; Kinney,

> > Michael D <michael.d.kinney@...>

> > Subject: Re: 回复: [edk2-devel] A proposal to reduce incompatible case

> >

> > On 11/20/20 10:02, Ard Biesheuvel wrote:

> > > On 11/20/20 8:27 AM, gaoliming wrote:

> > >> Zhiguang:

> > >>    This proposal can reduce the potential library class dependency.

> > >> Each package has its xxxPkgLib.dsc.inc file that includes the

> > >> library instances from this package.

> > >>    Platform DSC can include the required package lib.dsc.inc file

> > >> for the library instances. Can you work out the code changes to

> > >> demonstrate this idea?

> > >>

> > >

> > > +1 for this idea. This would allow us to remove a *lot* of

> > > +boilerplate

> > > in the .DSC files, and focus on the libraries that actually matter

> > > for the platform at hand.

> >

> > I feel like I'm in *cautious* agreement with this idea.

> >

> > In particular, I'd *only* like those lib class -> lib instance

> > resolutions to be extracted, to DSC include files, that *cannot*

> > involve platform choice. To put it differently, single-instance lib classes.

> >

> > ("Single-instance" can be interpreted per module type / per

> > architecture of course -- so if a lib class has exactly 1 instance for

> > PEIMs and exactly 1

> > (different) instance for DXE_DRIVERs, then those resolutions qualify

> > for

> > extraction.)

> >

> > If a library class genuinely has multiple instances in core edk2, then

> > extracting a "default" resolution would be *wrong*, in my opinion.

> > Every platform needs to make the choice explicitly, in such cases.

> > This applies even if a lib class only has two instances, a functional

> > one, and a Null one. The functional one should not be assumed default.

> >

> > Example: extracting the UefiLib resolution is valid. Extracting a

> > SerialPortLib class resolution is invalid.

> >

> > Basically, I'd like to avoid *overrides*. Overrides are terribly hard

> > to reason about.

> >

> > ... If someone wants to work on this, they'll have to implement this

> > with *super

> > small* patches, where every patch extracts resolutions for just one

> > lib class. I would reject any patch that required me to review the

> > extraction of two or more lib classes, from an OVMF or ArmVirt DSC file, at

> the same time.

> >

> > There is big potential in this proposal for making platform DSC files

> > *permanently* more difficult to understand & analyze. I don't

> > immediately see this proposal as a good solution for avoiding the

> > current kind of platform DSC breakage. Platform CI would be much

> > better, in my opinion. The proposal does seem workable, but the

> implementation needs to be surgical, IMO.

> >

> > OK, I'll get off my soap box now.

> >

> > Thanks

> > Laszlo

 


Re: [PATCH v1 0/3] MdeModulePkg/GraphicsConsole : Console cleanup

Samer El-Haj-Mahmoud
 

Any reviews for this series please? 

Thanks, 
Samer 


From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Samer El-Haj-Mahmoud via groups.io <samer.el-haj-mahmoud@...>
Sent: Tuesday, November 24, 2020, 2:20 PM
To: devel@edk2.groups.io
Cc: Jian J Wang; Hao A Wu; Zhichao Gao; Ray Ni; Ard Biesheuvel; Pete Batard
Subject: [edk2-devel] [PATCH v1 0/3] MdeModulePkg/GraphicsConsole : Console cleanup

This series fixes a few minor issues in the GraphicsConsole
and ConSplitter

 1. Remove extra cursor on graphics console during boot logo
 2. Fix misc spelling mistakes
 3. Change StdErr color from MAGENTA to LIGHTGRAY

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Zhichao Gao <zhichao.gao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>
Cc: Pete Batard <pete@...>
Signed-off-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>

Samer El-Haj-Mahmoud (3):
  MdeModulePkg/GraphicsConsoleDxe: Change default CursorVisible to FALSE
  MdeModulePkg/Graphics: Fix spelling mistakes
  MdeModulePkg/ConSplitter: Change StdErr color to EFI_LIGHTGRAY

 MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.h |  8 +--
 MdePkg/Include/Protocol/SimpleTextOut.h                             |  6 +-
 MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c         | 68 ++++++++++----------
 MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsole.c | 14 ++--
 4 files changed, 48 insertions(+), 48 deletions(-)

--
2.25.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67905): https://edk2.groups.io/g/devel/message/67905
Mute This Topic: https://groups.io/mt/78484572/1945644
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-mahmoud@...]
-=-=-=-=-=-=



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: [PATCH v1 1/1] ArmPkg/ArmGicDxe: fix writes to GICD_IPRIORITYR<n> when ARE enable

Quan Nguyen
 

No, Ard,
ArmGicDisableInterrupt() does not access these registers (IPRIORITYR)

-Quan

From: Ard Biesheuvel <ard.biesheuvel@...>
Date: Friday, November 27, 2020 at 14:17
To: Quan Nguyen OS <quan@...>, Leif Lindholm <leif@...>, devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Review <OpenSourceReview@...>, Victor Gallardo OS <Victor@...>
Subject: Re: [PATCH v1 1/1] ArmPkg/ArmGicDxe: fix writes to GICD_IPRIORITYR<n> when ARE enable
On 10/28/20 2:21 AM, Quan Nguyen wrote:
According to ARM doc IHI 0069F, section 11.9.18, "Accessing the
GICD_IPRIORITYR<n>:", "These registers are always used when affinity
routing is not enabled. When affinity routing is enabled for the
Security state of an interrupt:
   * GICR_IPRIORITYR<n> is used instead of GICD_IPRIORITYR<n> where n = 0
to 7 (that is, for SGIs and PPIs)."

The current ArmGicV3 code tries to initialize all the IPRIORITYR
registers to a default state via the Distributor (GICD), so skip
writes to the first eight IPRIORITYR registers when Affinity Routing
is Enabled.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Victor Gallardo <Victor@...>
Signed-off-by: Quan Nguyen <quan@...>
Isn't this already being taken into account in ArmGicDisableInterrupt()?

---
   ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 11 +++++++++++
   1 file changed, 11 insertions(+)

diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
index d7da1f198d9e..bc543502481b 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
+++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
@@ -378,6 +378,7 @@ GicV3DxeInitialize (
     UINTN                   RegShift;
     UINT64                  CpuTarget;
     UINT64                  MpId;
+  BOOLEAN                 AffinityRoutingEnabled = FALSE;
  
     // Make sure the Interrupt Controller Protocol is not already installed in
     // the system.
@@ -391,11 +392,21 @@ GicV3DxeInitialize (
     // Routing enabled. So ensure that the ARE bit is set.
     if (!FeaturePcdGet (PcdArmGicV3WithV2Legacy)) {
       MmioOr32 (mGicDistributorBase + ARM_GIC_ICDDCR, ARM_GIC_ICDDCR_ARE);
+    // If Affinity Routing is Enabled, the first 32 interrupts (SGI and PPI)
+    // can be programmed only through Redistributor interface (GICR).
+    // Initializing the GICD_IPRIORITYR registers for these interrupts can be
+    // skipped as the Redistributor will be powered up and initialized
+    // at the appropriate time (e.g. in EL3 by trusted firmware).
+    AffinityRoutingEnabled = TRUE;
     }
  
     for (Index = 0; Index < mGicNumInterrupts; Index++) {
       GicV3DisableInterruptSource (&gHardwareInterruptV3Protocol, Index);
  
+    if (AffinityRoutingEnabled && Index < 32) {
+      continue;
+    }
+
       // Set Priority
       RegOffset = Index / 4;
       RegShift = (Index % 4) * 8;


Re: [PATCH EDK2 v2 1/1] OvmfPkg/XenPvBlkDxe: add return value if allocting fail

Laszlo Ersek
 

On 11/24/20 10:19, wenyi,xie via groups.io wrote:
It's OK to me.

Thanks
Wenyi
Merged as commit f69a2b9a4202, via
<https://github.com/tianocore/edk2/pull/1151>.

Thanks
Laszlo

On 2020/11/24 17:16, Laszlo Ersek wrote:
On 11/24/20 03:06, Wenyi Xie wrote:
return EFI_OUT_OF_RESOURCES if pool allocating fail.

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Julien Grall <julien@...>
Signed-off-by: Wenyi Xie <@xiewenyi>
---
OvmfPkg/XenPvBlkDxe/BlockFront.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/OvmfPkg/XenPvBlkDxe/BlockFront.c b/OvmfPkg/XenPvBlkDxe/BlockFront.c
index 122a6baed25a..32d70c36cef2 100644
--- a/OvmfPkg/XenPvBlkDxe/BlockFront.c
+++ b/OvmfPkg/XenPvBlkDxe/BlockFront.c
@@ -155,6 +155,10 @@ XenPvBlockFrontInitialization (
ASSERT (NodeName != NULL);

Dev = AllocateZeroPool (sizeof (XEN_BLOCK_FRONT_DEVICE));
+ if (Dev == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
Dev->Signature = XEN_BLOCK_FRONT_SIGNATURE;
Dev->NodeName = NodeName;
Dev->XenBusIo = XenBusIo;
Reviewed-by: Laszlo Ersek <lersek@...>

Unless it's urgent for you, I'd like to merge this after
edk2-stable202011. It's a bugfix alright, but the issue seems to have
existed for a long time, with presumably no symptoms for most users.

Thanks
Laszlo

.




Re: [PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables

Leif Lindholm
 

On Fri, Nov 27, 2020 at 15:44:04 +0100, Ard Biesheuvel wrote:
As it turns out, the DeveloperBox platform never described its Ethernet
PHY mode correctly: the 'rgmii' value it exposes to the OS was inherited
from the SynQuacer evaluation board, which uses a different PHY, and the
Realtek PHY used on DeveloperBox is integrated on the board with straps
that configure it to 'rgmii-id' mode.

We never noticed because the Realtek PHY driver in Linux ignored the PHY
mode to begin with, and simply used the configuration that was active at
boot. Unfortunately, that has changed, and recent versions of the Linux
kernel (including stable releases) will now honour the firmware provided
PHY mode, and therefore configure the PHY incorrectly on these boards,
resulting in loss of network connectivity.

For ACPI boot, we can fix this by just setting the PHY mode to the empty
string - the Linux driver will be updated (and the change backported) to
ignore it anyway, as ACPI boot implies rich firmware, and it is reasonable
to assume that the PHY will be configured before the OS boots.

For DT, let's fix the description instead. This involves moving the
'phy-mode' property out of the shared .dtsi, as the change should only
apply to DeveloperBox, not to the SynQuacer evaluation board.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
Reviewed-by: Leif Lindholm <leif@...>

---
Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts | 4 ++++
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 3 +--
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 ++++
4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index bca484763d2c..3fecc570498b 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -170,7 +170,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
- Package (2) { "phy-mode", "rgmii" },
+ Package (2) { "phy-mode", "" },
Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
Package (2) { "max-speed", 1000 },
Package (2) { "max-frame-size", 9000 },
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
index 47ac27109929..c9bd436f745c 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
@@ -44,6 +44,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};

+&netsec {
+ phy-mode = "rgmii-id";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@7 {
compatible = "ethernet-phy-ieee802.3-c22";
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 2ee3821fca0b..ad418bf292db 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -418,14 +418,13 @@
#clock-cells = <0>;
};

- ethernet@522d0000 {
+ netsec: ethernet@522d0000 {
compatible = "socionext,synquacer-netsec";
reg = <0 0x522d0000 0x0 0x10000>,
<0 FixedPcdGet32 (PcdNetsecEepromBase) 0x0 0x10000>;
interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk_netsec>;
clock-names = "phy_ref_clk";
- phy-mode = "rgmii";
max-speed = <1000>;
max-frame-size = <9000>;
phy-handle = <&phy_netsec>;
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
index f437ee4cccf1..013a3a617748 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
@@ -24,6 +24,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};

+&netsec {
+ phy-mode = "rgmii";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
--
2.17.1


Re: Cannot boot ISO on RamDisk

Laszlo Ersek
 

On 11/26/20 20:53, Nahim Souza via groups.io wrote:
Greetings!

We are trying to implement an .efi application that receives an ISO file from an Android device, loads it into a buffer and try to boot an Operating System. To do this, we tested some implementations using Ram Disk but after creating the virtual disk and running BOOTX64.efi and found on the FAT32 partition, but some problems occur after boot, as if I did not find the other files in the ISO. To solve this problem, we tried some approaches:

* Change the parameters when calling RamDiskProtocol->Register() function (GUID and DevicePath)
* Tried with different OS (Xubuntu, Windows 10, Windows 7, Ubuntu and OpenSuse)
* Compared with HttpBoot implementation from EDK2, where we saw that an ISO file could be loaded into memory to boot an OS, but the RamDisk implementation in HttpBootRegisterRamDisk was very similar to ours
Based on this, I have some questions:

* In our understanding, HttpBoot downloads the ISO and boots the OS using the RamDisk. Is that correct?
I think so.

https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-disk-boot-from-http

* We saw that RamDiskDxe has some dependencies from ACPI tables, since it uses NFIT (NVDIMM Firmware Interface Table) to save some persistent information. Is there some hardware/driver requirement to make the OS boot through RamDisk? Did we need to have this NVDIMM to support ACPI feature from RamDiskDxe?
The booted operating system needs to understand where to look for files
on the virtual disk. More precisely, the booted operating system must
understand that the particular reserved memory range *is* a RAM disk. If
there is no NFIT, the booted operating system will have no idea where to
load other files from.

A similar issue is discussed in this RHBZ:

https://bugzilla.redhat.com/show_bug.cgi?id=1671345

There, the problem was that the guest kernel in question didn't have a
driver for consuming the NFIT table. Different reason, same result --
the information describing the UEFI-originated virtual disk to the OS
was lost, so the OS couldn't finish booting.

* Would you have any other suggestions for solving this scenario?
Make sure you have enough (contiguous) RAM, as the ISO can be large, and
you still need enough RAM left after the reserved memory allocation, for
booting the OS.

https://github.com/tianocore/tianocore.github.io/wiki/HTTP-Boot#ram-disk-image-size

HTH
Laszlo


Re: [PATCH v2 0/3] various bhyve tweaks and updates

Laszlo Ersek
 

On 11/24/20 02:19, Rebecca Cran wrote:
On 11/23/20 6:04 PM, Laszlo Ersek wrote:

Can this whole set wait until after edk2-stable202011, or would you like
me to merge patch#1 ("OvmfPkg/Bhyve: Add VariablePolicy engine to
Bhyve") in isolation, during the Hard Feature Freeze? (It definitely
qualifies, because it fixes a build regression caused by the patches for
TianoCore#2522.)

I don't think we should merge patches #2 and #3 during the HFF.
I'm not sure it makes sense to have patch #1 without #3 to make it
usable on AMD, so let's leave the whole set until after the stable tag.
We can leave bhyve users on 2014.SP1 firmware a few days longer :)

Series merged as commit range 872f953262d6..73b604bb1e13, via
<https://github.com/tianocore/edk2/pull/1150>.

Thanks!
Laszlo


Cannot boot ISO on RamDisk

Nahim Souza
 

Greetings!

We are trying to implement an .efi application that receives an ISO file from an Android device, loads it into a buffer and try to boot an Operating System. To do this, we tested some implementations using Ram Disk but after creating the virtual disk and running BOOTX64.efi and found on the FAT32 partition, but some problems occur after boot, as if I did not find the other files in the ISO. To solve this problem, we tried some approaches:

  1. Change the parameters when calling RamDiskProtocol->Register() function (GUID and DevicePath)
  2. Tried with different OS (Xubuntu, Windows 10, Windows 7, Ubuntu and OpenSuse)
  3. Compared with HttpBoot implementation from EDK2, where we saw that an ISO file could be loaded into memory to boot an OS, but the RamDisk implementation in HttpBootRegisterRamDisk was very similar to ours
Based on this, I have some questions:

  1. In our understanding, HttpBoot downloads the ISO and boots the OS using the RamDisk. Is that correct?
  2. We saw that RamDiskDxe has some dependencies from ACPI tables, since it uses NFIT (NVDIMM Firmware Interface Table) to save some persistent information. Is there some hardware/driver requirement to make the OS boot through RamDisk? Did we need to have this NVDIMM to support ACPI feature from RamDiskDxe?
  3. Would you have any other suggestions for solving this scenario?

Best regards!
Nahim.


Re: [edk2-platforms] [PATCH v4] Platform/ARM/SgiPkg: Fix constant-logical-operand clang error

Ard Biesheuvel
 

On 11/27/20 3:39 PM, Vijayenthiran Subramaniam wrote:
Fix "use of logical '&&' with constant operand" error when built with
clang.
Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@...>
---
Changes since v3:
- Update commit message.
- Remove comparing boolean expression with TRUE.
- v3 discussion: https://edk2.groups.io/g/devel/topic/78500293
Changes since v2:
- Reviewed-by added from:
https://edk2.groups.io/g/devel/topic/71391950#55868
- Rebased to latest master and repost.
Changes since v1:
- Fix Copyright year
Note:
Fix Clang build error reported by Leif in
https://edk2.groups.io/g/devel/message/54586
Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@...>

Pushed as f182372f928f..3f71a8fb114a

Thanks!


diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
index 9e5f7e70..5cf8f6a7 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2018-2020, Arm Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -82,7 +82,7 @@ InitVirtioDevices (
// Install protocol interface for storage device
if ((FeaturePcdGet (PcdVirtioBlkSupported)) &&
- (FixedPcdGet32 (PcdVirtioBlkBaseAddress))) {
+ (FixedPcdGet32 (PcdVirtioBlkBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoBlkController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioBlockDevicePath);
@@ -111,7 +111,7 @@ InitVirtioDevices (
// Install protocol interface for network device
if ((FeaturePcdGet (PcdVirtioNetSupported)) &&
- (FixedPcdGet32 (PcdVirtioNetBaseAddress))) {
+ (FixedPcdGet32 (PcdVirtioNetBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoNetController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioNetDevicePath);


[PATCH edk2-platforms 1/1] Silicon/SynQuacer: set PHY mode as appropriate in ACPI and DT tables

Ard Biesheuvel
 

As it turns out, the DeveloperBox platform never described its Ethernet
PHY mode correctly: the 'rgmii' value it exposes to the OS was inherited
from the SynQuacer evaluation board, which uses a different PHY, and the
Realtek PHY used on DeveloperBox is integrated on the board with straps
that configure it to 'rgmii-id' mode.

We never noticed because the Realtek PHY driver in Linux ignored the PHY
mode to begin with, and simply used the configuration that was active at
boot. Unfortunately, that has changed, and recent versions of the Linux
kernel (including stable releases) will now honour the firmware provided
PHY mode, and therefore configure the PHY incorrectly on these boards,
resulting in loss of network connectivity.

For ACPI boot, we can fix this by just setting the PHY mode to the empty
string - the Linux driver will be updated (and the change backported) to
ignore it anyway, as ACPI boot implies rich firmware, and it is reasonable
to assume that the PHY will be configured before the OS boots.

For DT, let's fix the description instead. This involves moving the
'phy-mode' property out of the shared .dtsi, as the change should only
apply to DeveloperBox, not to the SynQuacer evaluation board.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl | 2 +-
Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts | 4 ++++
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 3 +--
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts | 4 ++++
4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index bca484763d2c..3fecc570498b 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -170,7 +170,7 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
- Package (2) { "phy-mode", "rgmii" },
+ Package (2) { "phy-mode", "" },
Package (2) { "phy-channel", FixedPcdGet32 (PcdNetsecPhyAddress) },
Package (2) { "max-speed", 1000 },
Package (2) { "max-frame-size", 9000 },
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
index 47ac27109929..c9bd436f745c 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/DeveloperBox.dts
@@ -44,6 +44,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};

+&netsec {
+ phy-mode = "rgmii-id";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@7 {
compatible = "ethernet-phy-ieee802.3-c22";
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 2ee3821fca0b..ad418bf292db 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -418,14 +418,13 @@
#clock-cells = <0>;
};

- ethernet@522d0000 {
+ netsec: ethernet@522d0000 {
compatible = "socionext,synquacer-netsec";
reg = <0 0x522d0000 0x0 0x10000>,
<0 FixedPcdGet32 (PcdNetsecEepromBase) 0x0 0x10000>;
interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clk_netsec>;
clock-names = "phy_ref_clk";
- phy-mode = "rgmii";
max-speed = <1000>;
max-frame-size = <9000>;
phy-handle = <&phy_netsec>;
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
index f437ee4cccf1..013a3a617748 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacerEvalBoard.dts
@@ -24,6 +24,10 @@
"PEC-PD28", "PEC-PD29", "PEC-PD30", "PEC-PD31";
};

+&netsec {
+ phy-mode = "rgmii";
+};
+
&mdio_netsec {
phy_netsec: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
--
2.17.1


[edk2-platforms] [PATCH v4] Platform/ARM/SgiPkg: Fix constant-logical-operand clang error

Vijayenthiran Subramaniam
 

Fix "use of logical '&&' with constant operand" error when built with
clang.

Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@...>
---

Changes since v3:
- Update commit message.
- Remove comparing boolean expression with TRUE.
- v3 discussion: https://edk2.groups.io/g/devel/topic/78500293

Changes since v2:
- Reviewed-by added from:
https://edk2.groups.io/g/devel/topic/71391950#55868
- Rebased to latest master and repost.

Changes since v1:
- Fix Copyright year

Note:
Fix Clang build error reported by Leif in
https://edk2.groups.io/g/devel/message/54586

Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
index 9e5f7e70..5cf8f6a7 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2018-2020, Arm Limited. All rights reserved.

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -82,7 +82,7 @@ InitVirtioDevices (

// Install protocol interface for storage device
if ((FeaturePcdGet (PcdVirtioBlkSupported)) &&
- (FixedPcdGet32 (PcdVirtioBlkBaseAddress))) {
+ (FixedPcdGet32 (PcdVirtioBlkBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoBlkController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioBlockDevicePath);
@@ -111,7 +111,7 @@ InitVirtioDevices (

// Install protocol interface for network device
if ((FeaturePcdGet (PcdVirtioNetSupported)) &&
- (FixedPcdGet32 (PcdVirtioNetBaseAddress))) {
+ (FixedPcdGet32 (PcdVirtioNetBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoNetController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioNetDevicePath);
--
2.17.1


Re: [PATCH EDK2-platform v1 1/1] Maintainers.txt: add reviewer to HiSilicon

Leif Lindholm
 

On Fri, Nov 27, 2020 at 09:05:49 +0800, Wenyi Xie wrote:
Add myself to Maintainers.txt as a Hisilicon reviewer.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Wenyi Xie <@xiewenyi>
Thanks!
Reviewed-by: Leif Lindholm <leif@...>
Pushed as f182372f928f.

---
Maintainers.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 26b9959bc879..da90c8a11452 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -125,6 +125,7 @@ F: Platform/Hisilicon/
F: Silicon/Hisilicon/
M: Leif Lindholm <leif@...>
R: Ard Biesheuvel <ard.biesheuvel@...>
+R: Wenyi Xie <@xiewenyi>

Features/Intel
F: Features/Intel/
--
2.20.1.windows.1


Re: [PATCH edk2-platforms 06/17] Silicon/Socionext: fix unused variable error NorFlashFvb

Leif Lindholm
 

On Fri, Nov 27, 2020 at 12:42:46 +0100, Ard Biesheuvel wrote:
On 11/27/20 12:31 PM, Leif Lindholm wrote:
When building in NOOPT mode, SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
fails to build with -Werror=unused-but-set-variable due to the Instance
variable being set from the This pointer but never actually referenced.

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Leif Lindholm <leif@...>
Acked-by: Ard Biesheuvel <ard.biesheuvel@...>
Thanks!

Series pushed as 734fed7db671..abd92d65cf3b, with FV size patch at the
end as previously mentioned.

/
Leif

---

Submitting as a single additional patch rather than a full v2.
(Patch 17 is Ard's new one increasing the FV size for DeveloperBoxMm.)

Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
index 387789fb9782..48774e955faf 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
@@ -209,9 +209,6 @@ FvbGetAttributes(
)
{
EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes;
- NOR_FLASH_INSTANCE *Instance;
-
- Instance = INSTANCE_FROM_FVB_THIS(This);
FlashFvbAttributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS |
EFI_FVB2_WRITE_ENABLED_CAP | EFI_FVB2_WRITE_STATUS |


Re: [edk2-platforms] [PATCH v3] Platform/ARM/SgiPkg: Fix constant-logical-operand clang error

Vijayenthiran Subramaniam
 

Hi Leif,

On Wed, Nov 25, 2020 at 5:58 PM Leif Lindholm <leif@...> wrote:

On Wed, Nov 25, 2020 at 20:00:30 +0530, Vijayenthiran Subramaniam wrote:
Fix "use of logical '&&' with constant operand" error when built with
CLANG38 toolchain.
CLANG38 is the toolchain profile in the build system, it is highly
unlikely you are actually building with clang 3.8.
clang --version tells you the toolchain *version*.
I will update the commit message in v3.

Thanks,
Vijay

Reviewed-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@...>
---

Changes since v2:
- Reviewed-bys added from:
https://edk2.groups.io/g/devel/topic/71391950#55868
- Rebased to latest master and repost.

Changes since v1:
- Fix Copyright year

Note:
Fix Clan error reported by Leif in
https://edk2.groups.io/g/devel/message/54586

Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
index 9e5f7e70..f91724b9 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2018-2020, ARM Limited. All rights reserved.

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -81,8 +81,8 @@ InitVirtioDevices (
STATIC EFI_HANDLE mVirtIoNetController = NULL;

// Install protocol interface for storage device
- if ((FeaturePcdGet (PcdVirtioBlkSupported)) &&
- (FixedPcdGet32 (PcdVirtioBlkBaseAddress))) {
+ if ((FeaturePcdGet (PcdVirtioBlkSupported) == TRUE) &&
+ (FixedPcdGet32 (PcdVirtioBlkBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoBlkController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioBlockDevicePath);
@@ -110,8 +110,8 @@ InitVirtioDevices (
}

// Install protocol interface for network device
- if ((FeaturePcdGet (PcdVirtioNetSupported)) &&
- (FixedPcdGet32 (PcdVirtioNetBaseAddress))) {
+ if ((FeaturePcdGet (PcdVirtioNetSupported) == TRUE) &&
+ (FixedPcdGet32 (PcdVirtioNetBaseAddress) != 0)) {
Status = gBS->InstallProtocolInterface (&mVirtIoNetController,
&gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
&mVirtioNetDevicePath);
--
2.17.1




Re: [PATCH edk2-platforms 1/1] Platform/Socionext/DeveloperBoxMm: increase FV size

Ard Biesheuvel
 

On 11/27/20 12:50 PM, Leif Lindholm wrote:
On Thu, Nov 26, 2020 at 16:22:48 +0000, Leif Lindholm wrote:
On Thu, Nov 26, 2020 at 16:40:46 +0100, Ard Biesheuvel wrote:
Due to recent changes in core EDK2, the DeveloperBoxMM standalone MM
build no longer fits when built in DEBUG or NOOPT mode. So increase
the FD size a little bit - we still have some spare room available in
the FIP image.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
Reviewed-by: Leif Lindholm <leif@...>

I will push this with (after) the VariablePolicy set since the problem
doesn't actually trigger until then.
Urgh. This fixes DEBUG, but in my setup NOOPT is still too big.
Would it be OK if I folded in an increase from 0x50000 to 0x60000
before pushing?
Yeah that's fine.


/
Leif

/
Leif

---
Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
index 83a3c4660cb3..58efed89b26e 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
@@ -21,12 +21,12 @@
[FD.STANDALONE_MM]
BaseAddress = 0xfc000000
-Size = 0x00040000
+Size = 0x00050000
ErasePolarity = 1
# This one is tricky, it must be: BlockSize * NumBlocks = Size
BlockSize = 0x00010000
-NumBlocks = 0x4
+NumBlocks = 0x5
################################################################################
#
@@ -44,7 +44,7 @@ [FD.STANDALONE_MM]
#
################################################################################
-0x00000000|0x00040000
+0x00000000|0x00050000
FV = FvStandaloneMmCompact
################################################################################
--
2.17.1


Re: [PATCH edk2-platforms 1/1] Platform/Socionext/DeveloperBoxMm: increase FV size

Leif Lindholm
 

On Thu, Nov 26, 2020 at 16:22:48 +0000, Leif Lindholm wrote:
On Thu, Nov 26, 2020 at 16:40:46 +0100, Ard Biesheuvel wrote:
Due to recent changes in core EDK2, the DeveloperBoxMM standalone MM
build no longer fits when built in DEBUG or NOOPT mode. So increase
the FD size a little bit - we still have some spare room available in
the FIP image.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
Reviewed-by: Leif Lindholm <leif@...>

I will push this with (after) the VariablePolicy set since the problem
doesn't actually trigger until then.
Urgh. This fixes DEBUG, but in my setup NOOPT is still too big.
Would it be OK if I folded in an increase from 0x50000 to 0x60000
before pushing?

/
Leif

/
Leif

---
Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
index 83a3c4660cb3..58efed89b26e 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
@@ -21,12 +21,12 @@

[FD.STANDALONE_MM]
BaseAddress = 0xfc000000
-Size = 0x00040000
+Size = 0x00050000
ErasePolarity = 1

# This one is tricky, it must be: BlockSize * NumBlocks = Size
BlockSize = 0x00010000
-NumBlocks = 0x4
+NumBlocks = 0x5

################################################################################
#
@@ -44,7 +44,7 @@ [FD.STANDALONE_MM]
#
################################################################################

-0x00000000|0x00040000
+0x00000000|0x00050000
FV = FvStandaloneMmCompact

################################################################################
--
2.17.1


Re: [PATCH edk2-platforms 06/17] Silicon/Socionext: fix unused variable error NorFlashFvb

Ard Biesheuvel
 

On 11/27/20 12:31 PM, Leif Lindholm wrote:
When building in NOOPT mode, SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
fails to build with -Werror=unused-but-set-variable due to the Instance
variable being set from the This pointer but never actually referenced.
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Leif Lindholm <leif@...>
Acked-by: Ard Biesheuvel <ard.biesheuvel@...>

---
Submitting as a single additional patch rather than a full v2.
(Patch 17 is Ard's new one increasing the FV size for DeveloperBoxMm.)
Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
index 387789fb9782..48774e955faf 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
@@ -209,9 +209,6 @@ FvbGetAttributes(
)
{
EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes;
- NOR_FLASH_INSTANCE *Instance;
-
- Instance = INSTANCE_FROM_FVB_THIS(This);
FlashFvbAttributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS |
EFI_FVB2_WRITE_ENABLED_CAP | EFI_FVB2_WRITE_STATUS |


[PATCH edk2-platforms 06/17] Silicon/Socionext: fix unused variable error NorFlashFvb

Leif Lindholm
 

When building in NOOPT mode, SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
fails to build with -Werror=unused-but-set-variable due to the Instance
variable being set from the This pointer but never actually referenced.

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Signed-off-by: Leif Lindholm <leif@...>
---

Submitting as a single additional patch rather than a full v2.
(Patch 17 is Ard's new one increasing the FV size for DeveloperBoxMm.)

Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
index 387789fb9782..48774e955faf 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
@@ -209,9 +209,6 @@ FvbGetAttributes(
)
{
EFI_FVB_ATTRIBUTES_2 FlashFvbAttributes;
- NOR_FLASH_INSTANCE *Instance;
-
- Instance = INSTANCE_FROM_FVB_THIS(This);

FlashFvbAttributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS |
EFI_FVB2_WRITE_ENABLED_CAP | EFI_FVB2_WRITE_STATUS |
--
2.20.1


Re: [edk2-platforms] [PATCH v3] Platform/ARM/SgiPkg: Fix constant-logical-operand clang error

Ard Biesheuvel
 

On 11/27/20 11:45 AM, Vijayenthiran Subramanian wrote:
Hi Ard,
On Wed, Nov 25, 2020 at 3:05 PM Ard Biesheuvel <ard.biesheuvel@...> wrote:

On 11/25/20 3:30 PM, Vijayenthiran Subramaniam wrote:
Fix "use of logical '&&' with constant operand" error when built with
CLANG38 toolchain.

Reviewed-by: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@...>
---

Changes since v2:
- Reviewed-bys added from:
https://edk2.groups.io/g/devel/topic/71391950#55868
- Rebased to latest master and repost.

Changes since v1:
- Fix Copyright year

Note:
Fix Clan error reported by Leif in
https://edk2.groups.io/g/devel/message/54586

Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
index 9e5f7e70..f91724b9 100644
--- a/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
+++ b/Platform/ARM/SgiPkg/Drivers/PlatformDxe/VirtioDevices.c
@@ -1,6 +1,6 @@
/** @file

- Copyright (c) 2018, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2018-2020, ARM Limited. All rights reserved.

SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -81,8 +81,8 @@ InitVirtioDevices (
STATIC EFI_HANDLE mVirtIoNetController = NULL;

// Install protocol interface for storage device
- if ((FeaturePcdGet (PcdVirtioBlkSupported)) &&
- (FixedPcdGet32 (PcdVirtioBlkBaseAddress))) {
+ if ((FeaturePcdGet (PcdVirtioBlkSupported) == TRUE) &&
+ (FixedPcdGet32 (PcdVirtioBlkBaseAddress) != 0)) {
Nack

Please fix this by disabling this bogus warning in the compiler. The
original code is perfectly reasonable, and comparing a boolean
expression to TRUE could equally trigger a diagnostic in another
compiler for being redundant. (The != 0 check for a UINT32 is fine)
As you suggested, Boolean comparison with TRUE is not required for
CLANG as well. But != 0 check for UINT32 is required to fix the error while
building with the CLANG toolchain profile. Would it be ok if I post another
version with just != 0 check with UNT32?
Yes, that is fine.


Re: [Patch v2 1/1] BaseTools: Collect full Header files for struct finding.

Bob Feng
 

Reviewed-by: Bob Feng <bob.c.feng@...>

-----Original Message-----
From: Chen, Christine <yuwei.chen@...>
Sent: Tuesday, November 24, 2020 4:40 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@...>; Liming Gao <gaoliming@...>
Subject: [Patch v2 1/1] BaseTools: Collect full Header files for struct finding.

Currently, only parts of the Header files can be collected which caused some struct definition can not be found. To solve this issue, Header files full collection has been added in this file to support the struct finding.

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Signed-off-by: Yuwei Chen <yuwei.chen@...>
---
BaseTools/Scripts/ConvertFceToStructurePcd.py | 25 ++++++++++++++-----
1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Scripts/ConvertFceToStructurePcd.py b/BaseTools/Scripts/ConvertFceToStructurePcd.py
index 89e6a727a201..aeda3ff26dfe 100644
--- a/BaseTools/Scripts/ConvertFceToStructurePcd.py
+++ b/BaseTools/Scripts/ConvertFceToStructurePcd.py
@@ -370,7 +370,7 @@ class PATH(object):
def __init__(self,path):
self.path=path
self.rootdir=self.get_root_dir()
- self.usefuldir=[]
+ self.usefuldir=set()
self.lstinf = {}
for path in self.rootdir:
for o_root, o_dir, o_file in os.walk(os.path.join(path, "OUTPUT"), topdown=True, followlinks=False):
@@ -381,7 +381,7 @@ class PATH(object):
for LST in l_file:
if os.path.splitext(LST)[1] == '.lst':
self.lstinf[os.path.join(l_root, LST)] = os.path.join(o_root, INF)
- self.usefuldir.append(path)
+ self.usefuldir.add(path)

def get_root_dir(self):
rootdir=[]
@@ -410,7 +410,7 @@ class PATH(object):

def header(self,struct):
header={}
- head_re = re.compile('typedef.*} %s;[\n]+(.*?)(?:typedef|formset)'%struct,re.M|re.S)
+ head_re = re.compile('typedef.*}
+ %s;[\n]+(.*)(?:typedef|formset)'%struct,re.M|re.S)
head_re2 = re.compile(r'#line[\s\d]+"(\S+h)"')
for i in list(self.lstinf.keys()):
with open(i,'r') as lst:
@@ -421,9 +421,21 @@ class PATH(object):
if head:
format = head[0].replace('\\\\','/').replace('\\','/')
name =format.split('/')[-1]
- head = self.makefile(name).replace('\\','/')
- header[struct] = head
+ head = self.headerfileset.get(name)
+ if head:
+ head = head.replace('\\','/')
+ header[struct] = head
return header
+ @property
+ def headerfileset(self):
+ headerset = dict()
+ for root,dirs,files in os.walk(self.path):
+ for file in files:
+ if os.path.basename(file) == 'deps.txt':
+ with open(os.path.join(root,file),"r") as fr:
+ for line in fr.readlines():
+ headerset[os.path.basename(line).strip()] = line.strip()
+ return headerset

def makefile(self,filename):
re_format = re.compile(r'DEBUG_DIR.*(?:\S+Pkg)\\(.*\\%s)'%filename)
@@ -433,6 +445,7 @@ class PATH(object):
dir = re_format.findall(read)
if dir:
return dir[0]
+ return None

class mainprocess(object):

@@ -479,7 +492,7 @@ class mainprocess(object):
WARNING.append("Warning: No <HeaderFiles> for struct %s"%struct)
title2 = '%s%s|{0}|%s|0xFCD00000{\n <HeaderFiles>\n %s\n <Packages>\n%s\n}\n' % (PCD_NAME, c_name, struct, '', self.LST.package()[self.lst_dict[lstfile]])
header_list.append(title2)
- else:
+ elif struct not in lst._ignore:
struct_dict ={}
print("ERROR: Struct %s can't found in lst file" %struct)
ERRORMSG.append("ERROR: Struct %s can't found in lst file" %struct)
--
2.27.0.windows.1