[edk2] [Patch] Vlv2TbltDevicePkg:Signal End of Dxe Event.

[lushifex]

According to PI spec,DxeSmmReadyToLock protocol is published immediately after 
signaling of the End of Dxe Event.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: lushifex 
---
 Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c 
b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
index 02e4616..e1f3524 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -179,10 +179,15 @@ InstallReadyToLock (
 EFI_NATIVE_INTERFACE,
 NULL
 );
 ASSERT_EFI_ERROR (Status);
 
+//
+// Signal EndOfDxe PI Event
+//
+EfiEventGroupSignal ();
+
 Handle = NULL;
 Status = gBS->InstallProtocolInterface (
 ,
 ,
 EFI_NATIVE_INTERFACE,
@@ -224,15 +229,10 @@ PlatformBdsInit (
   )
 {
   EFI_STATUS  Status;
   EFI_EVENT   ShellImageEvent;
   EFI_GUIDShellEnvProtocol = SHELL_ENVIRONMENT_INTERFACE_PROTOCOL;
-  
-  //
-  // Signal EndOfDxe PI Event
-  //
-  EfiEventGroupSignal ();
 
   #ifdef __GNUC__
   SerialPortWrite((UINT8 *)"BdsEntry[GCC]\r\n", 19);
   #else
   SerialPortWrite((UINT8 *)"BdsEntry\r\n", 14);
-- 
2.6.2.windows.1


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] BootableImageSupportTest\StorageSecurityCommandProtocolTest

[Jin, Eric]

For the TRUSTED RECEIVE commands of the ATA8-ACS command, 
in the ATA8-ACS spec, the total data length shall be 512 bytes. Pad bytes are 
appended as needed to meet this requirement. Pad bytes shall have a value of 
00h.

For the SECURITY PROTOCOL IN commands of the SPC-4 command,
In the SPC-4 spec, when INC_512 is 0, the ALLOCATION LENGTH field expresses the 
number of bytes to be transferred. It means any value.
If the length is larger than 8 bytes, the byte 6-7 indicate the SUPPORTED 
SECURITY PROTOCOL LIST LENGTH. If the length is larger than (SECURITY PROTOCOL 
LIST LENGTH + 8), all are returned and plus the pad data.


Best Regards
Eric

-Original Message-
From: Tian, Feng 
Sent: Thursday, September 1, 2016 10:42 AM
To: Ramesh R. ; edk2-devel ; Jin, 
Eric 
Cc: Tian, Feng 
Subject: RE: BootableImageSupportTest\StorageSecurityCommandProtocolTest

I checked the ATA spec, it says the transfer length of "Trust-Send" ATA cmd 
should be 512.

But for NVMe and other SCSI device, I didn't see any length limitation on 
"Security Protocol In" cmd with security protocol field 0 and security protocol 
specific field 0.

It seems user could pass in any length value to get security protocol 
information. And last, user could get the whole one by passing down "supported 
security protocol list length" + 8.

Ramesh, do you meet real failure case?

Eric, what's your opinion on this?

Thanks
Feng

-Original Message-
From: Ramesh R. [mailto:rame...@ami.com] 
Sent: Wednesday, August 31, 2016 1:20 AM
To: Tian, Feng ; edk2-devel ; 
Jin, Eric 
Subject: RE: BootableImageSupportTest\StorageSecurityCommandProtocolTest

Hi Feng,

  Any update or suggestion on this? Can we consider this as SCT tool issue and 
would be fixed in next version ?

Thanks,
Ramesh

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: 26 August 2016 12:54
To: Ramesh R.; edk2-devel; Jin, Eric
Cc: Tian, Feng
Subject: RE: BootableImageSupportTest\StorageSecurityCommandProtocolTest

Yes, I agree it's weird. 

We are looking at this and will get back to you if we have findings.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ramesh R.
Sent: Thursday, August 25, 2016 4:44 PM
To: edk2-devel 
Subject: [edk2] BootableImageSupportTest\StorageSecurityCommandProtocolTest

Hi,

   When the we run the 
"BootableImageSupportTest\StorageSecurityCommandProtocolTest" test on the NVME 
devices we are getting into error because of the below testing code.

//
// According to TCG definition, when the Security Protocol field is set to 
00h, and SP
// Specific is set to h in a TRUSTED RECEIVE command, return security 
protocol
// information. This Command is not associated with a security send command
//
Status = StorageSecurityCommand->ReceiveData (
   StorageSecurityCommand,
   BlockIo->Media->MediaId,
   1,// Timeout 
10-sec
   0,// 
SecurityProtocol
   0,// 
SecurityProtocolSpecifcData
   10,   // 
PayloadBufferSize,
   DataBuffer,   // 
PayloadBuffer
   
   );
//
// for ATA8-ACS SecurityProtocol, 512 byte is a request
//
if (IsAtaDevice) {
  if((Status == EFI_DEVICE_ERROR) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
} else {
  if((!EFI_ERROR(Status)) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
}

For Ata devices, EFI_DEVICE_ERROR considered as valid error case and for the 
Nvme ( Non ATA) device it's considered as error. Could you please let us know 
why there is difference in this case ?.

Thanks,
Ramesh


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] BootableImageSupportTest\StorageSecurityCommandProtocolTest

[Tian, Feng]

I checked the ATA spec, it says the transfer length of "Trust-Send" ATA cmd 
should be 512.

But for NVMe and other SCSI device, I didn't see any length limitation on 
"Security Protocol In" cmd with security protocol field 0 and security protocol 
specific field 0.

It seems user could pass in any length value to get security protocol 
information. And last, user could get the whole one by passing down "supported 
security protocol list length" + 8.

Ramesh, do you meet real failure case?

Eric, what's your opinion on this?

Thanks
Feng

-Original Message-
From: Ramesh R. [mailto:rame...@ami.com] 
Sent: Wednesday, August 31, 2016 1:20 AM
To: Tian, Feng ; edk2-devel ; 
Jin, Eric 
Subject: RE: BootableImageSupportTest\StorageSecurityCommandProtocolTest

Hi Feng,

  Any update or suggestion on this? Can we consider this as SCT tool issue and 
would be fixed in next version ?

Thanks,
Ramesh

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: 26 August 2016 12:54
To: Ramesh R.; edk2-devel; Jin, Eric
Cc: Tian, Feng
Subject: RE: BootableImageSupportTest\StorageSecurityCommandProtocolTest

Yes, I agree it's weird. 

We are looking at this and will get back to you if we have findings.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ramesh R.
Sent: Thursday, August 25, 2016 4:44 PM
To: edk2-devel 
Subject: [edk2] BootableImageSupportTest\StorageSecurityCommandProtocolTest

Hi,

   When the we run the 
"BootableImageSupportTest\StorageSecurityCommandProtocolTest" test on the NVME 
devices we are getting into error because of the below testing code.

//
// According to TCG definition, when the Security Protocol field is set to 
00h, and SP
// Specific is set to h in a TRUSTED RECEIVE command, return security 
protocol
// information. This Command is not associated with a security send command
//
Status = StorageSecurityCommand->ReceiveData (
   StorageSecurityCommand,
   BlockIo->Media->MediaId,
   1,// Timeout 
10-sec
   0,// 
SecurityProtocol
   0,// 
SecurityProtocolSpecifcData
   10,   // 
PayloadBufferSize,
   DataBuffer,   // 
PayloadBuffer
   
   );
//
// for ATA8-ACS SecurityProtocol, 512 byte is a request
//
if (IsAtaDevice) {
  if((Status == EFI_DEVICE_ERROR) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
} else {
  if((!EFI_ERROR(Status)) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
}

For Ata devices, EFI_DEVICE_ERROR considered as valid error case and for the 
Nvme ( Non ATA) device it's considered as error. Could you please let us know 
why there is difference in this case ?.

Thanks,
Ramesh


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 09/10] MdeModulePkg NvmExpressDxe: Fix 'Event' won't be signaled for Admin cmds

[Hao Wu]

This commit fixes the issue that the caller event passed to
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() will not be signaled for
NVME Admin commands.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 96e9d88..2c30009 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -596,7 +596,7 @@ NvmExpressPassThru (
   //
   // Ring the submission queue doorbell.
   //
-  if (Event != NULL) {
+  if ((Event != NULL) && (QueueId != 0)) {
 Private->SqTdbl[QueueId].Sqt =
   (Private->SqTdbl[QueueId].Sqt + 1) % (NVME_ASYNC_CSQ_SIZE + 1);
   } else {
@@ -616,7 +616,7 @@ NvmExpressPassThru (
   // For non-blocking requests, return directly if the command is placed
   // in the submission queue.
   //
-  if (Event != NULL) {
+  if ((Event != NULL) && (QueueId != 0)) {
 AsyncRequest = AllocateZeroPool (sizeof (NVME_PASS_THRU_ASYNC_REQ));
 if (AsyncRequest == NULL) {
   Status = EFI_DEVICE_ERROR;
@@ -699,6 +699,15 @@ NvmExpressPassThru (

);
 
+  //
+  // For now, the code does not support the non-blocking feature for admin 
queue.
+  // If Event is not NULL for admin queue, signal the caller's event here.
+  //
+  if (Event != NULL) {
+ASSERT (QueueId == 0);
+gBS->SignalEvent (Event);
+  }
+
 EXIT:
   if (MapData != NULL) {
 PciIo->Unmap (
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 06/10] MdeModulePkg NvmExpressDxe: Add check on the attributes of NVME controller

[Hao Wu]

According to UEFI spec, an EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL with neither
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL nor
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL set in the Attributes field
is an illegal configuration.

This commit adds this check in the PassThru API to follow the spec.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 6b29260..c7ead21 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -375,6 +375,7 @@ NvmExpressPassThru (
   UINT64 *Prp;
   VOID   *PrpListHost;
   UINTN  PrpListNo;
+  UINT32 Attributes;
   UINT32 IoAlign;
   UINT32 Data;
   NVME_PASS_THRU_ASYNC_REQ   *AsyncRequest;
@@ -396,9 +397,20 @@ NvmExpressPassThru (
   }
 
   //
+  // 'Attributes' with neither EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL nor
+  // EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL set is an illegal
+  // configuration.
+  //
+  Attributes  = This->Mode->Attributes;
+  if ((Attributes & (EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL |
+EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL)) == 0) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  //
   // Buffer alignment check for TransferBuffer & MetadataBuffer.
   //
-  IoAlign = This->Mode->IoAlign;
+  IoAlign = This->Mode->IoAlign;
   if (IoAlign > 0 && (((UINTN) Packet->TransferBuffer & (IoAlign - 1)) != 0)) {
 return EFI_INVALID_PARAMETER;
   }
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 01/10] MdeModulePkg NvmExpressDxe: Avoid crashing 'Mode' during OpenProtocol

[Hao Wu]

The gBS->OpenProtocol() calls to open EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL
in NvmExpress.c will crash the data in 'Mode' field of
'Private->Passthru'.

The third parameter of gBS->OpenProtocol() is an output parameter that
stores the address where a pointer to the corresponding Protocol
Interface. The current code mistakenly pass '>Passthru' (a
pointer of the EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL) as the third
parameter. This will crash the data in 'Mode' filed.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index cb25b3e..255fa2b 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -76,6 +76,7 @@ EnumerateNvmeDevNamespace (
   UINT32LbaFmtIdx;
   UINT8 Sn[21];
   UINT8 Mn[41];
+  VOID  *DummyInterface;
 
   NewDevicePathNode = NULL;
   DevicePath= NULL;
@@ -264,7 +265,7 @@ EnumerateNvmeDevNamespace (
 gBS->OpenProtocol (
Private->ControllerHandle,
,
-   (VOID **) >Passthru,
+   (VOID **) ,
Private->DriverBindingHandle,
Device->DeviceHandle,
EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
@@ -392,10 +393,10 @@ UnregisterNvmeNamespace (
   EFI_STATUS   Status;
   EFI_BLOCK_IO_PROTOCOL*BlockIo;
   NVME_DEVICE_PRIVATE_DATA *Device;
-  NVME_CONTROLLER_PRIVATE_DATA *Private;
   EFI_STORAGE_SECURITY_COMMAND_PROTOCOL*StorageSecurity;
   BOOLEAN  IsEmpty;
   EFI_TPL  OldTpl;
+  VOID *DummyInterface;
 
   BlockIo = NULL;
 
@@ -412,7 +413,6 @@ UnregisterNvmeNamespace (
   }
 
   Device  = NVME_DEVICE_PRIVATE_DATA_FROM_BLOCK_IO (BlockIo);
-  Private = Device->Controller;
 
   //
   // Wait for the device's asynchronous I/O queue to become empty.
@@ -460,7 +460,7 @@ UnregisterNvmeNamespace (
 gBS->OpenProtocol (
Controller,
,
-   (VOID **) >Passthru,
+   (VOID **) ,
This->DriverBindingHandle,
Handle,
EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
@@ -490,7 +490,7 @@ UnregisterNvmeNamespace (
   gBS->OpenProtocol (
 Controller,
 ,
-(VOID **) >Passthru,
+(VOID **) ,
 This->DriverBindingHandle,
 Handle,
 EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 07/10] MdeModulePkg NvmExpressDxe: Add check for command packet in PassThru

[Hao Wu]

This commit adds serveral checks for the 'Packet' parameter passed to the
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() API:

The check for the 'TransferLength' field in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET to make sure the value will not
exceed the maximum data transfer size allowed by a controller.

The check for the 'TransferBuffer' and 'TransferLength' fields in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET when the Opcode of an NVME
command indicates a data transfer between controller and host.

The check for the 'MetadataLength' field in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET to make sure the value is not 0
when the corresponding 'MetadataBuffer' field has a non-NULL value.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c  | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index c7ead21..2209ee6 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -377,6 +377,7 @@ NvmExpressPassThru (
   UINTN  PrpListNo;
   UINT32 Attributes;
   UINT32 IoAlign;
+  UINT32 MaxTransLen;
   UINT32 Data;
   NVME_PASS_THRU_ASYNC_REQ   *AsyncRequest;
   EFI_TPLOldTpl;
@@ -420,6 +421,19 @@ NvmExpressPassThru (
   }
 
   Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
+
+  //
+  // Check whether TransferLength exceeds the maximum data transfer size.
+  //
+  if (Private->ControllerData->Mdts != 0) {
+MaxTransLen = (1 << (Private->ControllerData->Mdts)) *
+  (1 << (Private->Cap.Mpsmin + 12));
+if (Packet->TransferLength > MaxTransLen) {
+  Packet->TransferLength = MaxTransLen;
+  return EFI_BAD_BUFFER_SIZE;
+}
+  }
+
   PciIo   = Private->PciIo;
   MapData = NULL;
   MapMeta = NULL;
@@ -477,6 +491,10 @@ NvmExpressPassThru (
   // processor and a PCI Bus Master. It's caller's responsbility to ensure 
this.
   //
   if (((Sq->Opc & (BIT0 | BIT1)) != 0) && (Sq->Opc != NVME_ADMIN_CRIOCQ_CMD) 
&& (Sq->Opc != NVME_ADMIN_CRIOSQ_CMD)) {
+if ((Packet->TransferLength == 0) || (Packet->TransferBuffer == NULL)) {
+  return EFI_INVALID_PARAMETER;
+}
+
 if ((Sq->Opc & BIT0) != 0) {
   Flag = EfiPciIoOperationBusMasterRead;
 } else {
@@ -499,8 +517,7 @@ NvmExpressPassThru (
 Sq->Prp[0] = PhyAddr;
 Sq->Prp[1] = 0;
 
-MapLength = Packet->MetadataLength;
-if(Packet->MetadataBuffer != NULL) {
+if((Packet->MetadataLength != 0) && (Packet->MetadataBuffer != NULL)) {
   MapLength = Packet->MetadataLength;
   Status = PciIo->Map (
 PciIo,
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 10/10] MdeModulePkg NvmExpressDxe: Set the non-blocking I/O feature support bit

[Hao Wu]

Since current codes in NvmExpressDxe already support the non-blocking I/O
feature for EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL interface, the relative bit
in the 'Attributes' field of EFI_NVM_EXPRESS_PASS_THRU_MODE should be set
to reflect this.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index 255fa2b..39f49bd 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -39,7 +39,10 @@ EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL 
gNvmExpressDriverSupportedEfiVersion =
 // Template for NVM Express Pass Thru Mode data structure.
 //
 GLOBAL_REMOVE_IF_UNREFERENCED EFI_NVM_EXPRESS_PASS_THRU_MODE 
gEfiNvmExpressPassThruMode = {
-  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL | 
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL | 
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM,
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL   |
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL|
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_NONBLOCKIO |
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM,
   sizeof (UINTN),
   0x10100
 };
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 03/10] MdeModulePkg NvmExpressDxe: Refine GetNameSpace API to follow spec

[Hao Wu]

According to the UEFI spec,
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.GetNamespace() should return
EFI_NOT_FOUND when the input DevicePath is a device path node type that
the NVM Express Pass Thru driver supports, but there is not a valid
translation from DevicePath to a namespace ID. Current code will return
EFI_SUCCESS. This commit adds additional check in the GetNameSpace API to
make sure correct status is returned.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index ccff4f6..f0d2f5a 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -820,6 +820,7 @@ NvmExpressGetNamespace (
   )
 {
   NVME_NAMESPACE_DEVICE_PATH   *Node;
+  NVME_CONTROLLER_PRIVATE_DATA *Private;
 
   if ((This == NULL) || (DevicePath == NULL) || (NamespaceId == NULL)) {
 return EFI_INVALID_PARAMETER;
@@ -829,13 +830,22 @@ NvmExpressGetNamespace (
 return EFI_UNSUPPORTED;
   }
 
-  Node = (NVME_NAMESPACE_DEVICE_PATH *)DevicePath;
+  Node= (NVME_NAMESPACE_DEVICE_PATH *)DevicePath;
+  Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
 
   if (DevicePath->SubType == MSG_NVME_NAMESPACE_DP) {
 if (DevicePathNodeLength(DevicePath) != 
sizeof(NVME_NAMESPACE_DEVICE_PATH)) {
   return EFI_NOT_FOUND;
 }
 
+//
+// Check NamespaceId in the device path node is valid or not.
+//
+if ((Node->NamespaceId == 0) ||
+  (Node->NamespaceId > Private->ControllerData->Nn)) {
+  return EFI_NOT_FOUND;
+}
+
 *NamespaceId = Node->NamespaceId;
 
 return EFI_SUCCESS;
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 05/10] MdeModulePkg NvmExpressDxe: Add buffer alignment check in PassThru API

[Hao Wu]

According to the UEFI spec, the 'TransferBuffer' and 'MetadataBuffer' used
in a data transfer should be aligned on the boundary specified by the
IoAlign field in the EFI_NVM_EXPRESS_PASS_THRU_MODE structure.

This commit adds this check to follow the spec.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 55 +-
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index ec7507e..6b29260 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -357,27 +357,28 @@ NvmExpressPassThru (
   IN EFI_EVENT   Event OPTIONAL
   )
 {
-  NVME_CONTROLLER_PRIVATE_DATA  *Private;
-  EFI_STATUSStatus;
-  EFI_PCI_IO_PROTOCOL   *PciIo;
-  NVME_SQ   *Sq;
-  NVME_CQ   *Cq;
-  UINT16QueueId;
-  UINT32Bytes;
-  UINT16Offset;
-  EFI_EVENT TimerEvent;
-  EFI_PCI_IO_PROTOCOL_OPERATION Flag;
-  EFI_PHYSICAL_ADDRESS  PhyAddr;
-  VOID  *MapData;
-  VOID  *MapMeta;
-  VOID  *MapPrpList;
-  UINTN MapLength;
-  UINT64*Prp;
-  VOID  *PrpListHost;
-  UINTN PrpListNo;
-  UINT32Data;
-  NVME_PASS_THRU_ASYNC_REQ  *AsyncRequest;
-  EFI_TPL   OldTpl;
+  NVME_CONTROLLER_PRIVATE_DATA   *Private;
+  EFI_STATUS Status;
+  EFI_PCI_IO_PROTOCOL*PciIo;
+  NVME_SQ*Sq;
+  NVME_CQ*Cq;
+  UINT16 QueueId;
+  UINT32 Bytes;
+  UINT16 Offset;
+  EFI_EVENT  TimerEvent;
+  EFI_PCI_IO_PROTOCOL_OPERATION  Flag;
+  EFI_PHYSICAL_ADDRESS   PhyAddr;
+  VOID   *MapData;
+  VOID   *MapMeta;
+  VOID   *MapPrpList;
+  UINTN  MapLength;
+  UINT64 *Prp;
+  VOID   *PrpListHost;
+  UINTN  PrpListNo;
+  UINT32 IoAlign;
+  UINT32 Data;
+  NVME_PASS_THRU_ASYNC_REQ   *AsyncRequest;
+  EFI_TPLOldTpl;
 
   //
   // check the data fields in Packet parameter.
@@ -394,6 +395,18 @@ NvmExpressPassThru (
 return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Buffer alignment check for TransferBuffer & MetadataBuffer.
+  //
+  IoAlign = This->Mode->IoAlign;
+  if (IoAlign > 0 && (((UINTN) Packet->TransferBuffer & (IoAlign - 1)) != 0)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (IoAlign > 0 && (((UINTN) Packet->MetadataBuffer & (IoAlign - 1)) != 0)) {
+return EFI_INVALID_PARAMETER;
+  }
+
   Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
   PciIo   = Private->PciIo;
   MapData = NULL;
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 04/10] MdeModulePkg NvmExpressDxe: Refine GetNextNamespace API to follow spec

[Hao Wu]

According to the UEFI spec,
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.GetNextNamespace() should return
EFI_NOT_FOUND when the value pointed to by NamespaceId is the namespace ID
of the last namespace on the NVM Express controller. This commit modifies
the check for NamespaceId to follow this rule.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index f0d2f5a..ec7507e 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -758,11 +758,15 @@ NvmExpressGetNextNamespace (
 
 *NamespaceId = NextNamespaceId;
   } else {
-if (*NamespaceId >= Private->ControllerData->Nn) {
+if (*NamespaceId > Private->ControllerData->Nn) {
   return EFI_INVALID_PARAMETER;
 }
 
 NextNamespaceId = *NamespaceId + 1;
+if (NextNamespaceId > Private->ControllerData->Nn) {
+  return EFI_NOT_FOUND;
+}
+
 //
 // Allocate buffer for Identify Namespace data.
 //
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 08/10] MdeModulePkg NvmExpressDxe: Add NamespaceId validity check in PassThru

[Hao Wu]

According to the UEFI spec, EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru()
should return EFI_INVALID_PARAMETER if the input 'NamespaceId' is invalid
for the NVM Express controller. This commit adds check in PassThru() to
follow this rule.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 2209ee6..96e9d88 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -423,6 +423,14 @@ NvmExpressPassThru (
   Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
 
   //
+  // Check NamespaceId is valid or not.
+  //
+  if ((NamespaceId > Private->ControllerData->Nn) &&
+  (NamespaceId != (UINT32) -1)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  //
   // Check whether TransferLength exceeds the maximum data transfer size.
   //
   if (Private->ControllerData->Mdts != 0) {
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 00/10] Fix some bugs in NvmExpressDxe driver

[Hao Wu]

Compared with V1 of the series, the following changes are made:
1. Add NamespaceId validity check in
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru().

2. Fixes the issue that the caller event passed to
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() will not be signaled for
   NVME Admin commands.

3. Set the non-blocking I/O feature support bit in structure
   EFI_NVM_EXPRESS_PASS_THRU_MODE

Hao Wu (10):
  MdeModulePkg NvmExpressDxe: Avoid crashing 'Mode' during OpenProtocol
  MdeModulePkg NvmExpressDxe: Refine BuildDevicePath API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNameSpace API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNextNamespace API to follow spec
  MdeModulePkg NvmExpressDxe: Add buffer alignment check in PassThru API
  MdeModulePkg NvmExpressDxe: Add check on the attributes of NVME
controller
  MdeModulePkg NvmExpressDxe: Add check for command packet in PassThru
  MdeModulePkg NvmExpressDxe: Add NamespaceId validity check in PassThru
  MdeModulePkg NvmExpressDxe: Fix 'Event' won't be signaled for Admin
cmds
  MdeModulePkg NvmExpressDxe: Set the non-blocking I/O feature support
bit

 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c|  15 ++-
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 141 -
 2 files changed, 118 insertions(+), 38 deletions(-)

-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Wang, Sunny (HPS SW)]

Got it. Thanks for confirming this and working on the replacement.

Regards,
Sunny Wang

-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: Thursday, September 01, 2016 10:20 AM
To: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Dong, Eric 
Subject: RE: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
BootManagerMenuApp from LoadFile
Importance: High

Sunny:
  Yes. This is the replacement of Eric patch. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Thursday, September 01, 2016 10:18 AM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Dong, Eric 
> Subject: Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
> BootManagerMenuApp from LoadFile
> 
> Hi Liming,
> Except Ray's comment, others Look good to me.
> Reviewed-by: Sunny Wang 
> 
> Hi Ray and Eric,
> It looks like this code change is used for replacing the one which we 
> offline discussed to fix the duplicated Boot Manager menu issue, isn't it?
>  - [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Add OptioalData for 
> boot option.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Liming Gao
> Sent: Wednesday, August 31, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni ; Eric Dong 
> Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
> BootManagerMenuApp from LoadFile
> 
> BootManagerMenuApp boot option is handled by 
> EfiBootManagerGetBootManagerMenu.
> Don't need to handle it again when parse LoadFile protocol.
> 
> Cc: Ruiyu Ni 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++---
> ---
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index ecd0ae3..f8a3988 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
>UINTN Removable;
>UINTN Index;
>CHAR16*Description;
> -  UINT32BootAttributes;
> 
>ASSERT (BootOptionCount != NULL);
> 
> @@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
>   
>   );
>for (Index = 0; Index < HandleCount; Index++) {
> +//
> +// Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> +//
> +if (BmIsBootMenuAppFilePath (DevicePathFromHandle 
> + (Handles[Index])))
> {
> +  continue;
> +}
> 
>  Description = BmGetBootDescription (Handles[Index]);
>  BootOptions = ReallocatePool (
> @@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
>  );
>  ASSERT (BootOptions != NULL);
> 
> -//
> -// If LoadFile includes BootMenuApp, its boot attribue will be set to APP
> and HIDDEN.
> -//
> -BootAttributes = LOAD_OPTION_ACTIVE;
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> -  BootAttributes = LOAD_OPTION_CATEGORY_APP |
> LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN;
> -}
> -
>  Status = EfiBootManagerInitializeLoadOption (
> [(*BootOptionCount)++],
> LoadOptionNumberUnassigned,
> LoadOptionTypeBoot,
> -   BootAttributes,
> +   LOAD_OPTION_ACTIVE,
> Description,
> DevicePathFromHandle (Handles[Index]),
> NULL,
> --
> 2.8.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Gao, Liming]

Sunny:
  Yes. This is the replacement of Eric patch. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang, Sunny (HPS SW)
> Sent: Thursday, September 01, 2016 10:18 AM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Dong, Eric 
> Subject: Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore
> BootManagerMenuApp from LoadFile
> 
> Hi Liming,
> Except Ray's comment, others Look good to me.
> Reviewed-by: Sunny Wang 
> 
> Hi Ray and Eric,
> It looks like this code change is used for replacing the one which we offline
> discussed to fix the duplicated Boot Manager menu issue, isn't it?
>  - [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Add OptioalData for
> boot option.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Liming Gao
> Sent: Wednesday, August 31, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni ; Eric Dong 
> Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore
> BootManagerMenuApp from LoadFile
> 
> BootManagerMenuApp boot option is handled by
> EfiBootManagerGetBootManagerMenu.
> Don't need to handle it again when parse LoadFile protocol.
> 
> Cc: Ruiyu Ni 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++---
> ---
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index ecd0ae3..f8a3988 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
>UINTN Removable;
>UINTN Index;
>CHAR16*Description;
> -  UINT32BootAttributes;
> 
>ASSERT (BootOptionCount != NULL);
> 
> @@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
>   
>   );
>for (Index = 0; Index < HandleCount; Index++) {
> +//
> +// Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> +//
> +if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> +  continue;
> +}
> 
>  Description = BmGetBootDescription (Handles[Index]);
>  BootOptions = ReallocatePool (
> @@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
>  );
>  ASSERT (BootOptions != NULL);
> 
> -//
> -// If LoadFile includes BootMenuApp, its boot attribue will be set to APP
> and HIDDEN.
> -//
> -BootAttributes = LOAD_OPTION_ACTIVE;
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> -  BootAttributes = LOAD_OPTION_CATEGORY_APP |
> LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN;
> -}
> -
>  Status = EfiBootManagerInitializeLoadOption (
> [(*BootOptionCount)++],
> LoadOptionNumberUnassigned,
> LoadOptionTypeBoot,
> -   BootAttributes,
> +   LOAD_OPTION_ACTIVE,
> Description,
> DevicePathFromHandle (Handles[Index]),
> NULL,
> --
> 2.8.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Add NominalSpeed in Type 27 to black list

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Thursday, September 1, 2016 10:01 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen 
> Subject: [PATCH] MdeModulePkg SmbiosMeasurementDxe: Add
> NominalSpeed in Type 27 to black list
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 31
> ++
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurement
> Dxe.c
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurement
> Dxe.c
> index f9e0196677a9..bc5e7464e133 100644
> ---
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurement
> Dxe.c
> +++
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurement
> Dxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>This driver measures SMBIOS table to TPM.
> 
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -84,6 +84,9 @@ SMBIOS_FILTER_TABLE
> mSmbiosFilterType22BlackList[] = {
>  SMBIOS_FILTER_TABLE  mSmbiosFilterType23BlackList[] = {
>{0x17, OFFSET_OF(SMBIOS_TABLE_TYPE23, ResetCount),
> FIELD_SIZE_OF(SMBIOS_TABLE_TYPE23, ResetCount),  0},
>  };
> +SMBIOS_FILTER_TABLE  mSmbiosFilterType27BlackList[] = {
> +  {0x1B, OFFSET_OF(SMBIOS_TABLE_TYPE27, NominalSpeed),
> FIELD_SIZE_OF(SMBIOS_TABLE_TYPE27, NominalSpeed),0},
> +};
>  SMBIOS_FILTER_TABLE  mSmbiosFilterType39BlackList[] = {
>{0x27, OFFSET_OF(SMBIOS_TABLE_TYPE39, SerialNumber),
> FIELD_SIZE_OF(SMBIOS_TABLE_TYPE39, SerialNumber),
> SMBIOS_FILTER_TABLE_FLAG_IS_STRING},
>{0x27, OFFSET_OF(SMBIOS_TABLE_TYPE39, AssetTagNumber),
> FIELD_SIZE_OF(SMBIOS_TABLE_TYPE39, AssetTagNumber),
> SMBIOS_FILTER_TABLE_FLAG_IS_STRING},
> @@ -101,6 +104,7 @@ SMBIOS_FILTER_STRUCT
> mSmbiosFilterStandardTableBlackList[] = {
>{0x12, NULL, 0},
>{0x16, mSmbiosFilterType22BlackList,
> sizeof(mSmbiosFilterType22BlackList)/sizeof(mSmbiosFilterType22BlackList[0
> ])},
>{0x17, mSmbiosFilterType23BlackList,
> sizeof(mSmbiosFilterType23BlackList)/sizeof(mSmbiosFilterType23BlackList[0
> ])},
> +  {0x1B, mSmbiosFilterType27BlackList,
> sizeof(mSmbiosFilterType27BlackList)/sizeof(mSmbiosFilterType27BlackList[0
> ])},
>{0x1F, NULL, 0},
>{0x21, NULL, 0},
>{0x27, mSmbiosFilterType39BlackList,
> sizeof(mSmbiosFilterType39BlackList)/sizeof(mSmbiosFilterType39BlackList[0
> ])},
> @@ -281,18 +285,23 @@ FilterSmbiosEntry (
>  } else {
>Filter = FilterStruct->Filter;
>for (Index = 0; Index < FilterStruct->FilterCount; Index++) {
> -if ((Filter[Index].Flags &
> SMBIOS_FILTER_TABLE_FLAG_IS_STRING) != 0) {
> -  CopyMem (, (UINT8 *)TableEntry + Filter[Index].Offset,
> sizeof(StringId));
> -  if (StringId != 0) {
> -// set ' ' for string field
> -String = GetSmbiosStringById (TableEntry, StringId,
> );
> -ASSERT (String != NULL);
> -//DEBUG ((EFI_D_INFO,"StrId(0x%x)-%a(%d)\n", StringId,
> String, StringLen));
> -SetMem (String, StringLen, ' ');
> +if (((SMBIOS_STRUCTURE *) TableEntry)->Length >=
> (Filter[Index].Offset + Filter[Index].Size)) {
> +  //
> +  // The field is present in the SMBIOS entry.
> +  //
> +  if ((Filter[Index].Flags &
> SMBIOS_FILTER_TABLE_FLAG_IS_STRING) != 0) {
> +CopyMem (, (UINT8 *)TableEntry +
> Filter[Index].Offset, sizeof(StringId));
> +if (StringId != 0) {
> +  // set ' ' for string field
> +  String = GetSmbiosStringById (TableEntry, StringId,
> );
> +  ASSERT (String != NULL);
> +  //DEBUG ((EFI_D_INFO,"StrId(0x%x)-%a(%d)\n", StringId,
> String, StringLen));
> +  SetMem (String, StringLen, ' ');
> +}
>}
> +  // zero non-string field
> +  ZeroMem ((UINT8 *)TableEntry + Filter[Index].Offset,
> Filter[Index].Size);
>  }
> -// zero non-string field
> -ZeroMem ((UINT8 *)TableEntry + Filter[Index].Offset,
> Filter[Index].Size);
>}
>  }
>}
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Wang, Sunny (HPS SW)]

Hi Liming, 
Except Ray's comment, others Look good to me. 
Reviewed-by: Sunny Wang 

Hi Ray and Eric, 
It looks like this code change is used for replacing the one which we offline 
discussed to fix the duplicated Boot Manager menu issue, isn't it?  
 - [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Add OptioalData for boot 
option.

Regards,
Sunny Wang

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, August 31, 2016 4:19 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Eric Dong 
Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore 
BootManagerMenuApp from LoadFile

BootManagerMenuApp boot option is handled by EfiBootManagerGetBootManagerMenu.
Don't need to handle it again when parse LoadFile protocol.

Cc: Ruiyu Ni 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ecd0ae3..f8a3988 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
   UINTN Removable;
   UINTN Index;
   CHAR16*Description;
-  UINT32BootAttributes;
 
   ASSERT (BootOptionCount != NULL);
 
@@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
+//
+// Ignore BootMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
+//
+if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+  continue;
+}
 
 Description = BmGetBootDescription (Handles[Index]);
 BootOptions = ReallocatePool (
@@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
 );
 ASSERT (BootOptions != NULL);
 
-//
-// If LoadFile includes BootMenuApp, its boot attribue will be set to APP 
and HIDDEN.
-//
-BootAttributes = LOAD_OPTION_ACTIVE;
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
-  BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
-}
-
 Status = EfiBootManagerInitializeLoadOption (
[(*BootOptionCount)++],
LoadOptionNumberUnassigned,
LoadOptionTypeBoot,
-   BootAttributes,
+   LOAD_OPTION_ACTIVE,
Description,
DevicePathFromHandle (Handles[Index]),
NULL,
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Gao, Liming]

Ok. Got your comments. This patch just ignores BootManagerMenu from LoadFile. 
Next patch will rename BootMenuApp to BootManagerMenu. I will update them. 

Thanks
Liming
> -Original Message-
> From: Ni, Ruiyu
> Sent: Thursday, September 01, 2016 10:02 AM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: RE: [Patch] MdeModulePkg UefiBootManagerLib: Ignore
> BootManagerMenuApp from LoadFile
> 
> Liming,
> Please use the term "BootManagerMenu" instead of "BootMenuApp", to
> avoid confusion when reading from future developers.
> 
> // Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> -->
> // Ignore BootManagerMenu, which will be auto-created by
> EfiBootManagerGetBootManagerMenu().
> 
> BmIsBootMenuAppFilePath
> -->
> BmIsBootManagerMenuFilePath
> 
> With the above two changes,
> Reviewed-by: Ruiyu Ni 
> 
> > -Original Message-
> > From: Gao, Liming
> > Sent: Wednesday, August 31, 2016 4:19 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Dong, Eric 
> > Subject: [Patch] MdeModulePkg UefiBootManagerLib: Ignore
> > BootManagerMenuApp from LoadFile
> >
> > BootManagerMenuApp boot option is handled by
> > EfiBootManagerGetBootManagerMenu.
> > Don't need to handle it again when parse LoadFile protocol.
> >
> > Cc: Ruiyu Ni 
> > Cc: Eric Dong 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Liming Gao 
> > ---
> >  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++-
> --
> > ---
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > index ecd0ae3..f8a3988 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > @@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
> >UINTN Removable;
> >UINTN Index;
> >CHAR16*Description;
> > -  UINT32BootAttributes;
> >
> >ASSERT (BootOptionCount != NULL);
> >
> > @@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
> >   
> >   );
> >for (Index = 0; Index < HandleCount; Index++) {
> > +//
> > +// Ignore BootMenuApp. its boot option will be created by
> > BmRegisterBootManagerMenu().
> > +//
> > +if (BmIsBootMenuAppFilePath (DevicePathFromHandle
> (Handles[Index])))
> > {
> > +  continue;
> > +}
> >
> >  Description = BmGetBootDescription (Handles[Index]);
> >  BootOptions = ReallocatePool (
> > @@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
> >  );
> >  ASSERT (BootOptions != NULL);
> >
> > -//
> > -// If LoadFile includes BootMenuApp, its boot attribue will be set to 
> > APP
> > and HIDDEN.
> > -//
> > -BootAttributes = LOAD_OPTION_ACTIVE;
> > -if (BmIsBootMenuAppFilePath (DevicePathFromHandle
> (Handles[Index])))
> > {
> > -  BootAttributes = LOAD_OPTION_CATEGORY_APP |
> > LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN;
> > -}
> > -
> >  Status = EfiBootManagerInitializeLoadOption (
> > [(*BootOptionCount)++],
> > LoadOptionNumberUnassigned,
> > LoadOptionTypeBoot,
> > -   BootAttributes,
> > +   LOAD_OPTION_ACTIVE,
> > Description,
> > DevicePathFromHandle (Handles[Index]),
> > NULL,
> > --
> > 2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Rename BootMenuApp to BootManagerMenuApp

[Ni, Ruiyu]

Liming,
Please use the term "BootManagerMenu" instead of "BootMenuApp", to avoid 
confusion when reading from future developers.

BmIsBootMenuAppFilePath
-->
BmIsBootManagerMenuFilePath

With the above change, 
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Gao, Liming
> Sent: Wednesday, August 31, 2016 4:20 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [Patch] MdeModulePkg UefiBootManagerLib: Rename
> BootMenuApp to BootManagerMenuApp
> 
> Rename local function name BootMenuApp to BootManagerMenuApp to
> align to other public function name.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 20 ++--
> 
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index f8a3988..88bb13b 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1530,15 +1530,15 @@ EfiBootManagerGetLoadOptionBuffer (  }
> 
>  /**
> -  Check if it's a Device Path pointing to BootMenuApp.
> +  Check if it's a Device Path pointing to BootManagerMenuApp.
> 
>@param  DevicePath Input device path.
> 
> -  @retval TRUE   The device path is BootMenuApp File Device Path.
> -  @retval FALSE  The device path is NOT BootMenuApp File Device Path.
> +  @retval TRUE   The device path is BootManagerMenuApp File Device Path.
> +  @retval FALSE  The device path is NOT BootManagerMenuApp File Device
> Path.
>  **/
>  BOOLEAN
> -BmIsBootMenuAppFilePath (
> +BmIsBootManagerMenuAppFilePath (
>EFI_DEVICE_PATH_PROTOCOL *DevicePath
>  )
>  {
> @@ -1645,7 +1645,7 @@ EfiBootManagerBoot (
>// 3. Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to
> load and execute
>//the boot option.
>//
> -  if (BmIsBootMenuAppFilePath (BootOption->FilePath)) {
> +  if (BmIsBootManagerMenuAppFilePath (BootOption->FilePath)) {
>  DEBUG ((EFI_D_INFO, "[Bds] Booting Boot Manager Menu.\n"));
>  BmStopHotkeyService (NULL, NULL);
>} else {
> @@ -2070,9 +2070,9 @@ BmEnumerateBootOptions (
>   );
>for (Index = 0; Index < HandleCount; Index++) {
>  //
> -// Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> +// Ignore BootManagerMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
>  //
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> +if (BmIsBootManagerMenuAppFilePath (DevicePathFromHandle
> + (Handles[Index]))) {
>continue;
>  }
> 
> @@ -2212,7 +2212,7 @@ BmRegisterBootManagerMenu (
>DevicePath = NULL;
>Description = NULL;
>//
> -  // Try to find BootMenuApp from LoadFile protocol
> +  // Try to find BootManagerMenuApp from LoadFile protocol
>//
>gBS->LocateHandleBuffer (
>   ByProtocol,
> @@ -,7 +,7 @@ BmRegisterBootManagerMenu (
>   
>   );
>for (Index = 0; Index < HandleCount; Index++) {
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> +if (BmIsBootManagerMenuAppFilePath (DevicePathFromHandle
> + (Handles[Index]))) {
>DevicePath  = DuplicateDevicePath (DevicePathFromHandle
> (Handles[Index]));
>Description = BmGetBootDescription (Handles[Index]);
>break;
> @@ -2331,7 +2331,7 @@ EfiBootManagerGetBootManagerMenu (
>BootOptions = EfiBootManagerGetLoadOptions (,
> LoadOptionTypeBoot);
> 
>for (Index = 0; Index < BootOptionCount; Index++) {
> -if (BmIsBootMenuAppFilePath (BootOptions[Index].FilePath)) {
> +if (BmIsBootManagerMenuAppFilePath (BootOptions[Index].FilePath)) {
>  Status = EfiBootManagerInitializeLoadOption (
> BootOption,
> BootOptions[Index].OptionNumber,
> --
> 2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Ni, Ruiyu]

Liming,
Please use the term "BootManagerMenu" instead of "BootMenuApp", to avoid 
confusion when reading from future developers.

// Ignore BootMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
-->
// Ignore BootManagerMenu, which will be auto-created by 
EfiBootManagerGetBootManagerMenu().

BmIsBootMenuAppFilePath
-->
BmIsBootManagerMenuFilePath

With the above two changes, 
Reviewed-by: Ruiyu Ni 

> -Original Message-
> From: Gao, Liming
> Sent: Wednesday, August 31, 2016 4:19 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Dong, Eric 
> Subject: [Patch] MdeModulePkg UefiBootManagerLib: Ignore
> BootManagerMenuApp from LoadFile
> 
> BootManagerMenuApp boot option is handled by
> EfiBootManagerGetBootManagerMenu.
> Don't need to handle it again when parse LoadFile protocol.
> 
> Cc: Ruiyu Ni 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++---
> ---
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index ecd0ae3..f8a3988 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
>UINTN Removable;
>UINTN Index;
>CHAR16*Description;
> -  UINT32BootAttributes;
> 
>ASSERT (BootOptionCount != NULL);
> 
> @@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
>   
>   );
>for (Index = 0; Index < HandleCount; Index++) {
> +//
> +// Ignore BootMenuApp. its boot option will be created by
> BmRegisterBootManagerMenu().
> +//
> +if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> +  continue;
> +}
> 
>  Description = BmGetBootDescription (Handles[Index]);
>  BootOptions = ReallocatePool (
> @@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
>  );
>  ASSERT (BootOptions != NULL);
> 
> -//
> -// If LoadFile includes BootMenuApp, its boot attribue will be set to APP
> and HIDDEN.
> -//
> -BootAttributes = LOAD_OPTION_ACTIVE;
> -if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index])))
> {
> -  BootAttributes = LOAD_OPTION_CATEGORY_APP |
> LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN;
> -}
> -
>  Status = EfiBootManagerInitializeLoadOption (
> [(*BootOptionCount)++],
> LoadOptionNumberUnassigned,
> LoadOptionTypeBoot,
> -   BootAttributes,
> +   LOAD_OPTION_ACTIVE,
> Description,
> DevicePathFromHandle (Handles[Index]),
> NULL,
> --
> 2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Add NominalSpeed in Type 27 to black list

[Star Zeng]

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 31 ++
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c 
b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index f9e0196677a9..bc5e7464e133 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -1,7 +1,7 @@
 /** @file
   This driver measures SMBIOS table to TPM.
 
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -84,6 +84,9 @@ SMBIOS_FILTER_TABLE  mSmbiosFilterType22BlackList[] = {
 SMBIOS_FILTER_TABLE  mSmbiosFilterType23BlackList[] = {
   {0x17, OFFSET_OF(SMBIOS_TABLE_TYPE23, ResetCount),  
FIELD_SIZE_OF(SMBIOS_TABLE_TYPE23, ResetCount),  0},
 };
+SMBIOS_FILTER_TABLE  mSmbiosFilterType27BlackList[] = {
+  {0x1B, OFFSET_OF(SMBIOS_TABLE_TYPE27, NominalSpeed),
FIELD_SIZE_OF(SMBIOS_TABLE_TYPE27, NominalSpeed),0},
+};
 SMBIOS_FILTER_TABLE  mSmbiosFilterType39BlackList[] = {
   {0x27, OFFSET_OF(SMBIOS_TABLE_TYPE39, SerialNumber),
FIELD_SIZE_OF(SMBIOS_TABLE_TYPE39, SerialNumber),
SMBIOS_FILTER_TABLE_FLAG_IS_STRING},
   {0x27, OFFSET_OF(SMBIOS_TABLE_TYPE39, AssetTagNumber),  
FIELD_SIZE_OF(SMBIOS_TABLE_TYPE39, AssetTagNumber),  
SMBIOS_FILTER_TABLE_FLAG_IS_STRING},
@@ -101,6 +104,7 @@ SMBIOS_FILTER_STRUCT  mSmbiosFilterStandardTableBlackList[] 
= {
   {0x12, NULL, 0},
   {0x16, mSmbiosFilterType22BlackList, 
sizeof(mSmbiosFilterType22BlackList)/sizeof(mSmbiosFilterType22BlackList[0])},
   {0x17, mSmbiosFilterType23BlackList, 
sizeof(mSmbiosFilterType23BlackList)/sizeof(mSmbiosFilterType23BlackList[0])},
+  {0x1B, mSmbiosFilterType27BlackList, 
sizeof(mSmbiosFilterType27BlackList)/sizeof(mSmbiosFilterType27BlackList[0])},
   {0x1F, NULL, 0},
   {0x21, NULL, 0},
   {0x27, mSmbiosFilterType39BlackList, 
sizeof(mSmbiosFilterType39BlackList)/sizeof(mSmbiosFilterType39BlackList[0])},
@@ -281,18 +285,23 @@ FilterSmbiosEntry (
 } else {
   Filter = FilterStruct->Filter;
   for (Index = 0; Index < FilterStruct->FilterCount; Index++) {
-if ((Filter[Index].Flags & SMBIOS_FILTER_TABLE_FLAG_IS_STRING) != 0) {
-  CopyMem (, (UINT8 *)TableEntry + Filter[Index].Offset, 
sizeof(StringId));
-  if (StringId != 0) {
-// set ' ' for string field
-String = GetSmbiosStringById (TableEntry, StringId, );
-ASSERT (String != NULL);
-//DEBUG ((EFI_D_INFO,"StrId(0x%x)-%a(%d)\n", StringId, String, 
StringLen));
-SetMem (String, StringLen, ' ');
+if (((SMBIOS_STRUCTURE *) TableEntry)->Length >= (Filter[Index].Offset 
+ Filter[Index].Size)) {
+  //
+  // The field is present in the SMBIOS entry.
+  //
+  if ((Filter[Index].Flags & SMBIOS_FILTER_TABLE_FLAG_IS_STRING) != 0) 
{
+CopyMem (, (UINT8 *)TableEntry + Filter[Index].Offset, 
sizeof(StringId));
+if (StringId != 0) {
+  // set ' ' for string field
+  String = GetSmbiosStringById (TableEntry, StringId, );
+  ASSERT (String != NULL);
+  //DEBUG ((EFI_D_INFO,"StrId(0x%x)-%a(%d)\n", StringId, String, 
StringLen));
+  SetMem (String, StringLen, ' ');
+}
   }
+  // zero non-string field
+  ZeroMem ((UINT8 *)TableEntry + Filter[Index].Offset, 
Filter[Index].Size);
 }
-// zero non-string field
-ZeroMem ((UINT8 *)TableEntry + Filter[Index].Offset, 
Filter[Index].Size);
   }
 }
   }
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" command

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Wednesday, August 31, 2016 1:27 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben 
> Subject: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg"
> command
> 
> When user uses the command "bcfg driver|boot [dump [-v]]", the number
> of command line value parameters (doesn't include the
> flag) must be three. We can add this point to check whether using this
> command correctly.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c   |
> 4 
>  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni |
> 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> index 9baeecc..e2306bf 100644
> ---
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> @@ -1294,10 +1294,14 @@ ShellCommandRunBcfg (
>  if (ShellStatus == SHELL_SUCCESS && CurrentOperation.Target <
> BcfgTargetMax) {
>for (ParamNumber = 2 ; ParamNumber <
> ShellCommandLineGetCount(Package) && ShellStatus == SHELL_SUCCESS;
> ParamNumber++) {
>  CurrentParam = ShellCommandLineGetRawValue(Package,
> ParamNumber);
>  if(gUnicodeCollation->StriColl(gUnicodeCollation,
> (CHAR16*)CurrentParam, L"dump") == 0){
>CurrentOperation.Type = BcfgTypeDump;
> +  if (ShellCommandLineGetCount(Package) > 3) {
> +ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellBcfgHiiHandle, L"bcfg");
> +ShellStatus = SHELL_INVALID_PARAMETER;
> +  }
>  } else if (ShellCommandLineGetFlag(Package, L"-v")) {
>ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> gShellBcfgHiiHandle, L"bcfg", L"-v (without dump)");
>ShellStatus = SHELL_INVALID_PARAMETER;
>  } else if (gUnicodeCollation->StriColl(gUnicodeCollation,
> (CHAR16*)CurrentParam, L"add") == 0) {
>if ((ParamNumber + 3) >= ShellCommandLineGetCount(Package)) { diff
> --git
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> index 282494b..7668e49 100644
> ---
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.u
> +++ ni
> @@ -1,9 +1,9 @@
>  // /**
>  //
>  // (C) Copyright 2014-2015 Hewlett-Packard Development Company, L.P.
> -// Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
> +// Copyright (c) 2010 - 2016, Intel Corporation. All rights
> +reserved.
>  // This program and the accompanying materials  // are licensed and made
> available under the terms and conditions of the BSD License  // which
> accompanies this distribution. The full text of the license may be found at  
> //
> http://opensource.org/licenses/bsd-license.php
>  //
> @@ -30,10 +30,11 @@
>  #string STR_GEN_NO_VALUE  #language en-US "%H%s%N: Missing
> argument for flag - '%H%s%N'\r\n"
>  #string STR_GEN_PARAM_INV #language en-US "%H%s%N: Invalid
> argument - '%H%s%N'\r\n"
>  #string STR_GEN_NO_DRIVER_BOOT#language en-US "%H%s%N: Driver
> or Boot must be selected.\r\n"
>  #string STR_GEN_BOOT_ONLY #language en-US "%H%s%N: Boot must
> be selected for hot key options.\r\n"
>  #string STR_GEN_TOO_FEW   #language en-US "%H%s%N: Too few
> arguments.\r\n"
> +#string STR_GEN_TOO_MANY  #language en-US "%H%s%N: Too many
> arguments\r\n"
>  #string STR_GEN_FILE_OPEN_FAIL#language en-US "%H%s%N: Cannot
> open file - '%H%s%N'\r\n"
>  #string STR_GEN_FIND_FAIL #language en-US "%H%s%N: File not found
> - '%H%s%N'\r\n"
>  #string STR_GEN_OUT_MEM   #language en-US "%H%s%N: Memory
> allocation was not successful.\r\n"
>  #string STR_BCFG_WRITE_FAIL   #language en-US "%H%s%N: Unable to
> write to '%H%s%N'\r\n"
>  #string STR_BCFG_READ_FAIL#language en-US "%H%s%N: Unable to
> read from '%H%s%N'\r\n"
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" command

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Bi, Dandan
> Sent: Wednesday, August 31, 2016 6:09 PM
> To: Carsey, Jaben ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: RE: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg"
> command
> Importance: High
> 
> Our codes have already checked for "too few " case, and it will print " Too 
> few
> arguments" in this situation.
> 
> Thanks,
> Dandan
> 
> -Original Message-
> From: Carsey, Jaben
> Sent: Thursday, September 1, 2016 12:12 AM
> To: Bi, Dandan ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben 
> Subject: RE: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg"
> command
> 
> Should we check for too few also?  What if the user sends 2 params?
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Dandan Bi
> > Sent: Tuesday, August 30, 2016 10:27 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Carsey, Jaben
> > 
> > Subject: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg"
> > command
> > Importance: High
> >
> > When user uses the command "bcfg driver|boot [dump [-v]]", the number
> > of command line value parameters (doesn't include the
> > flag) must be three. We can add this point to check whether using this
> > command correctly.
> >
> > Cc: Ruiyu Ni 
> > Cc: Jaben Carsey 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Dandan Bi 
> > ---
> >  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c   |
> > 4 
> >  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> > |
> > 3 ++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > index 9baeecc..e2306bf 100644
> > ---
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > +++
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > @@ -1294,10 +1294,14 @@ ShellCommandRunBcfg (
> >  if (ShellStatus == SHELL_SUCCESS && CurrentOperation.Target <
> > BcfgTargetMax) {
> >for (ParamNumber = 2 ; ParamNumber <
> > ShellCommandLineGetCount(Package) && ShellStatus == SHELL_SUCCESS;
> > ParamNumber++) {
> >  CurrentParam = ShellCommandLineGetRawValue(Package,
> > ParamNumber);
> >  if(gUnicodeCollation->StriColl(gUnicodeCollation,
> > (CHAR16*)CurrentParam, L"dump") == 0){
> >CurrentOperation.Type = BcfgTypeDump;
> > +  if (ShellCommandLineGetCount(Package) > 3) {
> > +ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> > + (STR_GEN_TOO_MANY),
> > gShellBcfgHiiHandle, L"bcfg");
> > +ShellStatus = SHELL_INVALID_PARAMETER;
> > +  }
> >  } else if (ShellCommandLineGetFlag(Package, L"-v")) {
> >ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> > (STR_GEN_PARAM_INV), gShellBcfgHiiHandle, L"bcfg", L"-v (without dump)");
> >ShellStatus = SHELL_INVALID_PARAMETER;
> >  } else if (gUnicodeCollation->StriColl(gUnicodeCollation,
> > (CHAR16*)CurrentParam, L"add") == 0) {
> >if ((ParamNumber + 3) >= ShellCommandLineGetCount(Package))
> > { diff --git
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> > index 282494b..7668e49 100644
> > ---
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> > +++
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> > @@ -1,9 +1,9 @@
> >  // /**
> >  //
> >  // (C) Copyright 2014-2015 Hewlett-Packard Development Company,
> > L.P. -// Copyright (c) 2010 - 2014, Intel Corporation. All rights
> > reserved.
> > +// Copyright (c) 2010 - 2016, Intel Corporation. All rights
> > +reserved.
> >  // This program and the accompanying materials  // are licensed and
> > made available under the terms and conditions of the BSD License  //
> > which accompanies this distribution. The full text of the license may
> > be found at  // http://opensource.org/licenses/bsd-license.php
> >  //
> > @@ -30,10 +30,11 @@
> >  #string STR_GEN_NO_VALUE  #language en-US "%H%s%N: Missing
> > argument for flag - '%H%s%N'\r\n"
> >  #string STR_GEN_PARAM_INV #language en-US "%H%s%N: Invalid
> > argument - '%H%s%N'\r\n"
> >  #string STR_GEN_NO_DRIVER_BOOT#language en-US "%H%s%N: Driver
> > or Boot must be selected.\r\n"
> >  #string STR_GEN_BOOT_ONLY #language en-US "%H%s%N: Boot must
> > be selected for hot key options.\r\n"
> >  #string STR_GEN_TOO_FEW   

Re: [edk2] [patch] ShellPkg: Add the check of parameter number in "DrvCfg" command

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 
> -Original Message-
> From: Bi, Dandan
> Sent: Wednesday, August 31, 2016 1:27 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben 
> Subject: [patch] ShellPkg: Add the check of parameter number in "DrvCfg"
> command
> 
> In shell spec, the usage of "Drvcfg" command is: drvcfg [-l XXX] [-c] [-f
> |-v|-s] [DriverHandle [DeviceHandle [ChildHandle]]] [-i filename] [-o
> filename]. The parameter number(doesn't include the flags) cannot exceed 4,
> now we add this point to check whether using the command correctly.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> index 0d12f01..cc1c9ca 100644
> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> @@ -1210,10 +1210,15 @@ ShellCommandRunDrvCfg (
>  ASSERT(FALSE);
>}
>  }
>}
>if (ShellStatus == SHELL_SUCCESS) {
> +if (ShellCommandLineGetCount(Package) > 4) {
> +  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellDriver1HiiHandle, L"drvcfg");
> +  ShellStatus = SHELL_INVALID_PARAMETER;
> +  goto Done;
> +}
>  Lang = ShellCommandLineGetValue(Package, L"-l");
>  if (Lang != NULL) {
>Language = AllocateZeroPool(StrSize(Lang));
>AsciiSPrint(Language, StrSize(Lang), "%S", Lang);
>  } else if (ShellCommandLineGetFlag(Package, L"-l")){
> --
> 1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency

[Fan, Jeff]

Laszlo,

UefiCpuPkg/PiSmmCpuDxeSmm driver and UefiCpuPkg/Library/SmmCpuFeatuersLib have 
no such requirement on gEfiVariableArchProtocolGuid  to access HII type PCD.
In fact, our platform SmmCpuFeaturesLib instance (linked by PiSmmCpuDxeSmm) is 
trying to read HII type PCD.

The correct solution is to add gEfiVariableArchProtocolGuid  dependency in 
platform SmmCpuFeaturesLib instance and this dependency will be inherited by 
PiSmmCpuDxeSmm driver.

We will drop this patch #48 eventually.  It has been reverted suggested by you! 
:-)

Thanks your support on it.

Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, August 30, 2016 9:46 PM
To: Fan, Jeff; Zeng, Star; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Justen, Jordan L; Tian, Feng
Subject: Re: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add 
gEfiVariableArchProtocolGuid dependency

On 08/25/16 10:00, Fan, Jeff wrote:
> Laszlo,
> 
> After discussed with Star, I understood OVMF's circle dependency on Variable 
> Arch protocol and SMM CPU driver.
> 
> I will defer to check-in this patch till found the better solution.

Thank you!

> Before the proper fix adopted, our platforms might not set the HII types 
> dynamic PCDs used by PiSmmCpuDxeSmm driver.

If I understand correctly, such dynamic PCDs are stored in UEFI variables, 
which are in turn exposed via HII (forms). Based on 
"MdeModulePkg/Universal/PCD/Dxe/Pcd.inf", point 2.3 (b).

It seems to me that "Default Storage" vs. "Variable Storage" is only separated 
in the platform DSC file; the declaration for the dynamic PCDs in question 
should be identical in the package DEC file.

So maybe PiSmmCpuDxeSmm should only read the new dynamic PCDs, and if those are 
expected to come from UEFI variables, then the platform could plug a NULL 
library instance into PiSmmCpuDxeSmm that "imbued"
PiSmmCpuDxeSmm with a depex on gEfiVariableArchProtocolGuid.

Alternatively, maybe SmmCpuPlatformHookLib could be extended with a new 
function (or functions) for querying these dynamic settings. OvmfPkg could 
provide a library instance that returned constant defaults (or even dynamic 
PCDs, but without variable access). And UefiCpuPkg could provide a library 
instance with variable-backed PCDs, plus the depex.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" command

[Bi, Dandan]

Our codes have already checked for "too few " case, and it will print " Too few 
arguments" in this situation.

Thanks,
Dandan

-Original Message-
From: Carsey, Jaben 
Sent: Thursday, September 1, 2016 12:12 AM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Carsey, Jaben 
Subject: RE: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" 
command

Should we check for too few also?  What if the user sends 2 params?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Dandan Bi
> Sent: Tuesday, August 30, 2016 10:27 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben 
> 
> Subject: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg"
> command
> Importance: High
> 
> When user uses the command "bcfg driver|boot [dump [-v]]", the number 
> of command line value parameters (doesn't include the
> flag) must be three. We can add this point to check whether using this 
> command correctly.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c   |
> 4 
>  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni 
> |
> 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> index 9baeecc..e2306bf 100644
> ---
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> @@ -1294,10 +1294,14 @@ ShellCommandRunBcfg (
>  if (ShellStatus == SHELL_SUCCESS && CurrentOperation.Target <
> BcfgTargetMax) {
>for (ParamNumber = 2 ; ParamNumber <
> ShellCommandLineGetCount(Package) && ShellStatus == SHELL_SUCCESS;
> ParamNumber++) {
>  CurrentParam = ShellCommandLineGetRawValue(Package,
> ParamNumber);
>  if(gUnicodeCollation->StriColl(gUnicodeCollation,
> (CHAR16*)CurrentParam, L"dump") == 0){
>CurrentOperation.Type = BcfgTypeDump;
> +  if (ShellCommandLineGetCount(Package) > 3) {
> +ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN 
> + (STR_GEN_TOO_MANY),
> gShellBcfgHiiHandle, L"bcfg");
> +ShellStatus = SHELL_INVALID_PARAMETER;
> +  }
>  } else if (ShellCommandLineGetFlag(Package, L"-v")) {
>ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN 
> (STR_GEN_PARAM_INV), gShellBcfgHiiHandle, L"bcfg", L"-v (without dump)");
>ShellStatus = SHELL_INVALID_PARAMETER;
>  } else if (gUnicodeCollation->StriColl(gUnicodeCollation,
> (CHAR16*)CurrentParam, L"add") == 0) {
>if ((ParamNumber + 3) >= ShellCommandLineGetCount(Package)) 
> { diff --git 
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> index 282494b..7668e49 100644
> ---
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
> @@ -1,9 +1,9 @@
>  // /**
>  //
>  // (C) Copyright 2014-2015 Hewlett-Packard Development Company, 
> L.P. -// Copyright (c) 2010 - 2014, Intel Corporation. All rights 
> reserved.
> +// Copyright (c) 2010 - 2016, Intel Corporation. All rights 
> +reserved.
>  // This program and the accompanying materials  // are licensed and 
> made available under the terms and conditions of the BSD License  // 
> which accompanies this distribution. The full text of the license may 
> be found at  // http://opensource.org/licenses/bsd-license.php
>  //
> @@ -30,10 +30,11 @@
>  #string STR_GEN_NO_VALUE  #language en-US "%H%s%N: Missing
> argument for flag - '%H%s%N'\r\n"
>  #string STR_GEN_PARAM_INV #language en-US "%H%s%N: Invalid
> argument - '%H%s%N'\r\n"
>  #string STR_GEN_NO_DRIVER_BOOT#language en-US "%H%s%N: Driver
> or Boot must be selected.\r\n"
>  #string STR_GEN_BOOT_ONLY #language en-US "%H%s%N: Boot must
> be selected for hot key options.\r\n"
>  #string STR_GEN_TOO_FEW   #language en-US "%H%s%N: Too few
> arguments.\r\n"
> +#string STR_GEN_TOO_MANY  #language en-US "%H%s%N: Too many
> arguments\r\n"
>  #string STR_GEN_FILE_OPEN_FAIL#language en-US "%H%s%N: Cannot
> open file - '%H%s%N'\r\n"
>  #string STR_GEN_FIND_FAIL #language en-US "%H%s%N: File not found
> - '%H%s%N'\r\n"
>  #string STR_GEN_OUT_MEM   #language en-US "%H%s%N: Memory
> allocation was not successful.\r\n"
>  #string STR_BCFG_WRITE_FAIL   #language en-US "%H%s%N: Unable to
> write to '%H%s%N'\r\n"
>  #string 

Re: [edk2] UnicodeError: UTF-16 stream does not start with BOM error when update from UDK2015 to latest Github trunk

[Gao, Liming]

Matthew:
  The temp way is to convert all uni files to UTF16 format. You can use edk2 
trunk BaseTools\Scripts\ConvertUni.py to convert uni file format. 

Thanks
Liming
> -Original Message-
> From: Foster, Matthew I
> Sent: Thursday, September 01, 2016 3:04 AM
> To: Gao, Liming ; wang xiaofeng
> ; edk2-devel@lists.01.org
> Subject: RE: [edk2] UnicodeError: UTF-16 stream does not start with BOM
> error when update from UDK2015 to latest Github trunk
> 
> Is there a way to work-around this error without having to upgrade to the
> newest BaseTools?
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Monday, February 29, 2016 11:08 PM
> To: wang xiaofeng ; edk2-devel@lists.01.org
> Subject: Re: [edk2] UnicodeError: UTF-16 stream does not start with BOM
> error when update from UDK2015 to latest Github trunk
> 
> Do you get the latest BaseTools binary win32?
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > wang xiaofeng
> > Sent: Tuesday, March 01, 2016 2:04 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] UnicodeError: UTF-16 stream does not start with BOM
> > error when update from UDK2015 to latest Github trunk
> >
> > Hi All Edk2 developer,
> >  I meet a build error when update code from UDK2015 to latest
> > Github trunk. Do anyone meet similar issue before? I sync NetworkPkg
> > seperately to latest code but find the following error:
> >
> >
> > build...
> >  : error C0DE: Unknown fatal error when processing
> > [e:\code\cl174\NetworkPkg\Ip6Dxe\Ip6Dxe.inf]
> > (Please send email to edk2-de...@lists.sourceforge.net for help,
> > attaching following call stack trace!)
> >
> >
> > (Python 2.7.3 on win32) Traceback (most recent call last):
> >   File "build.py", line 2032, in Main
> >   File "build.py", line 1788, in Launch
> >   File "build.py", line 1618, in _MultiThreadBuildPlatform
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\AutoGen.py", line 3446, in CreateCodeFile
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\AutoGen.py", line 2854, in _GetAutoGenFileList
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\GenC.py", line 1552, in CreateCode
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\GenC.py", line 1456, in CreateUnicodeStringCode
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\StrGather.py", line 600, in GetStringFiles
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\UniClassObject.py", line 203, in __init__
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\UniClassObject.py", line 463, in LoadUniFiles
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\UniClassObject.py", line 377, in LoadUniFile
> >   File
> >
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> > oGen\UniClassObject.py", line 318, in PreProcess
> >   File "C:\Python\32-bit\2.7\lib\codecs.py", line 684, in next
> >   File "C:\Python\32-bit\2.7\lib\codecs.py", line 615, in next
> >   File "C:\Python\32-bit\2.7\lib\codecs.py", line 530, in readline
> >   File "C:\Python\32-bit\2.7\lib\codecs.py", line 477, in read
> >   File "C:\Python\32-bit\2.7\lib\encodings\utf_16.py", line 112, in
> > decode
> > UnicodeError: UTF-16 stream does not start with BOM
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Crc32

[valerij zaporogeci]

>> I was thinking that all the difference in the Tianocore impl. from the
>> pure crc is appending 32 1's at the beginning of the (input) message
>> and then negating the Crc itself in the end.

Thank you, Michael and Andrew for your help. I figured out what causes
the difference.
Apart from initializing crc with  at the beginning and
negating the resulting crc at the end, the difference is that
Tianocore impl. reverses bits in the byte sequence (of the input),
thus bit 7 of every byte becomes bit 0 and bit 0 - bit 7. And also in
the end, resulting Crc is also reverted bitwise (bit 31->0, 0->31).
Not that I understood everything, but applying these inversions into
the pure remainder calculation, gives matching with the Tianocore
function results.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] UnicodeError: UTF-16 stream does not start with BOM error when update from UDK2015 to latest Github trunk

[Foster, Matthew I]

Is there a way to work-around this error without having to upgrade to the 
newest BaseTools? 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao, 
Liming
Sent: Monday, February 29, 2016 11:08 PM
To: wang xiaofeng ; edk2-devel@lists.01.org
Subject: Re: [edk2] UnicodeError: UTF-16 stream does not start with BOM error 
when update from UDK2015 to latest Github trunk

Do you get the latest BaseTools binary win32?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> wang xiaofeng
> Sent: Tuesday, March 01, 2016 2:04 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] UnicodeError: UTF-16 stream does not start with BOM 
> error when update from UDK2015 to latest Github trunk
> 
> Hi All Edk2 developer,
>  I meet a build error when update code from UDK2015 to latest 
> Github trunk. Do anyone meet similar issue before? I sync NetworkPkg 
> seperately to latest code but find the following error:
> 
> 
> build...
>  : error C0DE: Unknown fatal error when processing 
> [e:\code\cl174\NetworkPkg\Ip6Dxe\Ip6Dxe.inf]
> (Please send email to edk2-de...@lists.sourceforge.net for help, 
> attaching following call stack trace!)
> 
> 
> (Python 2.7.3 on win32) Traceback (most recent call last):
>   File "build.py", line 2032, in Main
>   File "build.py", line 1788, in Launch
>   File "build.py", line 1618, in _MultiThreadBuildPlatform
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\AutoGen.py", line 3446, in CreateCodeFile
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\AutoGen.py", line 2854, in _GetAutoGenFileList
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\GenC.py", line 1552, in CreateCode
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\GenC.py", line 1456, in CreateUnicodeStringCode
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\StrGather.py", line 600, in GetStringFiles
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\UniClassObject.py", line 203, in __init__
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\UniClassObject.py", line 463, in LoadUniFiles
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\UniClassObject.py", line 377, in LoadUniFile
>   File
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Aut
> oGen\UniClassObject.py", line 318, in PreProcess
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 684, in next
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 615, in next
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 530, in readline
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 477, in read
>   File "C:\Python\32-bit\2.7\lib\encodings\utf_16.py", line 112, in 
> decode
> UnicodeError: UTF-16 stream does not start with BOM 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 5/6] ArmVirtPkg/FdtPciHostBridgeLib: add MMIO64 support

[Ard Biesheuvel]

If the pci-host-ecam-generic DT node describes a 64-bit MMIO region,
account for it in the PCI_ROOT_BRIDGE description that we return to
the generic PciHostBridgeDxe implementation, which will be able to
allocate BARs from it without any further changes.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=65
---
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 78 
++--
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  1 +
 2 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index c2aa4a339c19..efccedcca14f 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -87,8 +87,10 @@ EFI_STATUS
 ProcessPciHost (
   OUT  UINT64*IoBase,
   OUT  UINT64*IoSize,
-  OUT  UINT64*MmioBase,
-  OUT  UINT64*MmioSize,
+  OUT  UINT64*Mmio32Base,
+  OUT  UINT64*Mmio32Size,
+  OUT  UINT64*Mmio64Base,
+  OUT  UINT64*Mmio64Size,
   OUT  UINT32*BusMin,
   OUT  UINT32*BusMax
   )
@@ -101,7 +103,8 @@ ProcessPciHost (
   UINT32  RecordIdx;
   EFI_STATUS  Status;
   UINT64  IoTranslation;
-  UINT64  MmioTranslation;
+  UINT64  Mmio32Translation;
+  UINT64  Mmio64Translation;
 
   //
   // The following output arguments are initialized only in
@@ -109,17 +112,19 @@ ProcessPciHost (
   // *incorrectly* emitted by some gcc versions.
   //
   *IoBase = 0;
-  *MmioBase = 0;
+  *Mmio32Base = 0;
+  *Mmio64Base = MAX_UINT64;
   *BusMin = 0;
   *BusMax = 0;
 
   //
-  // *IoSize, *MmioSize and IoTranslation are initialized to zero because the
+  // *IoSize, *Mmio##Size and IoTranslation are initialized to zero because the
   // logic below requires it. However, since they are also affected by the 
issue
   // reported above, they are initialized early.
   //
   *IoSize = 0;
-  *MmioSize = 0;
+  *Mmio32Size = 0;
+  *Mmio64Size = 0;
   IoTranslation = 0;
 
   Status = gBS->LocateProtocol (, NULL,
@@ -209,28 +214,43 @@ ProcessPciHost (
   break;
 
 case DTB_PCI_HOST_RANGE_MMIO32:
-  *MmioBase = SwapBytes64 (Record->ChildBase);
-  *MmioSize = SwapBytes64 (Record->Size);
-  MmioTranslation = SwapBytes64 (Record->CpuBase) - *MmioBase;
+  *Mmio32Base = SwapBytes64 (Record->ChildBase);
+  *Mmio32Size = SwapBytes64 (Record->Size);
+  Mmio32Translation = SwapBytes64 (Record->CpuBase) - *Mmio32Base;
 
-  if (*MmioBase > MAX_UINT32 || *MmioSize > MAX_UINT32 ||
-  *MmioBase + *MmioSize > SIZE_4GB) {
+  if (*Mmio32Base > MAX_UINT32 || *Mmio32Size > MAX_UINT32 ||
+  *Mmio32Base + *Mmio32Size > SIZE_4GB) {
 DEBUG ((EFI_D_ERROR, "%a: MMIO32 space invalid\n", __FUNCTION__));
 return EFI_PROTOCOL_ERROR;
   }
 
-  ASSERT (PcdGet64 (PcdPciMmio32Translation) == MmioTranslation);
+  ASSERT (PcdGet64 (PcdPciMmio32Translation) == Mmio32Translation);
 
-  if (MmioTranslation != 0) {
+  if (Mmio32Translation != 0) {
 DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO32 translation "
-  "0x%Lx\n", __FUNCTION__, MmioTranslation));
+  "0x%Lx\n", __FUNCTION__, Mmio32Translation));
+return EFI_UNSUPPORTED;
+  }
+
+  break;
+
+case DTB_PCI_HOST_RANGE_MMIO64:
+  *Mmio64Base = SwapBytes64 (Record->ChildBase);
+  *Mmio64Size = SwapBytes64 (Record->Size);
+  Mmio64Translation = SwapBytes64 (Record->CpuBase) - *Mmio64Base;
+
+  ASSERT (PcdGet64 (PcdPciMmio64Translation) == Mmio64Translation);
+
+  if (Mmio64Translation != 0) {
+DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO64 translation "
+  "0x%Lx\n", __FUNCTION__, Mmio64Translation));
 return EFI_UNSUPPORTED;
   }
 
   break;
 }
   }
-  if (*IoSize == 0 || *MmioSize == 0) {
+  if (*IoSize == 0 || *Mmio32Size == 0) {
 DEBUG ((EFI_D_ERROR, "%a: %a space empty\n", __FUNCTION__,
   (*IoSize == 0) ? "IO" : "MMIO32"));
 return EFI_PROTOCOL_ERROR;
@@ -243,9 +263,9 @@ ProcessPciHost (
   ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase);
 
   DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] "
-"Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x0\n", __FUNCTION__, ConfigBase,
-ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, IoTranslation, *MmioBase,
-*MmioSize));
+"Io[0x%Lx+0x%Lx)@0x%Lx Mem32[0x%Lx+0x%Lx)@0x0 Mem64[0x%Lx+0x%Lx)@0x0\n",
+__FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize,
+IoTranslation, *Mmio32Base, *Mmio32Size, *Mmio64Base, *Mmio64Size));
   return EFI_SUCCESS;
 }
 
@@ -268,6 +288,7 @@ PciHostBridgeGetRootBridges (
 

[edk2] [PATCH v2 4/6] ArmVirtPkg/ArmVirtQemu: switch to generic PciHostBridgeDxe

[Ard Biesheuvel]

Wire up the FdtPciHostBridgeLib introduced in the previous patch
to the generic PciHostBridgeDxe implementation, and drop the special
ArmVirtPkg version. The former's dependency on gEfiCpuIo2ProtocolGuid
is satisfied by adding ArmPciCpuIo2Dxe.inf as well, and adding the PCD
gArmTokenSpaceGuid.PcdPciIoTranslation as a dynamic PCD.

In terms of functionality, the only effect this change should have is
that we will no longer use bounce buffers for DMA above 4 GB. Other
than that, no functional changes are intended.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=65
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 10 +-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc |  3 ++-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 10 +-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index f8b61402625a..c503ef243f9a 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -69,6 +69,8 @@ [LibraryClasses.common]
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+  
PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -210,6 +212,8 @@ [PcdsDynamicDefault.common]
   # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
 
+  gArmTokenSpaceGuid.PcdPciIoTranslation|0x0
+
   #
   # Set video resolution for boot options and for text setup.
   # PlatformDxe can set the former at runtime.
@@ -363,7 +367,11 @@ [Components.common]
   #
   # PCI support
   #
-  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
+  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
 
   NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 900fa54660aa..2571884b20ac 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -156,7 +156,8 @@ [FV.FvMain]
   #
   # PCI support
   #
-  INF ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
+  INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
   INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   INF OvmfPkg/Virtio10Dxe/Virtio10.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index ea722ddf8ea7..383d9b7d2c0b 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -71,6 +71,8 @@ [LibraryClasses.common]
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+  
PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -203,6 +205,8 @@ [PcdsDynamicDefault.common]
   # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
 
+  gArmTokenSpaceGuid.PcdPciIoTranslation|0x0
+
   #
   # Set video resolution for boot options and for text setup.
   # PlatformDxe can set the former at runtime.
@@ -349,7 +353,11 @@ [Components.common]
   #
   # PCI support
   #
-  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+  ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
+  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
 
   NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 3/6] ArmVirtPkg: implement FdtPciHostBridgeLib

[Ard Biesheuvel]

Implement PciHostBridgeLib for DT platforms that expose a PCI root bridge
via a pci-host-ecam-generic DT node. The DT parsing logic is copied from
the PciHostBridgeDxe implementation in ArmVirtPkg, with the one notable
difference that we don't set the various legacy PCI attributes for IDE
and VGA I/O ranges.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=65
---
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 400 

 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  56 +++
 2 files changed, 456 insertions(+)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
new file mode 100644
index ..c2aa4a339c19
--- /dev/null
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -0,0 +1,400 @@
+/** @file
+  PCI Host Bridge Library instance for pci-ecam-generic DT nodes
+
+  Copyright (c) 2016, Linaro Ltd. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+  IMPLIED.
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#pragma pack(1)
+typedef struct {
+  ACPI_HID_DEVICE_PATH AcpiDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL EndDevicePath;
+} EFI_PCI_ROOT_BRIDGE_DEVICE_PATH;
+#pragma pack ()
+
+STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mEfiPciRootBridgeDevicePath = {
+  {
+{
+  ACPI_DEVICE_PATH,
+  ACPI_DP,
+  {
+(UINT8) (sizeof(ACPI_HID_DEVICE_PATH)),
+(UINT8) ((sizeof(ACPI_HID_DEVICE_PATH)) >> 8)
+  }
+},
+EISA_PNP_ID(0x0A08), // PCI Express
+0
+  },
+
+  {
+END_DEVICE_PATH_TYPE,
+END_ENTIRE_DEVICE_PATH_SUBTYPE,
+{
+  END_DEVICE_PATH_LENGTH,
+  0
+}
+  }
+};
+
+GLOBAL_REMOVE_IF_UNREFERENCED
+CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
+  L"Mem", L"I/O", L"Bus"
+};
+
+//
+// We expect the "ranges" property of "pci-host-ecam-generic" to consist of
+// records like this.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 Type;
+  UINT64 ChildBase;
+  UINT64 CpuBase;
+  UINT64 Size;
+} DTB_PCI_HOST_RANGE_RECORD;
+#pragma pack ()
+
+#define DTB_PCI_HOST_RANGE_RELOCATABLE  BIT31
+#define DTB_PCI_HOST_RANGE_PREFETCHABLE BIT30
+#define DTB_PCI_HOST_RANGE_ALIASED  BIT29
+#define DTB_PCI_HOST_RANGE_MMIO32   BIT25
+#define DTB_PCI_HOST_RANGE_MMIO64   (BIT25 | BIT24)
+#define DTB_PCI_HOST_RANGE_IO   BIT24
+#define DTB_PCI_HOST_RANGE_TYPEMASK (BIT31 | BIT30 | BIT29 | BIT25 | BIT24)
+
+STATIC
+EFI_STATUS
+ProcessPciHost (
+  OUT  UINT64*IoBase,
+  OUT  UINT64*IoSize,
+  OUT  UINT64*MmioBase,
+  OUT  UINT64*MmioSize,
+  OUT  UINT32*BusMin,
+  OUT  UINT32*BusMax
+  )
+{
+  FDT_CLIENT_PROTOCOL *FdtClient;
+  INT32   Node;
+  UINT64  ConfigBase, ConfigSize;
+  CONST VOID  *Prop;
+  UINT32  Len;
+  UINT32  RecordIdx;
+  EFI_STATUS  Status;
+  UINT64  IoTranslation;
+  UINT64  MmioTranslation;
+
+  //
+  // The following output arguments are initialized only in
+  // order to suppress '-Werror=maybe-uninitialized' warnings
+  // *incorrectly* emitted by some gcc versions.
+  //
+  *IoBase = 0;
+  *MmioBase = 0;
+  *BusMin = 0;
+  *BusMax = 0;
+
+  //
+  // *IoSize, *MmioSize and IoTranslation are initialized to zero because the
+  // logic below requires it. However, since they are also affected by the 
issue
+  // reported above, they are initialized early.
+  //
+  *IoSize = 0;
+  *MmioSize = 0;
+  IoTranslation = 0;
+
+  Status = gBS->LocateProtocol (, NULL,
+  (VOID **));
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNode (FdtClient, "pci-host-ecam-generic",
+);
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_INFO,
+  "%a: No 'pci-host-ecam-generic' compatible DT node found\n",
+  __FUNCTION__));
+return EFI_NOT_FOUND;
+  }
+
+  DEBUG_CODE (
+INT32 Tmp;
+
+//
+// A DT can legally describe multiple PCI host bridges, but we are not
+// equipped to deal with that. So assert that there is only one.
+//
+Status = FdtClient->FindNextCompatibleNode (FdtClient,
+  "pci-host-ecam-generic", Node, );
+ASSERT (Status == EFI_NOT_FOUND);
+  );
+
+  Status = FdtClient->GetNodeProperty (FdtClient, Node, 

[edk2] [PATCH v2 0/6] ArmVirtQemu: move to generic PciHostBridgeDxe

[Ard Biesheuvel]

Now that Laszlo's virtio-gpu-pci series has removed the last remaining obstacle,
we can get rid of the special PciHostBridgeDxe implementation in ArmVirtPkg,
and move to the generic one. On AArch64, this will allow us to perform DMA above
4GB without bounce buffering, and use 64-bit MMIO BARs allocated above 4 GB.

Changes since v1:
- new patch #2 to move the IoTranslation discovery to FdtPciPcdProducerLib,
  which is a cleaner approach since other drivers (i.e., ArmPciCpuIo2Dxe)
  depend on it as well
- add support for ARM, i.e., disable the 64-bit range in that case, since we
  cannot access 64-bit MMIO BARs if they are allocated there
- use statically allocated PCI_ROOT_BRIDGE[] array of size 1
- enable support for ISA and VGA I/O range decoding
- various other minor fixes based on Laszlo's review comments
- added ref links and Laszlo's acks where appropriate, i.e., where given and
  where the version of the patch in this series does not deviate substantially
  from the suggested version on which the preliminary ack was based

Patch #1 removes the linux,pci-probe-only override which does more harm than
good now that we switched to virtio-gpu-pci, which does not expose a raw
framebuffer.

Patch #2 extends FdtPciPcdProducerLib so that it also sets PcdPciIoTranslation,
which is required for the FdtPciHostBridgeLib implementation this series
introduces, but also for ArmPciCpuIo2Dxe, which produces EFI_CPU_IO2_PROTOCOL
on which the generic PciHostBridgeDxe depends as well.

Patch #3 implements PciHostBridgeLib for platforms exposing a PCI host bridge
using a pci-host-ecam-generic DT node. The initial version is based on the
ArmVirtPkg implementation of PciHostBridgeDxe, so it does not support 64-bit
MMIO BARs allocated above 4 GB, but it does support DMA above 4 GB without
bounce buffering.

Patch #4 switches to the generic PciHostBridgeDxe, with no change in
functionality other than support for DMA above 4 GB without bounce buffering.

Patch #5 adds support for allocating 64-bit MMIO BARs above 4 GB.

Patch #6 removes the now obsolete PciHostBridgeDxe from ArmVirtPkg.


Ard Biesheuvel (6):
  ArmVirtPkg/PciHostBridgeDxe: don't set linux,pci-probe-only DT
property
  ArmVirtPkg/FdtPciPcdProducerLib: add handling of PcdPciIoTranslation
  ArmVirtPkg: implement FdtPciHostBridgeLib
  ArmVirtPkg/ArmVirtQemu: switch to generic PciHostBridgeDxe
  ArmVirtPkg/FdtPciHostBridgeLib: add MMIO64 support
  ArmVirtPkg: remove now unused PciHostBridgeDxe

 ArmVirtPkg/ArmVirtQemu.dsc   |   10 +-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc |3 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc |   10 +-
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  434 
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf   |   57 +
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   |  108 +-
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf |2 +
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 1496 
--
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |  499 -
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |   64 -
 ArmVirtPkg/PciHostBridgeDxe/PciRootBridgeIo.c| 2144 

 11 files changed, 611 insertions(+), 4216 deletions(-)
 create mode 100644 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
 create mode 100644 
ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 delete mode 100644 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
 delete mode 100644 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h
 delete mode 100644 ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
 delete mode 100644 ArmVirtPkg/PciHostBridgeDxe/PciRootBridgeIo.c

-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 1/6] ArmVirtPkg/PciHostBridgeDxe: don't set linux, pci-probe-only DT property

[Ard Biesheuvel]

Setting the linux,pci-probe-only was intended to align OSes booting via
DT with OSes booting via ACPI in the way they honor the PCI configuration
performed by the firmware. However, ACPI on arm64 does not currently honor
the firmware's PCI configuration, and the linux,pci-probe-only completely
prevents any PCI reconfiguration from occurring under the OS, including
what is needed to support PCI hotplug.

Since the primary use case was OS access to the GOP framebuffer (which
breaks when the framebuffer BAR is moved when the OS reconfigures the
PCI), we can undo this change now that ArmVirtQemu has moved to a GOP
implementation that does not expose a raw framebuffer in the first place.

This effectively reverts commit 8b816c624dd4 ("ArmVirtPkg/VirtFdtDxe: set
/chosen/linux,pci-probe-only to 1 in DTB")

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=65
---
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 38 
 1 file changed, 38 deletions(-)

diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c 
b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
index 5063782bb392..669c90355889 100644
--- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
+++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
@@ -79,42 +79,6 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = {
 // Implementation
 //
 
-STATIC
-VOID
-SetLinuxPciProbeOnlyProperty (
-  IN FDT_CLIENT_PROTOCOL *FdtClient
-  )
-{
-  INT32 Node;
-  UINT32Tmp;
-  EFI_STATUSStatus;
-
-  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
-//
-// Set the /chosen/linux,pci-probe-only property to 1, so that the PCI
-// setup we will perform in the firmware is honored by the Linux OS,
-// rather than torn down and done from scratch. This is generally a more
-// sensible approach, and aligns with what ACPI based OSes do typically.
-//
-// In case we are exposing an emulated VGA PCI device to the guest, which
-// may subsequently get exposed via the Graphics Output protocol and
-// driven as an efifb by Linux, we need this setting to prevent the
-// framebuffer from becoming unresponsive.
-//
-Status = FdtClient->GetOrInsertChosenNode (FdtClient, );
-
-if (!EFI_ERROR (Status)) {
-  Tmp = SwapBytes32 (1);
-  Status = FdtClient->SetNodeProperty (FdtClient, Node,
-"linux,pci-probe-only", , sizeof (Tmp));
-}
-if (EFI_ERROR (Status)) {
-  DEBUG ((EFI_D_WARN,
-"Failed to set /chosen/linux,pci-probe-only property\n"));
-}
-  }
-}
-
 //
 // We expect the "ranges" property of "pci-host-ecam-generic" to consist of
 // records like this.
@@ -293,8 +257,6 @@ ProcessPciHost (
   //
   ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase);
 
-  SetLinuxPciProbeOnlyProperty (FdtClient);
-
   DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] "
 "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, ConfigBase,
 ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation, *MmioBase,
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 2/6] ArmVirtPkg/FdtPciPcdProducerLib: add handling of PcdPciIoTranslation

[Ard Biesheuvel]

Add handling of the PcdPciIoTranslation PCD, so that modules that include
this library via NULL resolution are guaranteed that it will be set before
they reference it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=65
---
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   | 108 
++--
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf |   2 +
 2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c 
b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
index cc60940d487c..862ee227fffa 100644
--- a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
+++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
@@ -22,6 +22,68 @@
 
 #include 
 
+//
+// We expect the "ranges" property of "pci-host-ecam-generic" to consist of
+// records like this.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 Type;
+  UINT64 ChildBase;
+  UINT64 CpuBase;
+  UINT64 Size;
+} DTB_PCI_HOST_RANGE_RECORD;
+#pragma pack ()
+
+#define DTB_PCI_HOST_RANGE_RELOCATABLE  BIT31
+#define DTB_PCI_HOST_RANGE_PREFETCHABLE BIT30
+#define DTB_PCI_HOST_RANGE_ALIASED  BIT29
+#define DTB_PCI_HOST_RANGE_MMIO32   BIT25
+#define DTB_PCI_HOST_RANGE_MMIO64   (BIT25 | BIT24)
+#define DTB_PCI_HOST_RANGE_IO   BIT24
+#define DTB_PCI_HOST_RANGE_TYPEMASK (BIT31 | BIT30 | BIT29 | BIT25 | BIT24)
+
+STATIC
+RETURN_STATUS
+GetPciIoTranslation (
+  IN  FDT_CLIENT_PROTOCOL *FdtClient,
+  IN  INT32   Node,
+  OUT UINT64  *IoTranslation
+  )
+{
+  UINT32RecordIdx;
+  CONST VOID*Prop;
+  UINT32Len;
+  EFI_STATUSStatus;
+  UINT64IoBase;
+
+  //
+  // Iterate over "ranges".
+  //
+  Status = FdtClient->GetNodeProperty (FdtClient, Node, "ranges", , );
+  if (EFI_ERROR (Status) || Len == 0 ||
+  Len % sizeof (DTB_PCI_HOST_RANGE_RECORD) != 0) {
+DEBUG ((EFI_D_ERROR, "%a: 'ranges' not found or invalid\n", __FUNCTION__));
+return RETURN_PROTOCOL_ERROR;
+  }
+
+  for (RecordIdx = 0; RecordIdx < Len / sizeof (DTB_PCI_HOST_RANGE_RECORD);
+   ++RecordIdx) {
+CONST DTB_PCI_HOST_RANGE_RECORD *Record;
+UINT32  Type;
+
+Record = (CONST DTB_PCI_HOST_RANGE_RECORD *)Prop + RecordIdx;
+Type = SwapBytes32 (Record->Type) & DTB_PCI_HOST_RANGE_TYPEMASK;
+if (Type == DTB_PCI_HOST_RANGE_IO) {
+  IoBase = SwapBytes64 (Record->ChildBase);
+  *IoTranslation = SwapBytes64 (Record->CpuBase) - IoBase;
+
+  return RETURN_SUCCESS;
+}
+  }
+  return RETURN_NOT_FOUND;
+}
+
 RETURN_STATUS
 EFIAPI
 FdtPciPcdProducerLibConstructor (
@@ -31,11 +93,21 @@ FdtPciPcdProducerLibConstructor (
   UINT64  PciExpressBaseAddress;
   FDT_CLIENT_PROTOCOL *FdtClient;
   CONST UINT64*Reg;
-  UINT32  RegElemSize, RegSize;
+  UINT32  RegSize;
   EFI_STATUS  Status;
+  INT32   Node;
+  RETURN_STATUS   RetStatus;
+  UINT64  IoTranslation;
 
   PciExpressBaseAddress = PcdGet64 (PcdPciExpressBaseAddress);
   if (PciExpressBaseAddress != MAX_UINT64) {
+//
+// Assume that the fact that PciExpressBaseAddress has been changed from
+// its default value of MAX_UINT64 implies that this code has been
+// executed already, in the context of another module. That means we can
+// assume that PcdPciIoTranslation has been discovered from the DT node
+// as well.
+//
 return EFI_SUCCESS;
   }
 
@@ -43,17 +115,33 @@ FdtPciPcdProducerLibConstructor (
   (VOID **));
   ASSERT_EFI_ERROR (Status);
 
-  Status = FdtClient->FindCompatibleNodeReg (FdtClient,
-"pci-host-ecam-generic", (CONST VOID **),
-, );
+  PciExpressBaseAddress = 0;
+  Status = FdtClient->FindCompatibleNode (FdtClient, "pci-host-ecam-generic",
+);
+
+  if (!EFI_ERROR (Status)) {
+Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",
+  (CONST VOID **), );
+
+if (!EFI_ERROR (Status) && RegSize == 2 * sizeof(UINT64)) {
+  PciExpressBaseAddress = SwapBytes64 (*Reg);
 
-  if (EFI_ERROR (Status)) {
-PciExpressBaseAddress = 0;
-  } else {
-ASSERT (RegElemSize == sizeof (UINT64));
-PciExpressBaseAddress = SwapBytes64 (*Reg);
+  PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
 
-PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
+  RetStatus = GetPciIoTranslation (FdtClient, Node, );
+  if (!RETURN_ERROR (RetStatus)) {
+  PcdSet64 (PcdPciIoTranslation, IoTranslation);
+  } else {
+//
+// Support for I/O BARs is not mandatory, and so it does not make sense
+// to abort in the general case. So leave it up to the actual driver to
+// complain about this 

Re: [edk2] [patch] ShellPkg: Add the check of parameter number in "DrvCfg" command

[Carsey, Jaben]

Seems like a good check.

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Shah, Tapan [mailto:tapands...@hpe.com]
> Sent: Wednesday, August 31, 2016 7:24 AM
> To: Bi, Dandan ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Carsey, Jaben 
> Subject: RE: [edk2] [patch] ShellPkg: Add the check of parameter number in
> "DrvCfg" command
> Importance: High
> 
> Reviewed-by: Tapan Shah 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Wednesday, August 31, 2016 12:27 AM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni ; Jaben Carsey 
> Subject: [edk2] [patch] ShellPkg: Add the check of parameter number in
> "DrvCfg" command
> 
> In shell spec, the usage of "Drvcfg" command is: drvcfg [-l XXX] [-c] [-f
> |-v|-s] [DriverHandle [DeviceHandle [ChildHandle]]] [-i filename] [-o
> filename]. The parameter number(doesn't include the flags) cannot exceed
> 4, now we add this point to check whether using the command correctly.
> 
> Cc: Ruiyu Ni 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> index 0d12f01..cc1c9ca 100644
> --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
> @@ -1210,10 +1210,15 @@ ShellCommandRunDrvCfg (
>  ASSERT(FALSE);
>}
>  }
>}
>if (ShellStatus == SHELL_SUCCESS) {
> +if (ShellCommandLineGetCount(Package) > 4) {
> +  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellDriver1HiiHandle, L"drvcfg");
> +  ShellStatus = SHELL_INVALID_PARAMETER;
> +  goto Done;
> +}
>  Lang = ShellCommandLineGetValue(Package, L"-l");
>  if (Lang != NULL) {
>Language = AllocateZeroPool(StrSize(Lang));
>AsciiSPrint(Language, StrSize(Lang), "%S", Lang);
>  } else if (ShellCommandLineGetFlag(Package, L"-l")){
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Crc32

[valerij zaporogeci]

yes. it does as you said. very confusing in fact.

2016-08-31 6:54 GMT+03:00, Michael Zimmermann :
> it doesn't. it just seems that if the crc text field is not empty it
> calculates the crc of the crc, so you have to hit the "Reset CRC" button
> before reprocessing the text.
>
> On Wed, Aug 31, 2016 at 3:11 AM, valerij zaporogeci 
> wrote:
>
>> >> after testing it it indeed produces CCITT32 results like this online
>> generator:
>> >> http://g6auc.me.uk/CRC32/index.html
>>
>> I only now noticed that this  "calculator", gives DIFFERENT values on
>> the same input, no matter hex or text based.
>> Interestingly, how it could produce the same results as the Tianocore
>> implementation?
>>
>> I was thinking that all the difference in the Tianocore impl. from the
>> pure crc is appending 32 1's at the beginning of the (input) message
>> and then negating the Crc itself in the end.
>>
>> I see, the only way to check is to pull off the Tianocore function and
>> check.)
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] ShellPkg: Add the check of parameter number in "DrvCfg" command

[Shah, Tapan]

Reviewed-by: Tapan Shah 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Wednesday, August 31, 2016 12:27 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Jaben Carsey 
Subject: [edk2] [patch] ShellPkg: Add the check of parameter number in "DrvCfg" 
command

In shell spec, the usage of "Drvcfg" command is: drvcfg [-l XXX] [-c] [-f 
|-v|-s] [DriverHandle [DeviceHandle [ChildHandle]]] [-i filename] [-o 
filename]. The parameter number(doesn't include the flags) cannot exceed 4, now 
we add this point to check whether using the command correctly.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c 
b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
index 0d12f01..cc1c9ca 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
@@ -1210,10 +1210,15 @@ ShellCommandRunDrvCfg (
 ASSERT(FALSE);
   }
 }
   } 
   if (ShellStatus == SHELL_SUCCESS) {
+if (ShellCommandLineGetCount(Package) > 4) {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), 
gShellDriver1HiiHandle, L"drvcfg");
+  ShellStatus = SHELL_INVALID_PARAMETER;
+  goto Done;
+}
 Lang = ShellCommandLineGetValue(Package, L"-l");
 if (Lang != NULL) {
   Language = AllocateZeroPool(StrSize(Lang));
   AsciiSPrint(Language, StrSize(Lang), "%S", Lang);
 } else if (ShellCommandLineGetFlag(Package, L"-l")){
--
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" command

[Shah, Tapan]

Reviewed-by: Tapan Shah 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Wednesday, August 31, 2016 12:27 AM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni ; Jaben Carsey 
Subject: [edk2] [patch] ShellPkg: Add check for "dump" parameter in "bcfg" 
command

When user uses the command "bcfg driver|boot [dump [-v]]", the number of 
command line value parameters (doesn't include the
flag) must be three. We can add this point to check whether using this command 
correctly.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c   | 4 
 ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c 
b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
index 9baeecc..e2306bf 100644
--- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
+++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
@@ -1294,10 +1294,14 @@ ShellCommandRunBcfg (
 if (ShellStatus == SHELL_SUCCESS && CurrentOperation.Target < 
BcfgTargetMax) {
   for (ParamNumber = 2 ; ParamNumber < ShellCommandLineGetCount(Package) 
&& ShellStatus == SHELL_SUCCESS; ParamNumber++) {
 CurrentParam = ShellCommandLineGetRawValue(Package, ParamNumber);
 if(gUnicodeCollation->StriColl(gUnicodeCollation, 
(CHAR16*)CurrentParam, L"dump") == 0){
   CurrentOperation.Type = BcfgTypeDump;
+  if (ShellCommandLineGetCount(Package) > 3) {
+ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), 
gShellBcfgHiiHandle, L"bcfg");
+ShellStatus = SHELL_INVALID_PARAMETER;
+  }
 } else if (ShellCommandLineGetFlag(Package, L"-v")) {
   ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellBcfgHiiHandle, L"bcfg", L"-v (without dump)");  
   ShellStatus = SHELL_INVALID_PARAMETER;
 } else if (gUnicodeCollation->StriColl(gUnicodeCollation, 
(CHAR16*)CurrentParam, L"add") == 0) {
   if ((ParamNumber + 3) >= ShellCommandLineGetCount(Package)) { diff 
--git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni 
b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
index 282494b..7668e49 100644
--- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.uni
+++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.u
+++ ni
@@ -1,9 +1,9 @@
 // /**
 //
 // (C) Copyright 2014-2015 Hewlett-Packard Development Company, L.P. -// 
Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
+// Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+reserved.
 // This program and the accompanying materials  // are licensed and made 
available under the terms and conditions of the BSD License  // which 
accompanies this distribution. The full text of the license may be found at  // 
http://opensource.org/licenses/bsd-license.php
 //
@@ -30,10 +30,11 @@
 #string STR_GEN_NO_VALUE  #language en-US "%H%s%N: Missing argument 
for flag - '%H%s%N'\r\n"
 #string STR_GEN_PARAM_INV #language en-US "%H%s%N: Invalid argument - 
'%H%s%N'\r\n"
 #string STR_GEN_NO_DRIVER_BOOT#language en-US "%H%s%N: Driver or Boot must 
be selected.\r\n"
 #string STR_GEN_BOOT_ONLY #language en-US "%H%s%N: Boot must be 
selected for hot key options.\r\n"
 #string STR_GEN_TOO_FEW   #language en-US "%H%s%N: Too few 
arguments.\r\n"
+#string STR_GEN_TOO_MANY  #language en-US "%H%s%N: Too many 
arguments\r\n"
 #string STR_GEN_FILE_OPEN_FAIL#language en-US "%H%s%N: Cannot open file - 
'%H%s%N'\r\n"
 #string STR_GEN_FIND_FAIL #language en-US "%H%s%N: File not found - 
'%H%s%N'\r\n"
 #string STR_GEN_OUT_MEM   #language en-US "%H%s%N: Memory allocation 
was not successful.\r\n"
 #string STR_BCFG_WRITE_FAIL   #language en-US "%H%s%N: Unable to write to 
'%H%s%N'\r\n"
 #string STR_BCFG_READ_FAIL#language en-US "%H%s%N: Unable to read from 
'%H%s%N'\r\n"
--
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 5/5] ArmVirtPkg: remove now unused PciHostBridgeDxe

[Laszlo Ersek]

On 08/22/16 08:35, Ard Biesheuvel wrote:
> This code is now no longer used, so remove it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 1458 -
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |  499 -
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |   64 -
>  ArmVirtPkg/PciHostBridgeDxe/PciRootBridgeIo.c| 2144 
>  4 files changed, 4165 deletions(-)

Reviewed-by: Laszlo Ersek 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 4/5] ArmVirtPkg/FdtPciHostBridgeLib: add MMIO64 support

[Laszlo Ersek]

On 08/22/16 08:35, Ard Biesheuvel wrote:
> If the pci-host-ecam-generic DT node describes a 64-bit MMIO region,
> account for it in the PCI_ROOT_BRIDGE description that we return to
> the generic PciHostBridgeDxe implementation, which will be able to
> allocate BARs from it without any further changes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 59 
> +---
>  1 file changed, 38 insertions(+), 21 deletions(-)

(1) So this patch will have to be rebased to the IoTranslation change
discussed under patch 2/5,

> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 0aff149e8029..74fda4d23fa3 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -87,9 +87,10 @@ ProcessPciHost (
>OUT  UINT64*IoBase,
>OUT  UINT64*IoSize,
>OUT  UINT64*IoTranslation,
> -  OUT  UINT64*MmioBase,
> -  OUT  UINT64*MmioSize,
> -  OUT  UINT64*MmioTranslation,
> +  OUT  UINT64*Mmio32Base,
> +  OUT  UINT64*Mmio32Size,
> +  OUT  UINT64*Mmio64Base,
> +  OUT  UINT64*Mmio64Size,
>OUT  UINT32*BusMin,
>OUT  UINT32*BusMax
>)
> @@ -101,6 +102,7 @@ ProcessPciHost (
>UINT32  Len;
>UINT32  RecordIdx;
>EFI_STATUS  Status;
> +  UINT64  MmioTranslation;
>  
>//
>// The following output arguments are initialized only in
> @@ -109,8 +111,8 @@ ProcessPciHost (
>//
>*IoBase = 0;
>*IoTranslation = 0;
> -  *MmioBase = 0;
> -  *MmioTranslation = 0;
> +  *Mmio32Base = 0;
> +  *Mmio64Base = MAX_UINT64;
>*BusMin = 0;
>*BusMax = 0;
>  
> @@ -120,7 +122,8 @@ ProcessPciHost (
>// above, they are initialized early.
>//
>*IoSize = 0;
> -  *MmioSize = 0;
> +  *Mmio32Size = 0;
> +  *Mmio64Size = 0;

(2) and here the leading comment should be updated -- at the moment it
refers to IoSize and MmioSize.

>  
>Status = gBS->LocateProtocol (, NULL,
>(VOID **));
> @@ -207,26 +210,39 @@ ProcessPciHost (
>break;
>  
>  case DTB_PCI_HOST_RANGE_MMIO32:
> -  *MmioBase = SwapBytes64 (Record->ChildBase);
> -  *MmioSize = SwapBytes64 (Record->Size);
> -  *MmioTranslation = SwapBytes64 (Record->CpuBase) - *MmioBase;
> +  *Mmio32Base = SwapBytes64 (Record->ChildBase);
> +  *Mmio32Size = SwapBytes64 (Record->Size);
> +  MmioTranslation = SwapBytes64 (Record->CpuBase) - *Mmio32Base;
>  
> -  if (*MmioBase > MAX_UINT32 || *MmioSize > MAX_UINT32 ||
> -  *MmioBase + *MmioSize > SIZE_4GB) {
> +  if (*Mmio32Base > MAX_UINT32 || *Mmio32Size > MAX_UINT32 ||
> +  *Mmio32Base + *Mmio32Size > SIZE_4GB) {
>  DEBUG ((EFI_D_ERROR, "%a: MMIO32 space invalid\n", __FUNCTION__));
>  return EFI_PROTOCOL_ERROR;
>}
>  
> -  if (*MmioTranslation != 0) {
> +  if (MmioTranslation != 0) {
>  DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO32 translation "
> -  "0x%Lx\n", __FUNCTION__, *MmioTranslation));
> +  "0x%Lx\n", __FUNCTION__, MmioTranslation));
> +return EFI_UNSUPPORTED;
> +  }
> +
> +  break;
> +
> +case DTB_PCI_HOST_RANGE_MMIO64:
> +  *Mmio64Base = SwapBytes64 (Record->ChildBase);
> +  *Mmio64Size = SwapBytes64 (Record->Size);
> +  MmioTranslation = SwapBytes64 (Record->CpuBase) - *Mmio64Base;
> +
> +  if (MmioTranslation != 0) {
> +DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO64 translation "
> +  "0x%Lx\n", __FUNCTION__, MmioTranslation));
>  return EFI_UNSUPPORTED;
>}
>  
>break;
>  }
>}
> -  if (*IoSize == 0 || *MmioSize == 0) {
> +  if (*IoSize == 0 || *Mmio32Size == 0) {
>  DEBUG ((EFI_D_ERROR, "%a: %a space empty\n", __FUNCTION__,
>(*IoSize == 0) ? "IO" : "MMIO32"));
>  return EFI_PROTOCOL_ERROR;
> @@ -239,9 +255,9 @@ ProcessPciHost (
>ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase);
>  
>DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] "
> -"Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__, 
> ConfigBase,
> -ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation, 
> *MmioBase,
> -*MmioSize, *MmioTranslation));
> +"Io[0x%Lx+0x%Lx)@0x%Lx Mem32[0x%Lx+0x%Lx) Mem64[0x%Lx+0x%Lx)\n",
> +__FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize,
> +*IoTranslation, *Mmio32Base, *Mmio32Size, *Mmio64Base, *Mmio64Size));
>return EFI_SUCCESS;
>  }
>  
> @@ -263,7 +279,8 @@ PciHostBridgeGetRootBridges (
>  {
>PCI_ROOT_BRIDGE *RootBridge;
>UINT64  IoBase, IoSize, IoTranslation;
> -  UINT64  

Re: [edk2] [PATCH 2/5] ArmVirtPkg: implement FdtPciHostBridgeLib

[Laszlo Ersek]

On 08/24/16 17:19, Ard Biesheuvel wrote:
> On 24 August 2016 at 17:01, Laszlo Ersek  wrote:
>> On 08/22/16 02:35, Ard Biesheuvel wrote:

>>> +  PcdSet64 (PcdPciIoTranslation, IoTranslation);
>>
>> I agree this is necessary, but it's not in the right place, plus as-is,
>> it is not sufficient.
>>
>> In  I wrote
>> -- and I'm slightly reformatting it here, because the formatting got
>> lost in the GitHub -> BZ migration --:
>>
>> The main customization in ArmVirtPkg/PciHostBridgeDxe is that it
>> emulates IO port accesses with an MMIO range, whose base address is
>> parsed from the DTB.
>>
>> The core PciHostBridgeDxe driver delegates the IO port access
>> implementation to EFI_CPU_IO2_PROTOCOL. @ardbiesheuvel recently
>> implemented ArmPkg/Drivers/ArmPciCpuIo2Dxe (in commit 3dece14)
>> which provides this protocol, backed by the same kind of
>> translation as described above. The base address comes from
>> gArmTokenSpaceGuid.PcdPciIoTranslation.
>>
>> Therefore:
>>
>> (1) We should extend our ArmVirtPkg/Library/FdtPciPcdProducerLib
>> plugin library to locate the IO translation offset in the DTB,
>> and store it in the above PCD.
>>
>> (2) We should include ArmPciCpuIo2Dxe in the ArmVirtQemu builds,
>> plugging the above library into it.
>>
>> (3) We should implement a PciHostBridgeLib instance, and plug it
>> into the core PciHostBridgeDxe driver.
>>
>> We should create one PCI_ROOT_BRIDGE object, populating it with
>> the FTD Client code we are currently using in
>> ArmVirtPkg/PciHostBridgeDxe.
>>
>> (4) Extra benefit: the core PciHostBridgeDxe driver supports 64-bit
>> MMIO, we just have to parse those values from the DTB as well.
>>
>> Steps (2) through (4) are implemented by this series, but I don't think
>> the above PcdSet64() call satisfies (1). What guarantees that by the
>> time ArmPciCpuIo2Dxe looks at it, PcdPciIoTranslation will be set?
>>
>> ... Especially because PciHostBridgeDxe.inf has a DEPEX on
>> gEfiCpuIo2ProtocolGuid, so the PcdSet64() above is bound to run *after*
>> ArmPciCpuIo2Dxe is dispatched?
>>
>> H... Are you making the argument that the PCD is set *between*
>> ArmPciCpuIo2Dxe's dispatch and actual use of the PCD? That is:
>>
>> - ArmPciCpuIo2Dxe is dispatched
>> - PciHostBridgeDxe is dispatched
>> - we set the PCD in this lib (as part of PciHostBridgeDxe's startup)
>> - (much later) something makes a PCI IO or MEM access that causes
>>   PciHostBridgeDxe to talk to ArmPciCpuIo2Dxe
>> - ArmPciCpuIo2Dxe consumes the PCD
>>
>> I think this is a valid argument to make -- I've even checked:
>> ArmPciCpuIo2Dxe indeed performs a PcdGet64() on every CpuIoServiceRead()
>> / CpuIoServiceWrite(), and none in its entry point --, but it has to be
>> explained in detail.
>>
>> This borders on a hack -- because ArmPciCpuIo2Dxe is not actually ready
>> for use when its entry point exits with success --, but I think we can
>> allow it, both ArmPciCpuIo2Dxe and this lib being platform specific, as
>> long as we document it profusely.
>>
>> So please describe this quirk in the commit message, and add a large
>> comment right above the PcdSet64() call.
>>
>> (Also, did you test this series with std VGA, on TCG? That will actually
>> put the IO translation to use.)
>>
>> The alternative to the commit msg addition / code comment would be my
>> suggestion in (1), that is, to extend FdtPciPcdProducerLib with setting
>> PcdPciIoTranslation, and to link it into ArmPciCpuIo2Dxe via NULL
>> library resolution. I guess you don't like that, which is fine, but then
>> please add the docs. Thanks.
>>
> 
> Actually, I prefer your original suggestion, to add it to
> FdtPciPcdProducerLib. That way, we no longer have to set any PCDs in
> this code, which is much cleaner imo

Sounds good, thanks!

In this case, the IoTranslation variable should become local to
ProcessPciHost() -- and ProcessPciHost() only -- in this patch, plus I
think we should also add

  ASSERT (PcdGet64 (PcdPciIoTranslation) == IoTranslation);

to ProcessPciHost() here, under the DTB_PCI_HOST_RANGE_IO branch.

Furthermore, the zero-assignment of the now-local IoTranslation variable
should probably be moved out from under the
'-Werror=maybe-uninitialized' comment, because it won't be an output
parameter any longer. What do you think?

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer()

[Leif Lindholm]

On Wed, Aug 31, 2016 at 10:07:30AM +0100, Ard Biesheuvel wrote:
> The BaseMemoryLib API has recently been extended with IsZeroGuid() and
> IsZeroBuffer(), so copy the generic implementations into the ArmPkg version
> of this library.

Maybe this is a good point at which to move these into MdePkg, in the
hope that the ARM versions won't be overlooked in future API
revisions?

/
Leif

> Ard Biesheuvel (3):
>   ArmPkg: remove BaseMemoryLibVstm implementation of BaseMemoryLib
>   ArmPkg/BaseMemoryLibStm: implement new IsZeroGuid() API function
>   ArmPkg/BaseMemoryLibStm: implement new IsZeroBuffer() API function
> 
>  ArmPkg/ArmPkg.dsc
>  |   2 -
>  ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf 
>  |   1 +
>  ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => 
> BaseMemoryLibStm/IsZeroBufferWrapper.c} |  28 ++-
>  ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c  
>  |  29 +++
>  ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c 
>  |  29 +++
>  ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h
>  |  17 ++
>  ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S   
>  | 112 -
>  ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm 
>  | 114 -
>  ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S
>  |  76 --
>  ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm  
>  |  78 --
>  ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf   
>  |  70 --
>  ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c 
>  |  66 -
>  ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c   
>  |  62 -
>  ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c
>  |  63 -
>  ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c 
>  | 264 
>  ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c
>  | 132 --
>  ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h   
>  | 234 -
>  ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c  
>  |  67 -
>  ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c  
>  |  66 -
>  ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c  
>  |  67 -
>  ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c   
>  |  99 
>  ArmPkg/Library/BaseMemoryLibVstm/SetMem.c
>  |  53 
>  ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c   
>  |  64 -
>  ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c   
>  |  64 -
>  ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c   
>  |  64 -
>  ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c 
>  |  91 ---
>  26 files changed, 91 insertions(+), 1921 deletions(-)
>  rename ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => 
> BaseMemoryLibStm/IsZeroBufferWrapper.c} (53%)
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem.c
>  delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c
>  delete mode 100644 

Re: [edk2] [PATCH] Platforms/ARM/Juno: limit ACPI support to v5.0 and higher

[Evan Lloyd]

>-Original Message-
>From: Sudeep Holla [mailto:sudeep.ho...@arm.com]
>Sent: 25 August 2016 18:09
>To: edk2-devel@lists.01.org; Linaro UEFI; Evan Lloyd; Leif Lindholm
>Cc: Sudeep Holla; ard.biesheu...@linaro.org
>Subject: [PATCH] Platforms/ARM/Juno: limit ACPI support to v5.0 and higher
>
>The ACPI spec predates the AARCH64 architecture by 5 versions, so there
>is no point in supporting anything below v5.0. So set the PCD that
>controls the ACPI table generation to the appropriate value.
>
>Based on the commit e0692789058e ("ArmVirtPkg/ArmVirtQemu: limit ACPI
>support to v5.0 and higher") in the upstream TianoCore by Ard Biesheuvel
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Cc: Ard Biesheuvel 
>Cc: Leif Lindholm 
>Cc: Evan Lloyd 
>Signed-off-by: Sudeep Holla 
>---
> Platforms/ARM/Juno/ArmJuno.dsc | 4 
> 1 file changed, 4 insertions(+)
>
>diff --git a/Platforms/ARM/Juno/ArmJuno.dsc
>b/Platforms/ARM/Juno/ArmJuno.dsc
>index c51d8f2e21ab..302bc66f 100644
>--- a/Platforms/ARM/Juno/ArmJuno.dsc
>+++ b/Platforms/ARM/Juno/ArmJuno.dsc
>@@ -183,6 +183,10 @@
>   #
>   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0300
>   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>+  #
>+  # ACPI Table Version
>+  #
>+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
>
> [PcdsPatchableInModule]
>   # Console Resolution (Full HD)
>--
>2.7.4
Reviewed-by: Evan Lloyd 

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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

On 31 August 2016 at 10:52, Michael Zimmermann  wrote:
>>Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and
>> "Tested-by" > mean tested?
> Yes you're right, sorry. I guess I just had too much hope in reviewers
> actually testing the changes :D
>

You mean those reviewers that actually own a BeagleBoard?

> On Wed, Aug 31, 2016 at 11:47 AM, Evan Lloyd  wrote:
>>
>> >-Original Message-
>> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> >Michael Zimmermann
>> >Sent: 31 August 2016 05:33
>> >To: Leif Lindholm
>> >Cc: edk2-devel@lists.01.org; Cohen, Eugene; Laszlo Ersek;
>> >ard.biesheu...@linaro.org
>> >Subject: Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to
>> >ASM_FUNC() asm macro
>> >
>> >reviewed should mean tested ;)
>> >
>> Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and
>> "Tested-by" mean tested?
>>
>> Evan
>> ...
>>
>> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Michael Zimmermann]

>Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and
"Tested-by" > mean tested?
Yes you're right, sorry. I guess I just had too much hope in reviewers
actually testing the changes :D

On Wed, Aug 31, 2016 at 11:47 AM, Evan Lloyd  wrote:

> >-Original Message-
> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >Michael Zimmermann
> >Sent: 31 August 2016 05:33
> >To: Leif Lindholm
> >Cc: edk2-devel@lists.01.org; Cohen, Eugene; Laszlo Ersek;
> >ard.biesheu...@linaro.org
> >Subject: Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to
> >ASM_FUNC() asm macro
> >
> >reviewed should mean tested ;)
> >
> Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and
> "Tested-by" mean tested?
>
> Evan
> ...
>
> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Evan Lloyd]

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Michael Zimmermann
>Sent: 31 August 2016 05:33
>To: Leif Lindholm
>Cc: edk2-devel@lists.01.org; Cohen, Eugene; Laszlo Ersek;
>ard.biesheu...@linaro.org
>Subject: Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to
>ASM_FUNC() asm macro
>
>reviewed should mean tested ;)
>
Sorry to be pedantic, but shouldn't "Reviewed-by" mean code reviewed, and 
"Tested-by" mean tested?

Evan
...

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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

On 31 August 2016 at 10:43, Michael Zimmermann  wrote:
> ArmVirtQemuKernel uses ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S which is not
> affected by this bug.
>

Ah, of course. It does highlight why having so many copies of
essentially the same code is a bad idea, but it is a difficult pattern
to get rid of in Tianocore.

> as it turns out, only BeagleBoardPkg uses the generic PrePi code.
>

Thanks,
Ard.


> On Wed, Aug 31, 2016 at 11:14 AM, Ard Biesheuvel 
> wrote:
>>
>> On 31 August 2016 at 05:33, Michael Zimmermann 
>> wrote:
>> > reviewed should mean tested ;)
>> >
>>
>> Apologies for the breakage. As it turns out, ArmVirtQemuKernel does
>> work even with this bug, probably due to the fact that its
>> PcdCoreCount == 1 (or simply that it uses the UniCore flavor rather
>> than the MPCore flavor of PrePi)
>>
>> In any case, thanks for tracking this down, and for proposing a fix.
>>
>> > took me some time to find out why my system currently doesn't boot
>> > anymore
>> > but here's the fix for this commit:
>> >
>> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
>> > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
>> > index b7127ce..39030da 100644
>> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
>> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
>> > @@ -101,7 +101,7 @@ _GetStackBase:
>> >sub   r10, r1, r2
>> >
>> >// Stack for the secondary core = Number of Cores - 1
>> > -  MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) *
>> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
>> > +  MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) *
>> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M
>> >sub   r10, r10, r1
>> >
>> >// r10 = The base of the MpCore Stacks (primary stack & secondary
>> > stacks)
>> >
>> > On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm
>> > 
>> > wrote:
>> >>
>> >> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote:
>> >> > Annotate functions with ASM_FUNC() so that they are emitted into
>> >> > separate sections.
>> >>
>> >> Also replacing LoadConstantToReg. Add that to commit message and:
>> >> Reviewed-by: Leif Lindholm 
>> >>
>> >> > Contributed-under: TianoCore Contribution Agreement 1.0
>> >> > Signed-off-by: Ard Biesheuvel 
>> >> > ---
>> >> >  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49
>> >> > ++-
>> >> >  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50
>> >> > ++--
>> >> >  2 files changed, 29 insertions(+), 70 deletions(-)
>> >> >
>> >> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> >> > b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> >> > index 9538c70a237c..d0530a874726 100644
>> >> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> >> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> >> > @@ -12,24 +12,10 @@
>> >> >  //
>> >> >
>> >> >  #include 
>> >> > -#include 
>> >> > -#include 
>> >> > -#include 
>> >> > -
>> >> > -.text
>> >> > -.align 3
>> >> > -
>> >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
>> >> > -GCC_ASM_IMPORT(ArmReadMpidr)
>> >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction)
>> >> > -GCC_ASM_IMPORT(ArmPlatformStackSet)
>> >> > -GCC_ASM_EXPORT(_ModuleEntryPoint)
>> >> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>> >> >
>> >> > -StartupAddr:  .8byte ASM_PFX(CEntryPoint)
>> >> > -ASM_PFX(mSystemMemoryEnd):.8byte 0
>> >> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>> >> >
>> >> > -ASM_PFX(_ModuleEntryPoint):
>> >> > +ASM_FUNC(_ModuleEntryPoint)
>> >> >// Do early platform specific actions
>> >> >blASM_PFX(ArmPlatformPeiBootAction)
>> >> >
>> >> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit:
>> >> >cmp   x1, #0
>> >> >bne   _SetupStackPosition
>> >> >
>> >> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1)
>> >> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2)
>> >> > -  sub   x2, x2, #1
>> >> > -  add   x1, x1, x2
>> >> > +  MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) +
>> >> > FixedPcdGet64(PcdSystemMemorySize) - 1)
>> >> > +
>> >> >// Update the global variable
>> >> >adr   x2, mSystemMemoryEnd
>> >> >str   x1, [x2]
>> >> > @@ -61,13 +45,13 @@ _SetupStackPosition:
>> >> >// r1 = SystemMemoryTop
>> >> >
>> >> >// Calculate Top of the Firmware Device
>> >> > -  LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2)
>> >> > -  LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3)
>> >> > +  MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress))
>> >> > +  MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1)
>> >> >sub   x3, x3, #1
>> >> >add   x3, x3, x2  // x3 = FdTop = PcdFdBaseAddress + PcdFdSize
>> >> >
>> >> >// UEFI Memory Size (stacks are allocated in this region)
>> >> > -  LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize),
>> >> > x4)
>> >> > +  MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize))

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Michael Zimmermann]

ArmVirtQemuKernel uses ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S which is not
affected by this bug.

as it turns out, only BeagleBoardPkg uses the generic PrePi code.

On Wed, Aug 31, 2016 at 11:14 AM, Ard Biesheuvel 
wrote:

> On 31 August 2016 at 05:33, Michael Zimmermann 
> wrote:
> > reviewed should mean tested ;)
> >
>
> Apologies for the breakage. As it turns out, ArmVirtQemuKernel does
> work even with this bug, probably due to the fact that its
> PcdCoreCount == 1 (or simply that it uses the UniCore flavor rather
> than the MPCore flavor of PrePi)
>
> In any case, thanks for tracking this down, and for proposing a fix.
>
> > took me some time to find out why my system currently doesn't boot
> anymore
> > but here's the fix for this commit:
> >
> > diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> > b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> > index b7127ce..39030da 100644
> > --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> > +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> > @@ -101,7 +101,7 @@ _GetStackBase:
> >sub   r10, r1, r2
> >
> >// Stack for the secondary core = Number of Cores - 1
> > -  MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) *
> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
> > +  MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) *
> > FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M
> >sub   r10, r10, r1
> >
> >// r10 = The base of the MpCore Stacks (primary stack & secondary
> stacks)
> >
> > On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm <
> leif.lindh...@linaro.org>
> > wrote:
> >>
> >> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote:
> >> > Annotate functions with ASM_FUNC() so that they are emitted into
> >> > separate sections.
> >>
> >> Also replacing LoadConstantToReg. Add that to commit message and:
> >> Reviewed-by: Leif Lindholm 
> >>
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Ard Biesheuvel 
> >> > ---
> >> >  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49
> >> > ++-
> >> >  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50
> >> > ++--
> >> >  2 files changed, 29 insertions(+), 70 deletions(-)
> >> >
> >> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> >> > b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> >> > index 9538c70a237c..d0530a874726 100644
> >> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> >> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> >> > @@ -12,24 +12,10 @@
> >> >  //
> >> >
> >> >  #include 
> >> > -#include 
> >> > -#include 
> >> > -#include 
> >> > -
> >> > -.text
> >> > -.align 3
> >> > -
> >> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
> >> > -GCC_ASM_IMPORT(ArmReadMpidr)
> >> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction)
> >> > -GCC_ASM_IMPORT(ArmPlatformStackSet)
> >> > -GCC_ASM_EXPORT(_ModuleEntryPoint)
> >> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
> >> >
> >> > -StartupAddr:  .8byte ASM_PFX(CEntryPoint)
> >> > -ASM_PFX(mSystemMemoryEnd):.8byte 0
> >> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
> >> >
> >> > -ASM_PFX(_ModuleEntryPoint):
> >> > +ASM_FUNC(_ModuleEntryPoint)
> >> >// Do early platform specific actions
> >> >blASM_PFX(ArmPlatformPeiBootAction)
> >> >
> >> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit:
> >> >cmp   x1, #0
> >> >bne   _SetupStackPosition
> >> >
> >> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1)
> >> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2)
> >> > -  sub   x2, x2, #1
> >> > -  add   x1, x1, x2
> >> > +  MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) +
> >> > FixedPcdGet64(PcdSystemMemorySize) - 1)
> >> > +
> >> >// Update the global variable
> >> >adr   x2, mSystemMemoryEnd
> >> >str   x1, [x2]
> >> > @@ -61,13 +45,13 @@ _SetupStackPosition:
> >> >// r1 = SystemMemoryTop
> >> >
> >> >// Calculate Top of the Firmware Device
> >> > -  LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2)
> >> > -  LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3)
> >> > +  MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress))
> >> > +  MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1)
> >> >sub   x3, x3, #1
> >> >add   x3, x3, x2  // x3 = FdTop = PcdFdBaseAddress + PcdFdSize
> >> >
> >> >// UEFI Memory Size (stacks are allocated in this region)
> >> > -  LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize),
> x4)
> >> > +  MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize))
> >> >
> >> >//
> >> >// Reserve the memory for the UEFI region (contain stacks on its
> top)
> >> > @@ -98,9 +82,7 @@ _SetupAlignedStack:
> >> >  _SetupOverflowStack:
> >> >// Case memory at the top of the address space. Ensure the top of
> the
> >> > stack is EFI_PAGE_SIZE
> >> >// aligned (4KB)
> >> > -  LoadConstantToReg (EFI_PAGE_MASK, x11)
> >> > -  and   x11, x11, x1
> >> 

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

On 31 August 2016 at 05:33, Michael Zimmermann  wrote:
> reviewed should mean tested ;)
>

Apologies for the breakage. As it turns out, ArmVirtQemuKernel does
work even with this bug, probably due to the fact that its
PcdCoreCount == 1 (or simply that it uses the UniCore flavor rather
than the MPCore flavor of PrePi)

In any case, thanks for tracking this down, and for proposing a fix.

> took me some time to find out why my system currently doesn't boot anymore
> but here's the fix for this commit:
>
> diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> index b7127ce..39030da 100644
> --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> @@ -101,7 +101,7 @@ _GetStackBase:
>sub   r10, r1, r2
>
>// Stack for the secondary core = Number of Cores - 1
> -  MOV32 (r0, (FixedPcdGet32(PcdCoreCount) - 1) *
> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
> +  MOV32 (r1, (FixedPcdGet32(PcdCoreCount) - 1) *
> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))^M
>sub   r10, r10, r1
>
>// r10 = The base of the MpCore Stacks (primary stack & secondary stacks)
>
> On Thu, Aug 11, 2016 at 10:38 AM, Leif Lindholm 
> wrote:
>>
>> On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote:
>> > Annotate functions with ASM_FUNC() so that they are emitted into
>> > separate sections.
>>
>> Also replacing LoadConstantToReg. Add that to commit message and:
>> Reviewed-by: Leif Lindholm 
>>
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Ard Biesheuvel 
>> > ---
>> >  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49
>> > ++-
>> >  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50
>> > ++--
>> >  2 files changed, 29 insertions(+), 70 deletions(-)
>> >
>> > diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> > b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> > index 9538c70a237c..d0530a874726 100644
>> > --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> > +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
>> > @@ -12,24 +12,10 @@
>> >  //
>> >
>> >  #include 
>> > -#include 
>> > -#include 
>> > -#include 
>> > -
>> > -.text
>> > -.align 3
>> > -
>> > -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
>> > -GCC_ASM_IMPORT(ArmReadMpidr)
>> > -GCC_ASM_IMPORT(ArmPlatformPeiBootAction)
>> > -GCC_ASM_IMPORT(ArmPlatformStackSet)
>> > -GCC_ASM_EXPORT(_ModuleEntryPoint)
>> > -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>> >
>> > -StartupAddr:  .8byte ASM_PFX(CEntryPoint)
>> > -ASM_PFX(mSystemMemoryEnd):.8byte 0
>> > +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>> >
>> > -ASM_PFX(_ModuleEntryPoint):
>> > +ASM_FUNC(_ModuleEntryPoint)
>> >// Do early platform specific actions
>> >blASM_PFX(ArmPlatformPeiBootAction)
>> >
>> > @@ -49,10 +35,8 @@ _SystemMemoryEndInit:
>> >cmp   x1, #0
>> >bne   _SetupStackPosition
>> >
>> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1)
>> > -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2)
>> > -  sub   x2, x2, #1
>> > -  add   x1, x1, x2
>> > +  MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) +
>> > FixedPcdGet64(PcdSystemMemorySize) - 1)
>> > +
>> >// Update the global variable
>> >adr   x2, mSystemMemoryEnd
>> >str   x1, [x2]
>> > @@ -61,13 +45,13 @@ _SetupStackPosition:
>> >// r1 = SystemMemoryTop
>> >
>> >// Calculate Top of the Firmware Device
>> > -  LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2)
>> > -  LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3)
>> > +  MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress))
>> > +  MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1)
>> >sub   x3, x3, #1
>> >add   x3, x3, x2  // x3 = FdTop = PcdFdBaseAddress + PcdFdSize
>> >
>> >// UEFI Memory Size (stacks are allocated in this region)
>> > -  LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4)
>> > +  MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize))
>> >
>> >//
>> >// Reserve the memory for the UEFI region (contain stacks on its top)
>> > @@ -98,9 +82,7 @@ _SetupAlignedStack:
>> >  _SetupOverflowStack:
>> >// Case memory at the top of the address space. Ensure the top of the
>> > stack is EFI_PAGE_SIZE
>> >// aligned (4KB)
>> > -  LoadConstantToReg (EFI_PAGE_MASK, x11)
>> > -  and   x11, x11, x1
>> > -  sub   x1, x1, x11
>> > +  and   x1, x1, ~EFI_PAGE_MASK
>> >
>> >  _GetBaseUefiMemory:
>> >// Calculate the Base of the UEFI Memory
>> > @@ -109,22 +91,19 @@ _GetBaseUefiMemory:
>> >  _GetStackBase:
>> >// r1 = The top of the Mpcore Stacks
>> >// Stack for the primary core = PrimaryCoreStack
>> > -  LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)
>> > +  MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize))
>> >sub   x12, x1, x2
>> >
>> >// Stack for the 

[edk2] [PATCH 0/3] ArmPkg: introduce IsZeroGuid() and IsZeroBuffer()

[Ard Biesheuvel]

The BaseMemoryLib API has recently been extended with IsZeroGuid() and
IsZeroBuffer(), so copy the generic implementations into the ArmPkg version
of this library.

Ard Biesheuvel (3):
  ArmPkg: remove BaseMemoryLibVstm implementation of BaseMemoryLib
  ArmPkg/BaseMemoryLibStm: implement new IsZeroGuid() API function
  ArmPkg/BaseMemoryLibStm: implement new IsZeroBuffer() API function

 ArmPkg/ArmPkg.dsc  
   |   2 -
 ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf   
   |   1 +
 ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => 
BaseMemoryLibStm/IsZeroBufferWrapper.c} |  28 ++-
 ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c
   |  29 +++
 ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c   
   |  29 +++
 ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h  
   |  17 ++
 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S 
   | 112 -
 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm   
   | 114 -
 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S  
   |  76 --
 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm
   |  78 --
 ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf 
   |  70 --
 ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c   
   |  66 -
 ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c 
   |  62 -
 ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c  
   |  63 -
 ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c   
   | 264 
 ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c  
   | 132 --
 ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h 
   | 234 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c
   |  67 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c
   |  66 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c
   |  67 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c 
   |  99 
 ArmPkg/Library/BaseMemoryLibVstm/SetMem.c  
   |  53 
 ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c 
   |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c 
   |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c 
   |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c   
   |  91 ---
 26 files changed, 91 insertions(+), 1921 deletions(-)
 rename ArmPkg/Library/{BaseMemoryLibVstm/ZeroMemWrapper.c => 
BaseMemoryLibStm/IsZeroBufferWrapper.c} (53%)
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c
 delete mode 100644 ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c

-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/3] ArmPkg: remove BaseMemoryLibVstm implementation of BaseMemoryLib

[Ard Biesheuvel]

The BaseMemoryLibVstm implementation of BaseMemoryLib is ARM only, uses
the NEON register file despite the fact that the UEFI spec does not allow
it, and is currently not used anywhere. So remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/ArmPkg.dsc  |   2 -
 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S | 112 -
 ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.asm   | 114 -
 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.S  |  76 --
 ArmPkg/Library/BaseMemoryLibVstm/Arm/SetMem.asm|  78 --
 ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf |  70 --
 ArmPkg/Library/BaseMemoryLibVstm/CompareMemWrapper.c   |  66 -
 ArmPkg/Library/BaseMemoryLibVstm/CopyMem.c |  62 -
 ArmPkg/Library/BaseMemoryLibVstm/CopyMemWrapper.c  |  63 -
 ArmPkg/Library/BaseMemoryLibVstm/MemLibGeneric.c   | 264 

 ArmPkg/Library/BaseMemoryLibVstm/MemLibGuid.c  | 132 --
 ArmPkg/Library/BaseMemoryLibVstm/MemLibInternals.h | 234 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem16Wrapper.c|  67 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem32Wrapper.c|  66 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem64Wrapper.c|  67 -
 ArmPkg/Library/BaseMemoryLibVstm/ScanMem8Wrapper.c |  99 
 ArmPkg/Library/BaseMemoryLibVstm/SetMem.c  |  53 
 ArmPkg/Library/BaseMemoryLibVstm/SetMem16Wrapper.c |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMem32Wrapper.c |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMem64Wrapper.c |  64 -
 ArmPkg/Library/BaseMemoryLibVstm/SetMemWrapper.c   |  91 ---
 ArmPkg/Library/BaseMemoryLibVstm/ZeroMemWrapper.c  |  52 
 22 files changed, 1960 deletions(-)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 7b278cdd4124..6f9fc661fbdc 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -150,8 +150,6 @@ [Components.common]
   ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
 
 [Components.ARM]
-  ArmPkg/Library/BaseMemoryLibVstm/BaseMemoryLibVstm.inf
-
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA8Lib/ArmCortexA8Lib.inf
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Lib.inf
   ArmPkg/Drivers/ArmCpuLib/ArmCortexA15Lib/ArmCortexA15Lib.inf
diff --git a/ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S 
b/ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S
deleted file mode 100644
index 69de4c1fd48e..
--- a/ArmPkg/Library/BaseMemoryLibVstm/Arm/CopyMem.S
+++ /dev/null
@@ -1,112 +0,0 @@
-#--
-#
-# CopyMem() worker for ARM
-#
-# This file started out as C code that did 64 bit moves if the buffer was
-# 32-bit aligned, else it does a byte copy. It also does a byte copy for
-# any trailing bytes. Update using VSTM/SLDM to do 128 byte copies.
-#
-# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD 
License
-# which accompanies this distribution.  The full text of the license may be 
found at
-# http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-#--
-
-#include 
-
-/**
-  Copy Length bytes from Source to Destination. Overlap is OK.
-
-  This implementation
-
-  @param  Destination Target of copy
-  @param  Source  Place to copy from
-  @param  Length  Number of bytes to copy
-
-  @return Destination
-
-
-VOID *
-EFIAPI
-InternalMemCopyMem (
-  OUT VOID  *DestinationBuffer,
-  IN  CONST VOID*SourceBuffer,
-  IN  UINTN Length
-  )
-**/
-ASM_FUNC(InternalMemCopyMem)
-  stmfd  sp!, {r4, r9, lr}
-  tst  r0, #3
-  mov  r4, r0
-  mov  r9, r0
-  mov  ip, r2
-  mov  lr, r1
-  movne  r0, #0
-  bne  L4
-  tst  r1, #3
-  movne  r3, #0
-  moveq  r3, #1
-  cmp  r2, #127
-  movls  r0, #0
-  andhi  r0, r3, #1
-L4:
-  cmp  r4, r1
-  bcc  L26
-  bls  L7
-  rsb  r3, r1, r4
-  cmp  ip, r3
-  bcc  L26
-  cmp  ip, #0
-  beq  L7
-  add  r9, r4, ip
-  add  lr, ip, r1
-  b  L16
-L29:
-  sub  ip, ip, #8
-  cmp  ip, #7
-  ldrd  r2, [lr, #-8]!
-  movls  r0, #0
-  cmp  ip, #0
-  strd  r2, [r9, #-8]!
-  beq  L7
-L16:
-  cmp  r0, #0
-  bne  L29
-  sub  r3, lr, #1
-  sub  ip, ip, #1
-  ldrb  r3, [r3, #0]
-  sub  r2, r9, #1
-  cmp  ip, #0
-  sub  r9, r9, #1
-  sub  lr, lr, #1
-  strb  r3, [r2, #0]
-  bne  L16
-  b   L7
-L11:
-  ldrb  r3, [lr], #1
-  sub  ip, ip, #1
-  strb  r3, [r9], #1
-L26:
-  cmp  ip, #0
-  beq  L7
-L30:
-  cmp  r0, #0
-  beq  L11
-  sub  ip, ip, #128  // 32
-  cmp  ip, #127  // 31
-  

[edk2] [PATCH 3/3] ArmPkg/BaseMemoryLibStm: implement new IsZeroBuffer() API function

[Ard Biesheuvel]

BaseMemoryLib has recently been extended with an API function
IsZeroBuffer(), so copy the default implementation into BaseMemoryLibStm
as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf  |  1 +
 ArmPkg/Library/BaseMemoryLibStm/IsZeroBufferWrapper.c | 54 
 ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c   | 29 +++
 ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h | 17 ++
 4 files changed, 101 insertions(+)

diff --git a/ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf 
b/ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
index eac6cef96b31..30e2459bfd1e 100644
--- a/ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
+++ b/ArmPkg/Library/BaseMemoryLibStm/BaseMemoryLibStm.inf
@@ -45,6 +45,7 @@ [Sources.Common]
   SetMem16Wrapper.c
   SetMemWrapper.c
   CopyMemWrapper.c
+  IsZeroBufferWrapper.c
   MemLibGeneric.c
   MemLibGuid.c
   MemLibInternals.h
diff --git a/ArmPkg/Library/BaseMemoryLibStm/IsZeroBufferWrapper.c 
b/ArmPkg/Library/BaseMemoryLibStm/IsZeroBufferWrapper.c
new file mode 100644
index ..c42c1aa509b3
--- /dev/null
+++ b/ArmPkg/Library/BaseMemoryLibStm/IsZeroBufferWrapper.c
@@ -0,0 +1,54 @@
+/** @file
+  Implementation of IsZeroBuffer function.
+
+  The following BaseMemoryLib instances contain the same copy of this file:
+
+BaseMemoryLib
+BaseMemoryLibMmx
+BaseMemoryLibSse2
+BaseMemoryLibRepStr
+BaseMemoryLibOptDxe
+BaseMemoryLibOptPei
+PeiMemoryLib
+UefiMemoryLib
+
+  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "MemLibInternals.h"
+
+/**
+  Checks if the contents of a buffer are all zeros.
+
+  This function checks whether the contents of a buffer are all zeros. If the
+  contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+  If Length > 0 and Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  Buffer  The pointer to the buffer to be checked.
+  @param  Length  The size of the buffer (in bytes) to be checked.
+
+  @retval TRUEContents of the buffer are all zeros.
+  @retval FALSE   Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+  IN CONST VOID  *Buffer,
+  IN UINTN   Length
+  )
+{
+  ASSERT (!(Buffer == NULL && Length > 0));
+  ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+  return InternalMemIsZeroBuffer (Buffer, Length);
+}
diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c 
b/ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c
index 54c27012955f..43fbcb6b208f 100644
--- a/ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c
+++ b/ArmPkg/Library/BaseMemoryLibStm/MemLibGeneric.c
@@ -262,3 +262,32 @@ InternalMemScanMem64 (
   } while (--Length != 0);
   return NULL;
 }
+
+/**
+  Checks whether the contents of a buffer are all zeros.
+
+  @param  Buffer  The pointer to the buffer to be checked.
+  @param  Length  The size of the buffer (in bytes) to be checked.
+
+  @retval TRUEContents of the buffer are all zeros.
+  @retval FALSE   Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+InternalMemIsZeroBuffer (
+  IN CONST VOID  *Buffer,
+  IN UINTN   Length
+  )
+{
+  CONST UINT8 *BufferData;
+  UINTN   Index;
+
+  BufferData = Buffer;
+  for (Index = 0; Index < Length; Index++) {
+if (BufferData[Index] != 0) {
+  return FALSE;
+}
+  }
+  return TRUE;
+}
diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h 
b/ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h
index 10c741f2c311..f7b44f527059 100644
--- a/ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h
+++ b/ArmPkg/Library/BaseMemoryLibStm/MemLibInternals.h
@@ -231,4 +231,21 @@ InternalMemScanMem64 (
   IN  UINT64Value
   );
 
+/**
+  Checks whether the contents of a buffer are all zeros.
+
+  @param  Buffer  The pointer to the buffer to be checked.
+  @param  Length  The size of the buffer (in bytes) to be checked.
+
+  @retval TRUEContents of the buffer are all zeros.
+  @retval FALSE   Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+InternalMemIsZeroBuffer (
+  IN CONST VOID  *Buffer,
+  IN UINTN   Length
+  );
+
 #endif
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/3] ArmPkg/BaseMemoryLibStm: implement new IsZeroGuid() API function

[Ard Biesheuvel]

BaseMemoryLib has recently been extended with an API function
IsZeroGuid(), so copy the default implementation into BaseMemoryLibStm
as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c | 29 
 1 file changed, 29 insertions(+)

diff --git a/ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c 
b/ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c
index 2b4ed5775581..36d42d71d79a 100644
--- a/ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c
+++ b/ArmPkg/Library/BaseMemoryLibStm/MemLibGuid.c
@@ -130,3 +130,32 @@ ScanGuid (
   }
   return NULL;
 }
+
+/**
+  Checks if the given GUID is a zero GUID.
+
+  This function checks whether the given GUID is a zero GUID. If the GUID is
+  identical to a zero GUID then TRUE is returned. Otherwise, FALSE is returned.
+
+  If Guid is NULL, then ASSERT().
+
+  @param  GuidThe pointer to a 128 bit GUID.
+
+  @retval TRUEGuid is a zero GUID.
+  @retval FALSE   Guid is not a zero GUID.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroGuid (
+  IN CONST GUID  *Guid
+  )
+{
+  UINT64  LowPartOfGuid;
+  UINT64  HighPartOfGuid;
+
+  LowPartOfGuid  = ReadUnaligned64 ((CONST UINT64*) Guid);
+  HighPartOfGuid = ReadUnaligned64 ((CONST UINT64*) Guid + 1);
+
+  return (BOOLEAN) (LowPartOfGuid == 0 && HighPartOfGuid == 0);
+}
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v5 3/4] MdeModulePkg/EbcDxe AARCH64: use tail call for EBC to native thunk

[Ard Biesheuvel]

Instead of pessimistically copying at least 64 bytes from the VM stack
to the native stack, and popping off the register arguments again
before doing the native call, try to avoid touching the stack completely
if the VM stack frame is <= 64 bytes. Also, if the stack frame does exceed
64 bytes, there is no need to copy the first 64 bytes, since we are passing
those in registers anyway.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 85 +++-
 1 file changed, 65 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S 
b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
index b4b8531f1a01..34794c06a644 100644
--- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
+++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
@@ -35,30 +35,75 @@ ASM_GLOBAL ASM_PFX(mEbcInstructionBufferTemplate)
 //
 // UINTN EbcLLCALLEXNative(UINTN FuncAddr, UINTN NewStackPointer, VOID 
*FramePtr)
 ASM_PFX(EbcLLCALLEXNative):
-  stp  x19, x20, [sp, #-16]!
-  stp  x29, x30, [sp, #-16]!
+mov x8, x0 // Preserve x0
+mov x9, x1 // Preserve x1
 
-  mov  x19, x0
-  mov  x20, sp
-  sub  x2, x2, x1   // Length = NewStackPointer-FramePtr
-  sub  sp, sp, x2
-  sub  sp, sp, #64  // Make sure there is room for at least 8 args in the 
new stack
-  mov  x0, sp
-
-  bl   CopyMem  // Sp, NewStackPointer, Length
-
-  ldp  x0, x1, [sp], #16
-  ldp  x2, x3, [sp], #16
-  ldp  x4, x5, [sp], #16
-  ldp  x6, x7, [sp], #16
+//
+// If the EBC stack frame is smaller than or equal to 64 bytes, we know 
there
+// are no stacked arguments #9 and beyond that we need to copy to the 
native
+// stack. In this case, we can perform a tail call which is much more
+// efficient, since there is no need to touch the native stack at all.
+//
+sub x3, x2, x1  // Length = NewStackPointer - FramePtr
+cmp x3, #64
+b.gt1f
 
-  blr  x19
+//
+// While probably harmless in practice, we should not access the VM stack
+// outside of the interval [NewStackPointer, FramePtr), which means we
+// should not blindly fill all 8 argument registers with VM stack data.
+// So instead, calculate how many argument registers we can fill based on
+// the size of the VM stack frame, and skip the remaining ones.
+//
+adr x0, 0f  // Take address of 'br' instruction below
+bic x3, x3, #7  // Ensure correct alignment
+sub x0, x0, x3, lsr #1  // Subtract 4 bytes for each arg to unstack
+br  x0  // Skip remaining argument registers
+
+ldr x7, [x9, #56]   // Call with 8 arguments
+ldr x6, [x9, #48]   //  |
+ldr x5, [x9, #40]   //  |
+ldr x4, [x9, #32]   //  |
+ldr x3, [x9, #24]   //  |
+ldr x2, [x9, #16]   //  |
+ldr x1, [x9, #8]//  V
+ldr x0, [x9]// Call with 1 argument
+
+0:  br  x8  // Call with no arguments
 
-  mov  sp,  x20
-  ldp  x29, x30, [sp], #16
-  ldp  x19, x20, [sp], #16
+//
+// More than 64 bytes: we need to build the full native stack frame and 
copy
+// the part of the VM stack exceeding 64 bytes (which may contain stacked
+// arguments) to the native stack
+//
+1:  stp x29, x30, [sp, #-16]!
+mov x29, sp
 
-  ret
+//
+// Ensure that the stack pointer remains 16 byte aligned,
+// even if the size of the VM stack frame is not a multiple of 16
+//
+add x1, x1, #64 // Skip over [potential] reg params
+tbz x3, #3, 2f  // Multiple of 16?
+ldr x4, [x2, #-8]!  // No? Then push one word
+str x4, [sp, #-16]! // ... but use two slots
+b   3f
+
+2:  ldp x4, x5, [x2, #-16]!
+stp x4, x5, [sp, #-16]!
+3:  cmp x2, x1
+b.gt2b
+
+ldp x0, x1, [x9]
+ldp x2, x3, [x9, #16]
+ldp x4, x5, [x9, #32]
+ldp x6, x7, [x9, #48]
+
+blr x8
+
+mov sp, x29
+ldp x29, x30, [sp], #16
+ret
 
 //
 // EbcLLEbcInterpret
-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v5 4/4] MdeModulePkg/EbcDxe AARCH64: simplify interpreter entry point thunks

[Ard Biesheuvel]

The prototypes of EbcInterpret() and ExecuteEbcImageEntryPoint() are
private to the AARCH64 implementation of EbcDxe, so we can shuffle
the arguments around a bit and make the assembler thunking glue a lot
simpler.

For ExecuteEbcImageEntryPoint(), this involves passing the EntryPoint
argument as the third parameter, rather than the first, which allows
us to do a tail call. For EbcInterpret(), instead of copying each
argument beyond #8 from one native stack frame to the next (before
another copy is made into the VM stack), pass a pointer to the
argument stack.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 59 +--
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c  | 60 
 2 files changed, 36 insertions(+), 83 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S 
b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
index 34794c06a644..b1f09725ecf0 100644
--- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
+++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
@@ -110,50 +110,23 @@ ASM_PFX(EbcLLCALLEXNative):
 //
 // This function is called by the thunk code to handle an Native to EBC call
 // This can handle up to 16 arguments (1-8 on in x0-x7, 9-16 are on the stack)
-// x16 contains the Entry point that will be the first argument when
+// x16 contains the Entry point that will be the first stacked argument when
 // EBCInterpret is called.
 //
 //
 ASM_PFX(EbcLLEbcInterpret):
-stp  x29, x30, [sp, #-16]!
-
-// copy the current arguments 9-16 from old location and add arg 7 to stack
-// keeping 16 byte stack alignment
-sub sp, sp, #80
-str x7, [sp]
-ldr x11, [sp, #96]
-str x11, [sp, #8]
-ldr x11, [sp, #104]
-str x11, [sp, #16]
-ldr x11, [sp, #112]
-str x11, [sp, #24]
-ldr x11, [sp, #120]
-str x11, [sp, #32]
-ldr x11, [sp, #128]
-str x11, [sp, #40]
-ldr x11, [sp, #136]
-str x11, [sp, #48]
-ldr x11, [sp, #144]
-str x11, [sp, #56]
-ldr x11, [sp, #152]
-str x11, [sp, #64]
-
-// Shift arguments and add entry point and as argument 1
-mov x7, x6
-mov x6, x5
-mov x5, x4
-mov x4, x3
-mov x3, x2
-mov x2, x1
-mov x1, x0
-mov x0, x16
+stp x29, x30, [sp, #-16]!
+mov x29, sp
 
-// call C-code
-bl ASM_PFX(EbcInterpret)
-add sp, sp, #80
+// push the entry point and the address of args #9 - #16 onto the stack
+add x17, sp, #16
+stp x16, x17, [sp, #-16]!
 
-ldp  x29, x30, [sp], #16
+// call C-code
+bl  ASM_PFX(EbcInterpret)
 
+add sp, sp, #16
+ldp x29, x30, [sp], #16
 ret
 
 //
@@ -165,16 +138,10 @@ ASM_PFX(EbcLLEbcInterpret):
 //
 //
 ASM_PFX(EbcLLExecuteEbcImageEntryPoint):
-stp  x29, x30, [sp, #-16]!
-// build new parameter calling convention
-mov  x2, x1
-mov  x1, x0
-mov  x0, x16
+mov x2, x16
 
-// call C-code
-bl ASM_PFX(ExecuteEbcImageEntryPoint)
-ldp  x29, x30, [sp], #16
-ret
+// tail call to C code
+b   ASM_PFX(ExecuteEbcImageEntryPoint)
 
 //
 // mEbcInstructionBufferTemplate
diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c 
b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c
index a5f21f400274..c5cc76d7bdcb 100644
--- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c
+++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c
@@ -89,7 +89,6 @@ PushU64 (
 
   This is a thunk function.
 
-  @param  EntryPointThe entrypoint of EBC code.
   @param  Arg1  The 1st argument.
   @param  Arg2  The 2nd argument.
   @param  Arg3  The 3rd argument.
@@ -98,14 +97,8 @@ PushU64 (
   @param  Arg6  The 6th argument.
   @param  Arg7  The 7th argument.
   @param  Arg8  The 8th argument.
-  @param  Arg9  The 9th argument.
-  @param  Arg10 The 10th argument.
-  @param  Arg11 The 11th argument.
-  @param  Arg12 The 12th argument.
-  @param  Arg13 The 13th argument.
-  @param  Arg14 The 14th argument.
-  @param  Arg15 The 15th argument.
-  @param  Arg16 The 16th argument.
+  @param  EntryPointThe entrypoint of EBC code.
+  @param  Args9_16[]Array containing arguments #9 to #16.
 
   @return The value returned by the EBC application we're going to run.
 
@@ -113,23 

[edk2] [PATCH v5 1/4] MdeModulePkg/EbcDxe AARCH64: clean up comment style in ASM file

[Ard Biesheuvel]

Change to consistent // style comments. Also, remove bogus global
definitions for external functions, and move the real exports to
the top of the file.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
---
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 111 +---
 1 file changed, 52 insertions(+), 59 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S 
b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
index e858227586a8..17f379248a62 100644
--- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
+++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S
@@ -1,40 +1,35 @@
-#/** @file
-#
-#This code provides low level routines that support the Virtual Machine
-#   for option ROMs.
-#
-#  Copyright (c) 2015, The Linux Foundation. All rights reserved.
-#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
-#  This program and the accompanying materials
-#  are licensed and made available under the terms and conditions of the BSD 
License
-#  which accompanies this distribution.  The full text of the license may be 
found at
-#  http://opensource.org/licenses/bsd-license.php
-#
-#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-#
-#**/
-
-#---
-# Equate files needed.
-#---
-
-ASM_GLOBAL ASM_PFX(CopyMem);
-ASM_GLOBAL ASM_PFX(EbcInterpret);
-ASM_GLOBAL ASM_PFX(ExecuteEbcImageEntryPoint);
-
-#
-# EbcLLCALLEX
-#
-# This function is called to execute an EBC CALLEX instruction.
-# This instruction requires that we thunk out to external native
-# code. For AArch64, we copy the VM stack into the main stack and then pop
-# the first 8 arguments off according to the AArch64 Procedure Call Standard
-# On return, we restore the stack pointer to its original location.
-#
-#
-# UINTN EbcLLCALLEXNative(UINTN FuncAddr, UINTN NewStackPointer, VOID 
*FramePtr)
-ASM_GLOBAL ASM_PFX(EbcLLCALLEXNative);
+///** @file
+//
+//  This code provides low level routines that support the Virtual Machine
+//  for option ROMs.
+//
+//  Copyright (c) 2015, The Linux Foundation. All rights reserved.
+//  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+//  This program and the accompanying materials
+//  are licensed and made available under the terms and conditions of the BSD 
License
+//  which accompanies this distribution.  The full text of the license may be 
found at
+//  http://opensource.org/licenses/bsd-license.php
+//
+//  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+//
+//**/
+
+ASM_GLOBAL ASM_PFX(EbcLLCALLEXNative)
+ASM_GLOBAL ASM_PFX(EbcLLEbcInterpret)
+ASM_GLOBAL ASM_PFX(EbcLLExecuteEbcImageEntryPoint)
+
+//
+// EbcLLCALLEX
+//
+// This function is called to execute an EBC CALLEX instruction.
+// This instruction requires that we thunk out to external native
+// code. For AArch64, we copy the VM stack into the main stack and then pop
+// the first 8 arguments off according to the AArch64 Procedure Call Standard
+// On return, we restore the stack pointer to its original location.
+//
+//
+// UINTN EbcLLCALLEXNative(UINTN FuncAddr, UINTN NewStackPointer, VOID 
*FramePtr)
 ASM_PFX(EbcLLCALLEXNative):
   stp  x19, x20, [sp, #-16]!
   stp  x29, x30, [sp, #-16]!
@@ -61,16 +56,15 @@ ASM_PFX(EbcLLCALLEXNative):
 
   ret
 
-#
-# EbcLLEbcInterpret
-#
-# This function is called by the thunk code to handle an Native to EBC call
-# This can handle up to 16 arguments (1-8 on in x0-x7, 9-16 are on the stack)
-# x9 contains the Entry point that will be the first argument when
-# EBCInterpret is called.
-#
-#
-ASM_GLOBAL ASM_PFX(EbcLLEbcInterpret);
+//
+// EbcLLEbcInterpret
+//
+// This function is called by the thunk code to handle an Native to EBC call
+// This can handle up to 16 arguments (1-8 on in x0-x7, 9-16 are on the stack)
+// x9 contains the Entry point that will be the first argument when
+// EBCInterpret is called.
+//
+//
 ASM_PFX(EbcLLEbcInterpret):
 stp  

[edk2] [PATCH v5 0/4] deModulePkg/EbcDxe: AARCH64 improvements

[Ard Biesheuvel]

This is v5 of my proposed changes to the AARCH64 implementation of EbcDxe
contributed by Jeff Brasen, which has recently been merged into Tianocore.

Changes since v5:
- added Leif's ack (#3)
- cc MdeModulePkg maintainers

Changes since v3:
- fix typo in comment (#1)
- clarify comments around computed goto in EBC to native thunk, and make sure
  the jump target is 32-bit aligned (#3)
- fix comment and constify Args9_16[] in EbcInterpret() prototype (#4)
- add Leif's R-b (#1, #2, #4)

Feng, Star: I know you will not be able to review or test this code in detail,
but please let me know if you are happy for me to merge it. Thanks.

Ard Biesheuvel (4):
  MdeModulePkg/EbcDxe AARCH64: clean up comment style in ASM file
  MdeModulePkg/EbcDxe AARCH64: use a fixed size thunk structure
  MdeModulePkg/EbcDxe AARCH64: use tail call for EBC to native thunk
  MdeModulePkg/EbcDxe AARCH64: simplify interpreter entry point thunks

 MdeModulePkg/Universal/EbcDxe/AArch64/EbcLowLevel.S | 285 +++-
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c  | 193 -
 2 files changed, 210 insertions(+), 268 deletions(-)

-- 
2.7.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg UefiBootManagerLib: Rename BootMenuApp to BootManagerMenuApp

[Liming Gao]

Rename local function name BootMenuApp to BootManagerMenuApp to align
to other public function name.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index f8a3988..88bb13b 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1530,15 +1530,15 @@ EfiBootManagerGetLoadOptionBuffer (
 }
 
 /**
-  Check if it's a Device Path pointing to BootMenuApp.
+  Check if it's a Device Path pointing to BootManagerMenuApp.
 
   @param  DevicePath Input device path.
 
-  @retval TRUE   The device path is BootMenuApp File Device Path.
-  @retval FALSE  The device path is NOT BootMenuApp File Device Path.
+  @retval TRUE   The device path is BootManagerMenuApp File Device Path.
+  @retval FALSE  The device path is NOT BootManagerMenuApp File Device Path.
 **/
 BOOLEAN
-BmIsBootMenuAppFilePath (
+BmIsBootManagerMenuAppFilePath (
   EFI_DEVICE_PATH_PROTOCOL *DevicePath
 )
 {
@@ -1645,7 +1645,7 @@ EfiBootManagerBoot (
   // 3. Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load 
and execute
   //the boot option.
   //
-  if (BmIsBootMenuAppFilePath (BootOption->FilePath)) {
+  if (BmIsBootManagerMenuAppFilePath (BootOption->FilePath)) {
 DEBUG ((EFI_D_INFO, "[Bds] Booting Boot Manager Menu.\n"));
 BmStopHotkeyService (NULL, NULL);
   } else {
@@ -2070,9 +2070,9 @@ BmEnumerateBootOptions (
  );
   for (Index = 0; Index < HandleCount; Index++) {
 //
-// Ignore BootMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
+// Ignore BootManagerMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
 //
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+if (BmIsBootManagerMenuAppFilePath (DevicePathFromHandle 
(Handles[Index]))) {
   continue;
 }
 
@@ -2212,7 +2212,7 @@ BmRegisterBootManagerMenu (
   DevicePath = NULL;
   Description = NULL;
   //
-  // Try to find BootMenuApp from LoadFile protocol
+  // Try to find BootManagerMenuApp from LoadFile protocol
   //
   gBS->LocateHandleBuffer (
  ByProtocol,
@@ -,7 +,7 @@ BmRegisterBootManagerMenu (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+if (BmIsBootManagerMenuAppFilePath (DevicePathFromHandle 
(Handles[Index]))) {
   DevicePath  = DuplicateDevicePath (DevicePathFromHandle 
(Handles[Index]));
   Description = BmGetBootDescription (Handles[Index]);
   break;
@@ -2331,7 +2331,7 @@ EfiBootManagerGetBootManagerMenu (
   BootOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeBoot);
 
   for (Index = 0; Index < BootOptionCount; Index++) {
-if (BmIsBootMenuAppFilePath (BootOptions[Index].FilePath)) {
+if (BmIsBootManagerMenuAppFilePath (BootOptions[Index].FilePath)) {
 Status = EfiBootManagerInitializeLoadOption (
BootOption,
BootOptions[Index].OptionNumber,
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg UefiBootManagerLib: Ignore BootManagerMenuApp from LoadFile

[Liming Gao]

BootManagerMenuApp boot option is handled by EfiBootManagerGetBootManagerMenu.
Don't need to handle it again when parse LoadFile protocol.

Cc: Ruiyu Ni 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ecd0ae3..f8a3988 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1940,7 +1940,6 @@ BmEnumerateBootOptions (
   UINTN Removable;
   UINTN Index;
   CHAR16*Description;
-  UINT32BootAttributes;
 
   ASSERT (BootOptionCount != NULL);
 
@@ -2070,6 +2069,12 @@ BmEnumerateBootOptions (
  
  );
   for (Index = 0; Index < HandleCount; Index++) {
+//
+// Ignore BootMenuApp. its boot option will be created by 
BmRegisterBootManagerMenu().
+//
+if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+  continue;
+}
 
 Description = BmGetBootDescription (Handles[Index]);
 BootOptions = ReallocatePool (
@@ -2079,19 +2084,11 @@ BmEnumerateBootOptions (
 );
 ASSERT (BootOptions != NULL);
 
-//
-// If LoadFile includes BootMenuApp, its boot attribue will be set to APP 
and HIDDEN.
-//
-BootAttributes = LOAD_OPTION_ACTIVE;
-if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
-  BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
-}
-
 Status = EfiBootManagerInitializeLoadOption (
[(*BootOptionCount)++],
LoadOptionNumberUnassigned,
LoadOptionTypeBoot,
-   BootAttributes,
+   LOAD_OPTION_ACTIVE,
Description,
DevicePathFromHandle (Handles[Index]),
NULL,
-- 
2.8.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg VarCheck: #### in L"Boot####" are upper case hex

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, August 26, 2016 5:41 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg VarCheck:  in L"Boot" are
> upper case hex
> 
> UEFI spec:
> Each load option entry resides in a Boot, Driver, SysPrep,
> OsRecovery or PlatformRecovery variable where  is replaced
> by a unique option number in printable hexadecimal representation using
> the digits 0-9, and the upper case versions of the characters A-F (-).
> 
> The patch also makes L"HwErrRec" follow this rule.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Library/VarCheckLib/VarCheckLib.c   | 16 -
> ---
>  .../Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c   | 14 +++---
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c
> b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c
> index 41b209da169f..5ca0d3edca44 100644
> --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c
> +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>Implementation functions and structures for var check services.
> 
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  This program and the accompanying materials  are licensed and made
> available under the terms and conditions of the BSD License  which
> accompanies this distribution.  The full text of the license may be found at
> @@ -96,17 +96,17 @@ VARIABLE_ENTRY_PROPERTY
> mVarCheckVariableWithWildcardName[] = {  };
> 
>  /**
> -  Check if a Unicode character is a hexadecimal character.
> +  Check if a Unicode character is an upper case hexadecimal character.
> 
> -  This function checks if a Unicode character is a
> -  hexadecimal character.  The valid hexadecimal character is
> -  L'0' to L'9', L'a' to L'f', or L'A' to L'F'.
> +  This function checks if a Unicode character is an upper case
> + hexadecimal character.  The valid upper case hexadecimal character is
> + L'0' to L'9', or L'A' to L'F'.
> 
> 
>@param[in] Char   The character to check against.
> 
> -  @retval TRUE  If the Char is a hexadecmial character.
> -  @retval FALSE If the Char is not a hexadecmial character.
> +  @retval TRUE  If the Char is an upper case hexadecmial character.
> +  @retval FALSE If the Char is not an upper case hexadecmial 
> character.
> 
>  **/
>  BOOLEAN
> @@ -115,7 +115,7 @@ VarCheckInternalIsHexaDecimalDigitCharacter (
>IN CHAR16 Char
>)
>  {
> -  return (BOOLEAN) ((Char >= L'0' && Char <= L'9') || (Char >= L'A' && Char
> <= L'F') || (Char >= L'a' && Char <= L'f'));
> +  return (BOOLEAN) ((Char >= L'0' && Char <= L'9') || (Char >= L'A' &&
> + Char <= L'F'));
>  }
> 
>  /**
> diff --git
> a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
> b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
> index 8f7126e6aea6..80dc6341adcf 100644
> --- a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
> +++ b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLibNullClass.c
> @@ -678,17 +678,17 @@ EFI_GUID *mUefiDefinedGuid[] = {  };
> 
>  /**
> -  Check if a Unicode character is a hexadecimal character.
> +  Check if a Unicode character is an upper case hexadecimal character.
> 
> -  This function checks if a Unicode character is a
> -  hexadecimal character.  The valid hexadecimal character is
> -  L'0' to L'9', L'a' to L'f', or L'A' to L'F'.
> +  This function checks if a Unicode character is an upper case
> + hexadecimal character.  The valid upper case hexadecimal character is
> + L'0' to L'9', or L'A' to L'F'.
> 
> 
>@param[in] Char   The character to check against.
> 
> -  @retval TRUE  If the Char is a hexadecmial character.
> -  @retval FALSE If the Char is not a hexadecmial character.
> +  @retval TRUE  If the Char is an upper case hexadecmial character.
> +  @retval FALSE If the Char is not an upper case hexadecmial 
> character.
> 
>  **/
>  BOOLEAN
> @@ -697,7 +697,7 @@ VarCheckUefiIsHexaDecimalDigitCharacter (
>IN CHAR16 Char
>)
>  {
> -  return (BOOLEAN) ((Char >= L'0' && Char <= L'9') || (Char >= L'A' && Char
> <= L'F') || (Char >= L'a' && Char <= L'f'));
> +  return (BOOLEAN) ((Char >= L'0' && Char <= L'9') || (Char >= L'A' &&
> + Char <= L'F'));
>  }
> 
>  /**
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel