Re: [edk2] [PATCH v2] NetworkPkg: Update HttpDxe to consume EFI_HTTP_UTILITIES_PROTOCOL

[Fu, Siyuan]

I think here should be always consume, other part is ok to me.
+  gEfiHttpUtilitiesProtocolGuid## TO_START


Reviewed-by: Fu Siyuan 



-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, August 25, 2015 9:01 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Samer 
El-Haj-Mahmoud 
Subject: [PATCH v2] NetworkPkg: Update HttpDxe to consume 
EFI_HTTP_UTILITIES_PROTOCOL

v2:
* Register a notification function to be executed for Http utilities protocol 
in the drivers entry points.

Since we add EFI_HTTP_UTILITIES_PROTOCOL support, HttpDxe driver should be 
updated to remove internal http utilities functions and consume this protocol 
directly.

Cc: Ye Ting 
Cc: Siyuan Fu 
Cc: Samer El-Haj-Mahmoud 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpDriver.c|  54 +++-
 NetworkPkg/HttpDxe/HttpDriver.h|   4 +-
 NetworkPkg/HttpDxe/HttpDxe.inf |   3 +-
 NetworkPkg/HttpDxe/HttpImpl.c  |  20 +-
 NetworkPkg/HttpDxe/HttpProto.c |  30 +-
 NetworkPkg/HttpDxe/HttpUtilities.c | 622 -
 NetworkPkg/HttpDxe/HttpUtilities.h |  82 -
 7 files changed, 94 insertions(+), 721 deletions(-)  delete mode 100644 
NetworkPkg/HttpDxe/HttpUtilities.c
 delete mode 100644 NetworkPkg/HttpDxe/HttpUtilities.h

diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c 
index 43f42e2..bd1d04e 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -13,10 +13,12 @@
 
 **/
 
 #include "HttpDriver.h"
 
+EFI_HTTP_UTILITIES_PROTOCOL *mHttpUtilities = NULL;
+
 ///
 /// Driver Binding Protocol instance
 ///
 EFI_DRIVER_BINDING_PROTOCOL gHttpDxeDriverBinding = {
   HttpDxeDriverBindingSupported,
@@ -99,10 +101,39 @@ HttpCleanService (
   );
   }
 }
 
 /**
+  The event process routine when the http utilities protocol is 
+ installed  in the system.
+
+  @param[in] Event Not used.
+  @param[in] Context   The pointer to the IP4 config2 instance data.
+
+**/
+VOID
+EFIAPI
+HttpUtilitiesInstalledCallback (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  gBS->LocateProtocol (
+ &gEfiHttpUtilitiesProtocolGuid, 
+ NULL, 
+ (VOID **) &mHttpUtilities
+ );
+
+  //
+  // Close the event if Http utilities protocol is loacted.
+  //
+  if (mHttpUtilities != NULL && Event != NULL) {
+ gBS->CloseEvent (Event);
+  }
+}
+
+/**
   This is the declaration of an EFI image entry point. This entry point is
   the same for UEFI Applications, UEFI OS Loaders, and UEFI Drivers including
   both device drivers and bus drivers.
 
   @param  ImageHandle   The firmware allocated handle for the UEFI 
image.
@@ -116,11 +147,32 @@ EFI_STATUS
 EFIAPI
 HttpDxeDriverEntryPoint (
   IN EFI_HANDLEImageHandle,
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
-{
+{ 
+  VOID   *Registration;
+
+  gBS->LocateProtocol (
+ &gEfiHttpUtilitiesProtocolGuid, 
+ NULL, 
+ (VOID **) &mHttpUtilities
+ );
+
+  if (mHttpUtilities == NULL) {
+//
+// No Http utilities protocol, register a notify.
+//
+EfiCreateProtocolNotifyEvent (
+  &gEfiHttpUtilitiesProtocolGuid,
+  TPL_CALLBACK,
+  HttpUtilitiesInstalledCallback,
+  NULL,
+  &Registration
+  );
+  }
+
   //
   // Install UEFI Driver Model protocol(s).
   //
   return EfiLibInstallDriverBindingComponentName2 (
ImageHandle,
diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h 
index 5bad705..d95b05b 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -39,10 +39,11 @@
 #include 
 
 //
 // Consumed Protocols
 //
+#include 
 #include 
 #include 
 #include 
 
 //
@@ -60,18 +61,19 @@
 //
 extern EFI_DRIVER_BINDING_PROTOCOL  gHttpDxeDriverBinding;  extern 
EFI_COMPONENT_NAME2_PROTOCOL gHttpDxeComponentName2;  extern 
EFI_COMPONENT_NAME_PROTOCOL  gHttpDxeComponentName;
 
+extern EFI_HTTP_UTILITIES_PROTOCOL  *mHttpUtilities;
+
 //
 // Include files with function prototypes  //  #include "ComponentName.h"
 #include "HttpImpl.h"
 #include "HttpProto.h"
 #include "HttpDns.h"
-#include "HttpUtilities.h"
 
 typedef struct {
   EFI_SERVICE_BINDING_PROTOCOL  *ServiceBinding;
   UINTN NumberOfChildren;
   EFI_HANDLE*ChildHandleBuffer;
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf 
index 4632934..d9652b3 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -36,12 +36,10 @@
   HttpDriver.c
   HttpImpl.h
   HttpImpl.c
   HttpProto.h
   HttpProto.c
-  HttpUtilities.h
-  HttpUtilities.c
 
 [LibraryClasses]
   UefiDriverEntryPoint
   UefiBootServicesTableLib
   MemoryAllocationLib
@@ -52,10 +50,11 @@
   HttpLib
 
 [Protocols]
   gEfiHttpServiceBindingProtocolGuid   ## BY_START
   gEfiHttpProtocolGuid

Re: [edk2] [Patch] NetworkPkg: Update the DnsDhcp.c to use BSD license

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 



-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, August 25, 2015 2:44 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan 
Subject: [Patch] NetworkPkg: Update the DnsDhcp.c to use BSD license

This patch is used to update the DnsDhcp.c to use BSD license.

Cc: Ye Ting 
Cc: Siyuan Fu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsDhcp.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsDhcp.c b/NetworkPkg/DnsDxe/DnsDhcp.c index 
a9fdfb6..6b409ba 100644
--- a/NetworkPkg/DnsDxe/DnsDhcp.c
+++ b/NetworkPkg/DnsDxe/DnsDhcp.c
@@ -1,16 +1,16 @@
 /** @file
 Functions implementation related with DHCPv4/v6 for DNS driver.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved. -This software 
and associated documentation (if any) is furnished -under a license and may 
only be used or copied in accordance -with the terms of the license. Except as 
permitted by such -license, no part of this software or documentation may be 
-reproduced, stored in a retrieval system, or transmitted in any -form or by 
any means without the express written consent of -Intel Corporation.
+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 "DnsImpl.h"
 
--
1.9.5.msysgit.1

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

[edk2] [PATCH] MdeModulePkg FaultTolerantWrite: Error handling for erase operation failure

[Star Zeng]

There may be anti-flash wear out feature to forbid erase operation after end of 
dxe.
The code is missing some error handling for erase operation failure,
it should return directly after the erase operation failed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Cc: Liming Gao 
---
 .../Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c | 16 ++--
 .../Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c | 11 ++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c 
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c
index f08f280..7a6c377 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c
@@ -3,7 +3,7 @@
   These are the common Fault Tolerant Write (FTW) functions that are shared 
   by DXE FTW driver and SMM FTW driver.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, 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
@@ -526,6 +526,11 @@ FtwWrite (
   // Do not assume Spare Block and Target Block have same block size
   //
   Status  = FtwEraseSpareBlock (FtwDevice);
+  if (EFI_ERROR (Status)) {
+FreePool (MyBuffer);
+FreePool (SpareBuffer);
+return EFI_ABORTED;
+  }
   Ptr = MyBuffer;
   for (Index = 0; MyBufferSize > 0; Index += 1) {
 if (MyBufferSize > FtwDevice->SpareBlockSize) {
@@ -585,6 +590,10 @@ FtwWrite (
   // Restore spare backup buffer into spare block , if no failure happened 
during FtwWrite.
   //
   Status  = FtwEraseSpareBlock (FtwDevice);
+  if (EFI_ERROR (Status)) {
+FreePool (SpareBuffer);
+return EFI_ABORTED;
+  }
   Ptr = SpareBuffer;
   for (Index = 0; Index < FtwDevice->NumberOfSpareBlock; Index += 1) {
 MyLength = FtwDevice->SpareBlockSize;
@@ -706,7 +715,10 @@ FtwRestart (
   // Erase Spare block
   // This is restart, no need to keep spareblock content.
   //
-  FtwEraseSpareBlock (FtwDevice);
+  Status = FtwEraseSpareBlock (FtwDevice);
+  if (EFI_ERROR (Status)) {
+return EFI_ABORTED;
+  }
 
   DEBUG ((EFI_D_ERROR, "Ftw: Restart() success \n"));
   return EFI_SUCCESS;
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c 
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
index f3e6eac..31f1e0b 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
@@ -2,7 +2,7 @@
 
Internal functions to operate Working Block Space.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, 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
@@ -510,6 +510,11 @@ FtwReclaimWorkSpace (
   // Write the memory buffer to spare block
   //
   Status  = FtwEraseSpareBlock (FtwDevice);
+  if (EFI_ERROR (Status)) {
+FreePool (TempBuffer);
+FreePool (SpareBuffer);
+return EFI_ABORTED;
+  }
   Ptr = TempBuffer;
   for (Index = 0; TempBufferSize > 0; Index += 1) {
 if (TempBufferSize > FtwDevice->SpareBlockSize) {
@@ -584,6 +589,10 @@ FtwReclaimWorkSpace (
   // Restore spare backup buffer into spare block , if no failure happened 
during FtwWrite.
   //
   Status  = FtwEraseSpareBlock (FtwDevice);
+  if (EFI_ERROR (Status)) {
+FreePool (SpareBuffer);
+return EFI_ABORTED;
+  }
   Ptr = SpareBuffer;
   for (Index = 0; Index < FtwDevice->NumberOfSpareBlock; Index += 1) {
 Length = FtwDevice->SpareBlockSize;
-- 
1.9.5.msysgit.0

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Andrew Fish]

> On Aug 25, 2015, at 8:18 PM, Benjamin Herrenschmidt 
>  wrote:
> 
> On Wed, 2015-08-26 at 01:53 +, Tian, Feng wrote:
>> Hi, Ben & Andrew
>> 
>> The GetBarAttributes() returns a set of ACPI 2.0 resource descriptors
>> that describe the current configuration of the BARs of the PCI
>> controller.
>> 
>> What I understood is the returned value of GetBarAttributes() is a
>> CPU address rather than a PCI address. From the view of PciIo, nobody
>> needs to know/use Bar's PCI address.
> 
> So that needs to be clarified in the spec (see the discussion on the
> workgroup, some people seem to disagree :)
> 

It seems the ACPI spec is quite clear on the definition (UEFI Forum also owns 
the ACPI spec :)).
It might be good to clarify? But we should discuss UEFI Forum stuff on that 
mailing list as it is not related to the open source project. 

> I'm happy with this approach however. It matches driver expectations
> and doesn't require changing the APIs.
> 
> It will require some churn inside PciBusDxe to make it cope with the
> offsets but that's less of a problem as the overall APIs to drivers are
> unaffected and thus it's just a matter of me doing it :)
> 

Thanks,

Andrew Fish

> Cheers,
> Ben.
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_mailman_listinfo_edk2-2Ddevel&d=BQICAg&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw&m=B_M5JYhsLK3VkzURWBjiFCMMT7-atKMChNG-Pub_BZQ&s=uqP460LXZNzdNQAmSrufFeY7oZXVsVH-oX7moNnl_S8&e=
>  

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Wed, 2015-08-26 at 01:53 +, Tian, Feng wrote:
> Hi, Ben & Andrew
> 
> The GetBarAttributes() returns a set of ACPI 2.0 resource descriptors
> that describe the current configuration of the BARs of the PCI
> controller.
> 
> What I understood is the returned value of GetBarAttributes() is a
> CPU address rather than a PCI address. From the view of PciIo, nobody
> needs to know/use Bar's PCI address.

So that needs to be clarified in the spec (see the discussion on the
workgroup, some people seem to disagree :)

I'm happy with this approach however. It matches driver expectations
and doesn't require changing the APIs.

It will require some churn inside PciBusDxe to make it cope with the
offsets but that's less of a problem as the overall APIs to drivers are
unaffected and thus it's just a matter of me doing it :)

Cheers,
Ben.

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Tue, 2015-08-25 at 19:37 -0700, Andrew Fish wrote:
> But it looks like the default in ACPI is a CPU (processor-relative)
> address. Thus if the mapping is not 1:1 "Address Translation Offset”
> is the offset for the bus view. 
> 
> I see from the PCI bus driver that there is not a path to return the
> "Address Translation Offset”. It seems like the PCI Root Bridge IO
> protocol could return the ""Address Translation Offset”, and the PCI
> Bus driver could save that per Bar and then return that value in the
> ACPI Address Space Descriptor. Maybe Ben could look into that a
> propose a patch if it works. 

That would be an option. The main problem with that is that currently
the PciBusDxe code seems to be hijacking that field for other purposes
but I could look into it.

So if I understand things properly:

For any ACPI resource descriptor exchange between the root bus driver
and the core, the address value is a CPU address and the Address
Translation Offset is an offset to apply to obtain the PCI address ?

In most cases it's going to be negative, but I assume unsigned 2
-complement arithmetic should work fine here, it will jsut look like an
f* value. Or is the offset meant to be substracted ?

Doing that means that the existing GetBarAttributes() for getting a CPU
address usage by drivers will just work.

However, any driver using it and expecting a *BAR* value for whatever
other purpose would be broken, but that's less of a concern as it is
easy to fix (either by applying the offset which we can return as well,
or by just reading the BAR directly).

I still think we need to clarify this in the UEFI spec however, to make
sure everybody is on the same page.

From there I can give a try at fixing up PciBusDxe (though it may take
me a while, the resource allocation code in there is far from trivial).

Cheers,
Ben.

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

[edk2] [PATCH] ShellPkg: Fix the ASSERT issue in drvcfg command.

[Qiu Shumin]

Initialize the local pointer to avoid to free a dangling pointer.

Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c 
b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
index 27b8873..b2ba8ff 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvCfg.c
@@ -1181,6 +1181,8 @@ ShellCommandRunDrvCfg (
   Status = CommandInit();
   ASSERT_EFI_ERROR(Status);
 
+  ProblemParam = NULL;
+
   //
   // parse the command line
   //
-- 
1.9.5.msysgit.1

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Andrew Fish]

> On Aug 25, 2015, at 6:53 PM, Tian, Feng  wrote:
> 
> Hi, Ben & Andrew
> 
> The GetBarAttributes() returns a set of ACPI 2.0 resource descriptors that 
> describe the current configuration of the BARs of the PCI controller.
> 
> What I understood is the returned value of GetBarAttributes() is a CPU 
> address rather than a PCI address. From the view of PciIo, nobody needs to 
> know/use Bar's PCI address.
> 

I needed to go look it up in the ACPI spec to be sure, so it took me a while….

5.1.1 Address Space Translation
Some platforms may contain bridges that perform translations as I/O and/or 
Memory cycles pass through the bridges. This translation can take the form of 
the addition or subtraction of an offset. Or it can take the form of a 
conversion from I/O cycles into Memory cycles and back again. When translation 
takes place, the addresses placed on the processor bus by the processor during 
a read or write cycle are not the same addresses that are placed on the I/O bus 
by the I/O bus bridge. The address the processor places on the processor bus 
will be known here as the processor-relative address. And the address that the 
bridge places on the I/O bus will be known as the bus-relative address. Unless 
otherwise noted, all addresses used within this section are processor-relative 
addresses.
For example, consider a platform with two root PCI buses. The platform designer 
has several choices. One solution would be to split the 16-bit I/O space into 
two parts, assigning one part to the first root PCI bus and one part to the 
second root PCI bus. Another solution would be to make both root PCI buses 
decode the entire 16-bit I/O space, mapping the second root PCI bus’s I/O space 
into memory space. In this second scenario, when the processor needs to read 
from an I/O register of a device underneath the second root PCI bus, it would 
need to perform a memory read within the range that the root PCI bus bridge is 
using to map the I/O space.
…
Note: Industry standard PCs do not provide address space translations because 
of historical compatibility issues.



But it looks like the default in ACPI is a CPU (processor-relative) address. 
Thus if the mapping is not 1:1 "Address Translation Offset” is the offset for 
the bus view. 

I see from the PCI bus driver that there is not a path to return the "Address 
Translation Offset”. It seems like the PCI Root Bridge IO protocol could return 
the ""Address Translation Offset”, and the PCI Bus driver could save that per 
Bar and then return that value in the ACPI Address Space Descriptor. Maybe Ben 
could look into that a propose a patch if it works. 

Thanks,

Andrew Fish

> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Benjamin Herrenschmidt
> Sent: Wednesday, August 26, 2015 07:54
> To: Andrew Fish
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] PCI and non-1:1 mapping of MMIO space
> 
> (Argh, I keep sending these from my IBM address which isn't the one on the 
> list. Sorry Andrew for the duplicates).
> 
> On Tue, 2015-08-25 at 14:48 -0700, Andrew Fish wrote:
> 
>> 
>>> So if we stick to the definition of GetBarAttributes() only 
>>> returning a BAR value, then it's impossible to implement a PCI GOP 
>>> driver by following the spec.
>>> 
>> I came to that conclusion too. 
> 
> :(
> 
>>> Now, there's one thing that might work, which would be to exploit 
>>> the "AddressTranslationOffset" field of the ACPI resource 
>>> descriptor.
>>> 
>> How do you represent a system like this in ACPI? It might be a good 
>> idea to solve it in a similar way.
> 
> Unfortunately, I am not familiar with the details of ACPI, I only have a high 
> level understanding of it. Our platforms have been Open Firmware based for as 
> far as I've been involved with them and lately, we use C -generated flat 
> device-tree along with a custom runtime firmwarealled a  OPAL).
> 
> At this point I'm not (yet) considering a move to ACPI. I'm doing a proof of 
> concept ppc64 UEFI port to get a feel of the water temperature more than 
> anything else (though I'll be happy to contribute things back once I get the 
> appropriate legalese sorted internally to IBM).
> 
>> The PCI IO was designed to not required the 1:1 mapping. The direct 
>> access to the Framebuffer came along later, as was mostly a fallback 
>> for the OS on a safe mode boot kind of thing. I think we missed 
>> passing the frame buffer out on a system that was not 1:1.
> 
> Ok. Well, it will be needed even on non-1:1 systems, we will have similar 
> requirements of the OS needing an early boot framebuffer etc...
> and I suspect the desire to directly access BARs (at least prefetchable
> ones) from the CPU isn't going away in other areas either.
> 
> So maybe we should try to address this. I'm trying to figure out how to join 
> the appropriate working groups etc... so I can participate in the spec side 
> discussion as well (which I'll need t

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Tian, Feng]

Hi, Ben & Andrew

The GetBarAttributes() returns a set of ACPI 2.0 resource descriptors that 
describe the current configuration of the BARs of the PCI controller.

What I understood is the returned value of GetBarAttributes() is a CPU address 
rather than a PCI address. From the view of PciIo, nobody needs to know/use 
Bar's PCI address.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Benjamin 
Herrenschmidt
Sent: Wednesday, August 26, 2015 07:54
To: Andrew Fish
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] PCI and non-1:1 mapping of MMIO space

(Argh, I keep sending these from my IBM address which isn't the one on the 
list. Sorry Andrew for the duplicates).

On Tue, 2015-08-25 at 14:48 -0700, Andrew Fish wrote:

> 
> > So if we stick to the definition of GetBarAttributes() only 
> > returning a BAR value, then it's impossible to implement a PCI GOP 
> > driver by following the spec.
> > 
> I came to that conclusion too. 

:(

> > Now, there's one thing that might work, which would be to exploit 
> > the "AddressTranslationOffset" field of the ACPI resource 
> > descriptor.
> > 
> How do you represent a system like this in ACPI? It might be a good 
> idea to solve it in a similar way.

Unfortunately, I am not familiar with the details of ACPI, I only have a high 
level understanding of it. Our platforms have been Open Firmware based for as 
far as I've been involved with them and lately, we use C -generated flat 
device-tree along with a custom runtime firmwarealled a  OPAL).

At this point I'm not (yet) considering a move to ACPI. I'm doing a proof of 
concept ppc64 UEFI port to get a feel of the water temperature more than 
anything else (though I'll be happy to contribute things back once I get the 
appropriate legalese sorted internally to IBM).

> The PCI IO was designed to not required the 1:1 mapping. The direct 
> access to the Framebuffer came along later, as was mostly a fallback 
> for the OS on a safe mode boot kind of thing. I think we missed 
> passing the frame buffer out on a system that was not 1:1.

Ok. Well, it will be needed even on non-1:1 systems, we will have similar 
requirements of the OS needing an early boot framebuffer etc...
and I suspect the desire to directly access BARs (at least prefetchable
ones) from the CPU isn't going away in other areas either.

So maybe we should try to address this. I'm trying to figure out how to join 
the appropriate working groups etc... so I can participate in the spec side 
discussion as well (which I'll need to do for other reasons anyway if we are 
ever going to officially defining the ppc64 ABI in the spec).

Cheers,
Ben.

> Thanks,
> 
> Andrew Fish
> 
> > Cheers,
> > Ben.
___
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] Measured boot in OVMF with QEMU TPM passthrough?

[Yao, Jiewen]

Do you mean TcgPei is added in FDF, but not dispatched by PEI core?
If so, please make sure you have below dependency satisfied. I guess you have 
1st and 2nd.
For 3rd, you can consider adding Tcg2Config driver, or produce 
gEfiTpmDeviceSelectedGuid in your platform module.

[Depex]
  gEfiPeiMasterBootModePpiGuid AND
  gEfiPeiReadOnlyVariable2PpiGuid AND
  gEfiTpmDeviceSelectedGuid

-Original Message-
From: David Van Arnem [mailto:dvanar...@cmlab.biz] 
Sent: Wednesday, August 26, 2015 3:28 AM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen
Subject: Re: [edk2] Measured boot in OVMF with QEMU TPM passthrough?

On 08/22/2015 06:41 AM, Yao, Jiewen wrote:
> Hi
> Some clarification:
> 1) TPM is to support TCG trusted boot, it is not related to UEFI secure boot. 
> So enabling UEFI secure boot does not mean TCG trusted boot is enabled.
Ok, thank you.  I knew UEFI secure boot was not the same as TCG trusted boot, 
but I wasn't sure how related they were.  My focus is TCG trusted boot.
> 2) I think you need add both TcgPei and TcgDxe to record measured boot log, 
> because TPM device is started from TcgPei. Just adding TcgDxe may still cause 
> error.
I am using a TPM1.2, so I have added the TcgPei, TcgDxe, and related modules to 
OVMF (in both the platform DSC and FDF files), and I modeled my changes after 
the Vlv2TbltDevicePkg files.  I spoke incorrectly in one of my previous emails; 
I see PhysicalPresencePei and TcgPei in both Ovmf.map and PEIFV.Fv.map after 
OVMF build, so they should be present in the OVMF image, but they are not being 
loaded according to the OVMF debug output.  The DXE TCG modules *are* being 
loaded, but are throwing an error presumably because the necessary setup was 
not performed during PEI.  I thought that the SEC or PEI core would kind of 
"automatically" 
find and load them, so I have not been able to determine why they are not 
loading.  Do you have any thoughts on this?
> 3) TcgSmm is to support TPM PP and MOR, which is NOT related to measured boot 
> record.
Ok, thank you again.
>
> BTW: Would you please let me know your TPM passthru is for TPM1.2 or TPM2.0?
> For later, you need include Tcg2Pei/Tcg2Dxe, instead of TcgPei/TcgDxe.
>
> Let me know if you have more question.
>
> Thank you
> Yao Jiewen
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> David Van Arnem
> Sent: Saturday, August 22, 2015 12:44 AM To:edk2-devel@lists.01.org
> Subject: Re: [edk2] Measured boot in OVMF with QEMU TPM passthrough?
>
> Hello again,
>
> I've done some analysis and here's where I'm at with measured boot with OVMF 
> in a QEMU guest:
>
> I've verified that most of the relevant modules that need to be added 
> according to the instructions 
> athttp://tianocore.sourceforge.net/wiki/How_to_Enable_Security#HOW_TO_
> ENABLE_TCG_TPM are being placed in the OVMF image by looking at 
> Ovmf.map.  They are also loaded when OVMF boots, which I've verified from the 
> OVMF debug output.  I believe the only one I've added to OvmfX64.dsc/fdf that 
> does not appear in the map or debug output is TcgSmm, which handles the ACPI 
> methods for TCG.  I think this is a problem: I do see a TCPA table when I 
> dump the ACPI tables in my guest, and it has an address to the start of the 
> Event Log, but I still don't have any measurements recorded in 
> /sys/kernel/security/tpm0/ascii_bios_measurements.  And, the PCR values from 
> /sys/class/tpm/tpm0/device/pcrs don't change whether I have OVMF Secure Boot 
> enabled or not (I think it's supposed to extend PCR[07]?).
>
> So, my question about whether or not measured boot with event logging should 
> be possible in OVMF with a QEMU VM still stands. Additonally, I'm curious if 
> the way I've added components to the OvmfX64.dsc file is correct.  For 
> example, here's how I added the TcgDxe component:
>
> SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {
>   
> TpmCommLib|SecurityPkg/Library/TpmCommLib/TpmCommLib.inf
> }
>
> I added the LibraryClasses subsection after the build tool complained about 
> an instance of that class not being found, and did the same for the other 
> modules that had the same issue.  I see other LibraryClasses in TcgDxe.inf 
> that don't require an instance in the subsection, why is that?
>
> Finally, where in the code should I do steps 1 and 2 (clear memory, process 
> request) from the instructions linked above?
>
> Any guidance would be appreciated.
>
> Thanks,
> David
>
> On 08/19/2015 05:17 PM, David Van Arnem wrote:
>> Hello,
>>
>> Should it be possible to perform measured boot in OVMF to measure a 
>> QEMU guest (extend and log PCRs) using a TPM passed-through from the 
>> host?
>>
>> I have a host machine with a TPM (v1.2), and a QEMU Linux guest 
>> booting using an OVMF image with the modifications suggested in the 
>> following link (modifications were done to OvmfX64.dsc):
>> http://tianocore.sourceforge.net/wiki/How_to_Enable_Security  
>> (section "How To Enable TCG T

Re: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer ring when timeout happens

[Tian, Feng]

Hi, Jaben

The special check in xHCI is making additional clean-up on h/w and the clean-up 
is transparent for the caller. Other errors don't need that. The caller would 
get a non-zero status value and know there is error happened accordingly.

Thanks
Feng

-Original Message-
From: Carsey, Jaben 
Sent: Wednesday, August 26, 2015 01:14
To: Tian, Feng; Anbazhagan, Baraneedharan; Zeng, Star
Cc: edk2-devel@lists.01.org; Tian, Feng; Carsey, Jaben
Subject: RE: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer ring 
when timeout happens

What about other error return values in *TransferResult?  Both places you check 
for a specific set of errors, but the set is not exhaustive and if a different 
error occurs, will the caller know?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Tian, Feng
> Sent: Monday, August 24, 2015 8:17 PM
> To: Anbazhagan, Baraneedharan ; Zeng, Star
> 
> Cc: edk2-devel@lists.01.org; Tian, Feng 
> Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer
> ring when timeout happens
> 
> Thanks for your comments, Baranee. I updated the patch as below. Please
> review again.
> 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |  75 +++---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 193
> +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  80 +++
>  MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c   |  51 +--
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 235
> +++
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h |  81 ++-
>  6 files changed, 618 insertions(+), 97 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 390ca0a..39c28ab 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -905,17 +905,28 @@ XhcControlTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcControlTransfer:
> XhcDequeueTrbFromEndpoint failed\n"));
>  }
> -Status = EFI_DEVICE_ERROR;
>  goto FREE_URB;
>} else {
> -goto FREE_URB;
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcControlTransfer:
> XhcRecoverHaltedEndpoint failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
> +  goto FREE_URB;
> +} else {
> +  goto FREE_URB;
> +}
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1241,14 +1252,24 @@ XhcBulkTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint
> failed\n"));
> +}
> +  } else {
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
>  }
> -Status = EFI_DEVICE_ERROR;
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1538,14 +1559,24 @@ XhcSyncInterruptTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer:
> XhcRecoverHaltedEndpoint failed\n"));
> +  if (Status == EFI_TIMEOU

Re: [edk2] [patch] SecurityPkg: Fix one returned code issue in P7Verify Protocol

[Zhang, Chao B]

Qin:
  The patch is good to me.
  Reviewed-by: Chao Zhang 





Thanks & Best regards
Chao Zhang

-Original Message-
From: Long, Qin 
Sent: Wednesday, August 26, 2015 1:24 AM
To: Zhang, Chao B; edk2-devel@lists.01.org
Subject: [patch] SecurityPkg: Fix one returned code issue in P7Verify Protocol

VerifyBuffer() in PKCS7 Verify Protocol should return EFI_UNSUPPORTED when the 
embedded content is found in SignedData but InData is not NULL.
This patch is to comply with the spec definition.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 13c9138..07fdf55 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -833,6 +833,13 @@ VerifyBuffer (
 return EFI_UNSUPPORTED;
   }
   if (AttachedData != NULL) {
+if (InData != NULL) {
+  //
+  // The embedded content is found in SignedData but InData is not NULL
+  //
+  Status = EFI_UNSUPPORTED;
+  goto _Exit;
+}
 //
 // PKCS7-formatted signedData with attached content; Use the embedded
 // content for verification
--
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] NetworkPkg: Update the DnsDhcp.c to use BSD license

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, August 25, 2015 2:44 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: [Patch] NetworkPkg: Update the DnsDhcp.c to use BSD license

This patch is used to update the DnsDhcp.c to use BSD license.

Cc: Ye Ting 
Cc: Siyuan Fu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/DnsDxe/DnsDhcp.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsDhcp.c b/NetworkPkg/DnsDxe/DnsDhcp.c index 
a9fdfb6..6b409ba 100644
--- a/NetworkPkg/DnsDxe/DnsDhcp.c
+++ b/NetworkPkg/DnsDxe/DnsDhcp.c
@@ -1,16 +1,16 @@
 /** @file
 Functions implementation related with DHCPv4/v6 for DNS driver.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved. -This software 
and associated documentation (if any) is furnished -under a license and may 
only be used or copied in accordance -with the terms of the license. Except as 
permitted by such -license, no part of this software or documentation may be 
-reproduced, stored in a retrieval system, or transmitted in any -form or by 
any means without the express written consent of -Intel Corporation.
+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 "DnsImpl.h"
 
--
1.9.5.msysgit.1

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

(Argh, I keep sending these from my IBM address which isn't the one on
the list. Sorry Andrew for the duplicates).

On Tue, 2015-08-25 at 14:48 -0700, Andrew Fish wrote:

> 
> > So if we stick to the definition of GetBarAttributes() only
> > returning a
> > BAR value, then it's impossible to implement a PCI GOP driver by
> > following the spec.
> > 
> I came to that conclusion too. 

:(

> > Now, there's one thing that might work, which would be to exploit
> > the
> > "AddressTranslationOffset" field of the ACPI resource descriptor.
> > 
> How do you represent a system like this in ACPI? It might be a good
> idea to solve it in a similar way. 

Unfortunately, I am not familiar with the details of ACPI, I only have
a high level understanding of it. Our platforms have been Open Firmware
based for as far as I've been involved with them and lately, we use C
-generated flat device-tree along with a custom runtime firmwarealled a
 OPAL).

At this point I'm not (yet) considering a move to ACPI. I'm doing a
proof of concept ppc64 UEFI port to get a feel of the water temperature
more than anything else (though I'll be happy to contribute things back
once I get the appropriate legalese sorted internally to IBM).

> The PCI IO was designed to not required the 1:1 mapping. The direct
> access to the Framebuffer came along later, as was mostly a fallback
> for the OS on a safe mode boot kind of thing. I think we missed
> passing the frame buffer out on a system that was not 1:1.

Ok. Well, it will be needed even on non-1:1 systems, we will have
similar requirements of the OS needing an early boot framebuffer etc...
and I suspect the desire to directly access BARs (at least prefetchable
ones) from the CPU isn't going away in other areas either.

So maybe we should try to address this. I'm trying to figure out how to
join the appropriate working groups etc... so I can participate in the
spec side discussion as well (which I'll need to do for other reasons
anyway if we are ever going to officially defining the ppc64 ABI in the
spec).

Cheers,
Ben.

> Thanks,
> 
> Andrew Fish
> 
> > Cheers,
> > Ben.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] skip enumeration of a PCI device

[Hamel, Lee M]

The issue is being able to perform actions before and after the normal 
enumeration.  Think of it as a timing issue, and a SW fix for a HW issue on an 
early rev of silicon.
I know I can have the platform notification functions that are called pre and 
post actions in PciEnumerator.c:PciEnumerator().

There was a function disable bit to hide the device which I did implement, but 
this just hides the problem.

-Original Message-
From: Tim Lewis [mailto:tim.le...@insyde.com] 
Sent: Tuesday, August 25, 2015 4:01 PM
To: Hamel, Lee M; edk2-devel@lists.01.org
Subject: RE: [edk2] skip enumeration of a PCI device

Lee -- 

How will you prevent the OS from touching this device? It seems that preventing 
the BIOS from touching it only delays your pain ;-)

Can you disable this PCI device entirely? I.e. make it disappear from config 
space?

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hamel, 
Lee M
Sent: Wednesday, August 26, 2015 6:44 AM
To: edk2-devel@lists.01.org
Subject: [edk2] skip enumeration of a PCI device

oI have a platform that has an override for MdeModulePkg\Bus\Pci\PciBusDxe. 
 My goal is to remove this platform override.  There are times when a 
particular stepping of silicon needs to be skipped due to a HW bug, e.g. on a 
particular Bus/Device/Function, read the PCI Config Header and check the 
DeviceID value.  The following override files of PciBusDxe have changes:

1.  PciEnumeratorSupport.c: skip enumeration of a PCI device that is a TXT 
device

2.  PciDeviceSupport.c: skip video device registration



It doesn't appear I can install a platform PCI protocol on 
EFI_PCI_PLATFORM_PROTOCOL and have only PciDevicePresent() overridden.  This 
function touches the TXT device and causes the problem.  I can't use 
EFI_PCI_PLATFORM_PROTOCOL and override a greater scope of code because the core 
package bus enumeration is called after the platform notification functions, 
thus the TXT device is still touched.  I don't see a mechanism to tell core 
code to not touch particular PCI devices.



A.  Is there a mechanism to tell core code to not touch particular PCI 
devices?

B.  If A is false, then the route may be for me to submit code changes to 
achieve the goal.


___
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] skip enumeration of a PCI device

[Tim Lewis]

Lee -- 

How will you prevent the OS from touching this device? It seems that preventing 
the BIOS from touching it only delays your pain ;-)

Can you disable this PCI device entirely? I.e. make it disappear from config 
space?

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hamel, 
Lee M
Sent: Wednesday, August 26, 2015 6:44 AM
To: edk2-devel@lists.01.org
Subject: [edk2] skip enumeration of a PCI device

oI have a platform that has an override for MdeModulePkg\Bus\Pci\PciBusDxe. 
 My goal is to remove this platform override.  There are times when a 
particular stepping of silicon needs to be skipped due to a HW bug, e.g. on a 
particular Bus/Device/Function, read the PCI Config Header and check the 
DeviceID value.  The following override files of PciBusDxe have changes:

1.  PciEnumeratorSupport.c: skip enumeration of a PCI device that is a TXT 
device

2.  PciDeviceSupport.c: skip video device registration



It doesn't appear I can install a platform PCI protocol on 
EFI_PCI_PLATFORM_PROTOCOL and have only PciDevicePresent() overridden.  This 
function touches the TXT device and causes the problem.  I can't use 
EFI_PCI_PLATFORM_PROTOCOL and override a greater scope of code because the core 
package bus enumeration is called after the platform notification functions, 
thus the TXT device is still touched.  I don't see a mechanism to tell core 
code to not touch particular PCI devices.



A.  Is there a mechanism to tell core code to not touch particular PCI 
devices?

B.  If A is false, then the route may be for me to submit code changes to 
achieve the goal.


___
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] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Wed, 2015-08-26 at 07:58 +1000, Benjamin Herrenschmidt wrote:
> Another one is the UFS Host Controller protocol though it's local to
> EDK2 thankfully and not part of the spec. Here too, the protocol
> definition and underlying implementation rely on direct access.
> 
> I understand the worry with letting direct access to BARs. The
> semantics and ordering especially can be very platform/architecture
> dependent. Memory barriers might be needed for accesses.
> 
> It might make sense, if we provide a direct "map" API to define that
> it
> is weakly ordered and have an explicit barrier callback as well in
> there. Additionally it might make sense to also restrict it to
> prefetchable BARs.

Another approach, less clean than a new protocol but probably easier
would be to define a new BAR attribute "CPU mapped" which a driver can
set. As a result of that, a subsequent GetBarAttributes() would return
a processor physical address.

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

[edk2] skip enumeration of a PCI device

[Hamel, Lee M]

oI have a platform that has an override for MdeModulePkg\Bus\Pci\PciBusDxe. 
 My goal is to remove this platform override.  There are times when a 
particular stepping of silicon needs to be skipped due to a HW bug, e.g. on a 
particular Bus/Device/Function, read the PCI Config Header and check the 
DeviceID value.  The following override files of PciBusDxe have changes:

1.  PciEnumeratorSupport.c: skip enumeration of a PCI device that is a TXT 
device

2.  PciDeviceSupport.c: skip video device registration



It doesn't appear I can install a platform PCI protocol on 
EFI_PCI_PLATFORM_PROTOCOL and have only PciDevicePresent() overridden.  This 
function touches the TXT device and causes the problem.  I can't use 
EFI_PCI_PLATFORM_PROTOCOL and override a greater scope of code because the core 
package bus enumeration is called after the platform notification functions, 
thus the TXT device is still touched.  I don't see a mechanism to tell core 
code to not touch particular PCI devices.



A.  Is there a mechanism to tell core code to not touch particular PCI 
devices?

B.  If A is false, then the route may be for me to submit code changes to 
achieve the goal.


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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Tue, 2015-08-25 at 14:45 -0700, Andrew Fish wrote:
> Well the GOP returns a FrameBuffer, so that address needs to be
> adjusted. So that seems to be the bug in the spec. I’ll bring this up
> to the UEFI Forum. 
> 
> But I think for the non FrameBuffer case it would be better to fix
> the drivers. 

Another one is the UFS Host Controller protocol though it's local to
EDK2 thankfully and not part of the spec. Here too, the protocol
definition and underlying implementation rely on direct access.

I understand the worry with letting direct access to BARs. The
semantics and ordering especially can be very platform/architecture
dependent. Memory barriers might be needed for accesses.

It might make sense, if we provide a direct "map" API to define that it
is weakly ordered and have an explicit barrier callback as well in
there. Additionally it might make sense to also restrict it to
prefetchable BARs.

Cheers,
Ben.

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Tue, 2015-08-25 at 14:45 -0700, Andrew Fish wrote:
> 
> Well the GOP returns a FrameBuffer, so that address needs to be
> adjusted. So that seems to be the bug in the spec. I’ll bring this up
> to the UEFI Forum. 
> 
> But I think for the non FrameBuffer case it would be better to fix
> the drivers. 

Should we consider an optional protocol extension (or new protocol) to
the PCI IO protocol that provides "BarMap" and "BarUnmap" semantics
allowing a driver to effectively map a BAR into the CPU address space ?

On Intel those would just return the BAR address.

I am not yet familiar enough with UEFI to have a good grasp of how one
extends a protocol... I assume we could create a new PCI_IO_PROTOCOL_EX
that contains the new calls with its own GUID that a device can
optionally implement (which PciBusDxe would instanciate if the
corresponding root bridge protocol extension exists).

The other option as I mentioned earlier is to see if we can make use of
the translation filed in the ACPI resource descriptor, though I'm not
familiar with how it's usually used on ACPI based platforms.

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Andrew Fish]

Thanks,

Andrew Fish




> On Aug 25, 2015, at 2:43 PM, Benjamin Herrenschmidt  wrote:
> 
> On Wed, 2015-08-26 at 07:18 +1000, Benjamin Herrenschmidt wrote:
>> 
>>> I think you have it backwards. The UEFI spec abstracts PCI
>>> properly,
>>> and drivers are implemented incorrectly, but they work due to x86
>>> PC
>>> assumptions. 
>>> 
>>> GetBarAttributes() returns the BAR. 
>> 
>> Well, the spec doesn't spell it out precisely but ...
>> 
>>> That is the correct thing to do. If that Bar was accessed via>
>>> EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI
>>> _P
>>> CI_IO_PROTOCOL.CopyMem()  then it is being accessed relative to the
>>> BAR and everything should work. Casting a pointer to the value
>>> returned in GetBarAttributes() is not guaranteed to work. 
>> 
>> Except that a lot drivers do it :-(
>> 
>> For example anything using OptionRomPkg/Library/BitBltLib does it.
>> 
>> So the GOP drivers in-tree in EDK2 do it, I can ask nVidia what their
>> EFI driver does but I woudn't be surprised if it's everywhere.
> 
> Ok, correction, this is even worse. You *cannot* implement a complete
> GOP driver without that assumption. A GOP driver is supposed to provide
> a EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure, which contains a
> FrameBufferBase which is defined to be the CPU physical address of the
> frame buffer.
> 
> So if we stick to the definition of GetBarAttributes() only returning a
> BAR value, then it's impossible to implement a PCI GOP driver by
> following the spec.
> 

I came to that conclusion too. 

> Now, there's one thing that might work, which would be to exploit the
> "AddressTranslationOffset" field of the ACPI resource descriptor.
> 

How do you represent a system like this in ACPI? It might be a good idea to 
solve it in a similar way. 

> AFAIK, the UEFI spec doesn't do a very good job at describing how it
> uses these descriptors (this is an interesting "dependency" between
> UEFI and ACPI btw, my platform otherwise doesn't use ACPI). EDK2
> hijacks that field for other means bct that should be fixable.
> 
> I'll try to hop on the UEFI call tomorrow (morning my time, ie, .au) in
> place of the IBM rep (it's too early for Jeremy anyway) and start that
> discussion.
> 
> As it is, there is a contradiction in the spec between PCI trying to
> prevent direct access to BAR regions and at least GOP requiring to pass
> a physical address pointing to the device (and possibly other IO
> protocols, I haven't looked in details).
> 

The PCI IO was designed to not required the 1:1 mapping. The direct access to 
the Framebuffer came along later, as was mostly a fallback for the OS on a safe 
mode boot kind of thing. I think we missed passing the frame buffer out on a 
system that was not 1:1.

Thanks,

Andrew Fish

> Cheers,
> Ben.
> 

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Wed, 2015-08-26 at 07:18 +1000, Benjamin Herrenschmidt wrote:
> 
> > I think you have it backwards. The UEFI spec abstracts PCI
> > properly,
> > and drivers are implemented incorrectly, but they work due to x86
> > PC
> > assumptions. 
> > 
> > GetBarAttributes() returns the BAR. 
> 
> Well, the spec doesn't spell it out precisely but ...
> 
> > That is the correct thing to do. If that Bar was accessed via>
> > EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI
> > _P
> > CI_IO_PROTOCOL.CopyMem()  then it is being accessed relative to the
> > BAR and everything should work. Casting a pointer to the value
> > returned in GetBarAttributes() is not guaranteed to work. 
> 
> Except that a lot drivers do it :-(
>  
> For example anything using OptionRomPkg/Library/BitBltLib does it.
> 
> So the GOP drivers in-tree in EDK2 do it, I can ask nVidia what their
> EFI driver does but I woudn't be surprised if it's everywhere.

Ok, correction, this is even worse. You *cannot* implement a complete
GOP driver without that assumption. A GOP driver is supposed to provide
a EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE structure, which contains a
 FrameBufferBase which is defined to be the CPU physical address of the
frame buffer.

So if we stick to the definition of GetBarAttributes() only returning a
BAR value, then it's impossible to implement a PCI GOP driver by
following the spec.

Now, there's one thing that might work, which would be to exploit the
"AddressTranslationOffset" field of the ACPI resource descriptor.

AFAIK, the UEFI spec doesn't do a very good job at describing how it
uses these descriptors (this is an interesting "dependency" between
UEFI and ACPI btw, my platform otherwise doesn't use ACPI). EDK2
hijacks that field for other means bct that should be fixable.

I'll try to hop on the UEFI call tomorrow (morning my time, ie, .au) in
place of the IBM rep (it's too early for Jeremy anyway) and start that
discussion.

As it is, there is a contradiction in the spec between PCI trying to
prevent direct access to BAR regions and at least GOP requiring to pass
a physical address pointing to the device (and possibly other IO
protocols, I haven't looked in details).

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

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Andrew Fish]

Thanks,

Andrew Fish




> On Aug 25, 2015, at 2:18 PM, Benjamin Herrenschmidt 
>  wrote:
> 
> On Tue, 2015-08-25 at 06:47 -0700, Andrew Fish wrote:
> 
>>> Sadly it doesn't seem like the UEFI spec caters well to that non
>>> -1:1
>>> mapping (which is quite common outside of the x86 world), and
>>> doesn't
>>> really sat what GetBarAttributes() is supposed to return. However,
>>> practically, for GOP drivers (and possibly others) to work, we need
>>> to
>>> return a CPU physical address, so we need to *translate* the BAR
>>> value.
>>> 
>> I think you have it backwards. The UEFI spec abstracts PCI properly,
>> and drivers are implemented incorrectly, but they work due to x86 PC
>> assumptions. 
>> 
>> GetBarAttributes() returns the BAR. 
> 
> Well, the spec doesn't spell it out precisely but …
> 

The BAR is a PCI address by definition. 

>> That is the correct thing to do. If that Bar was accessed via>
>> EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI_P
>> CI_IO_PROTOCOL.CopyMem()  then it is being accessed relative to the
>> BAR and everything should work. Casting a pointer to the value
>> returned in GetBarAttributes() is not guaranteed to work. 
> 
> Except that a lot drivers do it :-(
> 

Then we should get these drivers fixed. 

> For example anything using OptionRomPkg/Library/BitBltLib does it.
> 
> So the GOP drivers in-tree in EDK2 do it, I can ask nVidia what their
> EFI driver does but I woudn't be surprised if it's everywhere.
> 

Well the GOP returns a FrameBuffer, so that address needs to be adjusted. So 
that seems to be the bug in the spec. I’ll bring this up to the UEFI Forum. 

But I think for the non FrameBuffer case it would be better to fix the drivers. 

Thanks,

Andrew Fish

> UfsPciHcDxe is another one in the tree.
> 
> Arguably it's the only way to have usable performance out of a GOP...
> So in any case, it doesn't change the situation. A spec not follow by
> drivers is worthless, only what the drivers do actually matter and thus
> I need to provide something that will make them work (famous Linus
> quote more/less :-)
> 
> One could argue that the BAR can be read just fine by a driver using a
> PCI configuration space read, and thus GetBarAttribute() is the perfect
> place to return the CPU address. This would be a nop on platforms using
> a 1:1 mapping and make things "just work" on others. That would require
> a simple clarification in the next spec version.
> 
>> There have  also been lots of bugs in regards to DMA. Drivers don’t
>> use Map()/Unmap()/AllocateBuffer() correctly, but since x86 makes DMA
>> cache coherent it all seems to work. When we did the 1st port to ARM
>> even the edk2 USB stack had issues not doing DMA correctly (On ARM
>> you end up doing DMA into non-cached buffers, so you need to follow
>> the rules).
> 
> Well, on some ARMs at least but yes. I see.
> 
>> Adding hacks to work around broken drivers is just going to cause
>> chaos. 
> 
> While I generally agree, I think in this specific case, it does make
> sense to redefine the semantics of GetBarAttributes(). That or provided
> a new optional protocol to allow direct mapping of the BAR into the CPU
> space.
> 
> Something tells me that otherwise a bunch of drivers are essentially
> going to be unfixable (/me eyes GPUs ...)
> 
>> You are going to need to work with the driver writers to get their
>> driver fixed.
>> 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__tianocore.sourceforge.net_wiki_UEFI-5FDriver-5FWriter-27s-5FGuide&d=BQIDaQ&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw&m=hlb6m5Pc6p6DlF5msA_xOLq7RbOHDol2huBe2-9uvT8&s=GZPYMjLcMvwAh5svYi8l4whKOKof3r3JVWIZUXsBB4g&e=
>>  
>> The PCI I/O Protocol provides services that allow a PCI driver to
>> easily access the resources of the PCI controllers it is currently
>> managing. These services hide platform- specific implementation
>> details and prevent a PCI driver from inadvertently accessing
>> resources of the motherboard or other PCI controllers. The PCI I/O
>> Protocol has also been designed to simplify the implementation of PCI
>> drivers. For example, a PCI driver should never read the BARs in the
>> PCI configuration header. Instead, the PCI driver passes in a
>> BarIndex and Offset into the PCI I/O Protocol services. The PCI bus
>> driver is responsible for managing the PCI controller’s BARs.
> 
> Yes, I understand that, but in practice I very much doubt those drivers
> will be fixed and the end user is going to blame the platform instead.
> 
> Ben.
> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> Cheers,
>>> Ben.
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_m
>>> ailman_listinfo_edk2
>>> -2Ddevel&d=BQICAg&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw
>>> &m=QE9d1rL3ijH-gexHgoNe54QWnW
>>> -gCrncwCMR3WJMIDs&s=MyGORn2OZERedpoAMdvuxIcbai_x3TEDR9WZqoxJmJg&e= 

___

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

On Tue, 2015-08-25 at 06:47 -0700, Andrew Fish wrote:

> > Sadly it doesn't seem like the UEFI spec caters well to that non
> > -1:1
> > mapping (which is quite common outside of the x86 world), and
> > doesn't
> > really sat what GetBarAttributes() is supposed to return. However,
> > practically, for GOP drivers (and possibly others) to work, we need
> > to
> > return a CPU physical address, so we need to *translate* the BAR
> > value.
> > 
> I think you have it backwards. The UEFI spec abstracts PCI properly,
> and drivers are implemented incorrectly, but they work due to x86 PC
> assumptions. 
> 
> GetBarAttributes() returns the BAR. 

Well, the spec doesn't spell it out precisely but ...

> That is the correct thing to do. If that Bar was accessed via>
> EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI_P
> CI_IO_PROTOCOL.CopyMem()  then it is being accessed relative to the
> BAR and everything should work. Casting a pointer to the value
> returned in GetBarAttributes() is not guaranteed to work. 

Except that a lot drivers do it :-(

For example anything using OptionRomPkg/Library/BitBltLib does it.

So the GOP drivers in-tree in EDK2 do it, I can ask nVidia what their
EFI driver does but I woudn't be surprised if it's everywhere.

UfsPciHcDxe is another one in the tree.

Arguably it's the only way to have usable performance out of a GOP...
So in any case, it doesn't change the situation. A spec not follow by
drivers is worthless, only what the drivers do actually matter and thus
I need to provide something that will make them work (famous Linus
quote more/less :-)

One could argue that the BAR can be read just fine by a driver using a
PCI configuration space read, and thus GetBarAttribute() is the perfect
place to return the CPU address. This would be a nop on platforms using
a 1:1 mapping and make things "just work" on others. That would require
a simple clarification in the next spec version.

> There have  also been lots of bugs in regards to DMA. Drivers don’t
> use Map()/Unmap()/AllocateBuffer() correctly, but since x86 makes DMA
> cache coherent it all seems to work. When we did the 1st port to ARM
> even the edk2 USB stack had issues not doing DMA correctly (On ARM
> you end up doing DMA into non-cached buffers, so you need to follow
> the rules).

Well, on some ARMs at least but yes. I see.

> Adding hacks to work around broken drivers is just going to cause
> chaos. 

While I generally agree, I think in this specific case, it does make
sense to redefine the semantics of GetBarAttributes(). That or provided
a new optional protocol to allow direct mapping of the BAR into the CPU
space.

Something tells me that otherwise a bunch of drivers are essentially
going to be unfixable (/me eyes GPUs ...)

> You are going to need to work with the driver writers to get their
> driver fixed.
> 
> http://tianocore.sourceforge.net/wiki/UEFI_Driver_Writer's_Guide
> The PCI I/O Protocol provides services that allow a PCI driver to
> easily access the resources of the PCI controllers it is currently
> managing. These services hide platform- specific implementation
> details and prevent a PCI driver from inadvertently accessing
> resources of the motherboard or other PCI controllers. The PCI I/O
> Protocol has also been designed to simplify the implementation of PCI
> drivers. For example, a PCI driver should never read the BARs in the
> PCI configuration header. Instead, the PCI driver passes in a
> BarIndex and Offset into the PCI I/O Protocol services. The PCI bus
> driver is responsible for managing the PCI controller’s BARs.

Yes, I understand that, but in practice I very much doubt those drivers
will be fixed and the end user is going to blame the platform instead.

Ben.
 
> Thanks,
> 
> Andrew Fish
> 
> > Cheers,
> > Ben.
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_m
> > ailman_listinfo_edk2
> > -2Ddevel&d=BQICAg&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw
> > &m=QE9d1rL3ijH-gexHgoNe54QWnW
> > -gCrncwCMR3WJMIDs&s=MyGORn2OZERedpoAMdvuxIcbai_x3TEDR9WZqoxJmJg&e= 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Measured boot in OVMF with QEMU TPM passthrough?

[David Van Arnem]

On 08/22/2015 06:41 AM, Yao, Jiewen wrote:

Hi
Some clarification:
1) TPM is to support TCG trusted boot, it is not related to UEFI secure boot. 
So enabling UEFI secure boot does not mean TCG trusted boot is enabled.
Ok, thank you.  I knew UEFI secure boot was not the same as TCG trusted 
boot, but I wasn't sure how related they were.  My focus is TCG trusted 
boot.

2) I think you need add both TcgPei and TcgDxe to record measured boot log, 
because TPM device is started from TcgPei. Just adding TcgDxe may still cause 
error.
I am using a TPM1.2, so I have added the TcgPei, TcgDxe, and related 
modules to OVMF (in both the platform DSC and FDF files), and I modeled 
my changes after the Vlv2TbltDevicePkg files.  I spoke incorrectly in 
one of my previous emails; I see PhysicalPresencePei and TcgPei in both 
Ovmf.map and PEIFV.Fv.map after OVMF build, so they should be present in 
the OVMF image, but they are not being loaded according to the OVMF 
debug output.  The DXE TCG modules *are* being loaded, but are throwing 
an error presumably because the necessary setup was not performed during 
PEI.  I thought that the SEC or PEI core would kind of "automatically" 
find and load them, so I have not been able to determine why they are 
not loading.  Do you have any thoughts on this?

3) TcgSmm is to support TPM PP and MOR, which is NOT related to measured boot 
record.

Ok, thank you again.


BTW: Would you please let me know your TPM passthru is for TPM1.2 or TPM2.0?
For later, you need include Tcg2Pei/Tcg2Dxe, instead of TcgPei/TcgDxe.

Let me know if you have more question.

Thank you
Yao Jiewen

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of David 
Van Arnem
Sent: Saturday, August 22, 2015 12:44 AM
To:edk2-devel@lists.01.org
Subject: Re: [edk2] Measured boot in OVMF with QEMU TPM passthrough?

Hello again,

I've done some analysis and here's where I'm at with measured boot with OVMF in 
a QEMU guest:

I've verified that most of the relevant modules that need to be added according 
to the instructions 
athttp://tianocore.sourceforge.net/wiki/How_to_Enable_Security#HOW_TO_ENABLE_TCG_TPM
are being placed in the OVMF image by looking at Ovmf.map.  They are also 
loaded when OVMF boots, which I've verified from the OVMF debug output.  I 
believe the only one I've added to OvmfX64.dsc/fdf that does not appear in the 
map or debug output is TcgSmm, which handles the ACPI methods for TCG.  I think 
this is a problem: I do see a TCPA table when I dump the ACPI tables in my 
guest, and it has an address to the start of the Event Log, but I still don't 
have any measurements recorded in 
/sys/kernel/security/tpm0/ascii_bios_measurements.  And, the PCR values from 
/sys/class/tpm/tpm0/device/pcrs don't change whether I have OVMF Secure Boot 
enabled or not (I think it's supposed to extend PCR[07]?).

So, my question about whether or not measured boot with event logging should be 
possible in OVMF with a QEMU VM still stands. Additonally, I'm curious if the 
way I've added components to the OvmfX64.dsc file is correct.  For example, 
here's how I added the TcgDxe component:

SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {
  
TpmCommLib|SecurityPkg/Library/TpmCommLib/TpmCommLib.inf
}

I added the LibraryClasses subsection after the build tool complained about an 
instance of that class not being found, and did the same for the other modules 
that had the same issue.  I see other LibraryClasses in TcgDxe.inf that don't 
require an instance in the subsection, why is that?

Finally, where in the code should I do steps 1 and 2 (clear memory, process 
request) from the instructions linked above?

Any guidance would be appreciated.

Thanks,
David

On 08/19/2015 05:17 PM, David Van Arnem wrote:

Hello,

Should it be possible to perform measured boot in OVMF to measure a
QEMU guest (extend and log PCRs) using a TPM passed-through from the
host?

I have a host machine with a TPM (v1.2), and a QEMU Linux guest
booting using an OVMF image with the modifications suggested in the
following link (modifications were done to OvmfX64.dsc):
http://tianocore.sourceforge.net/wiki/How_to_Enable_Security  (section
"How To Enable TCG TPM").  I've enabled QEMU TPM passthrough from the
host to the guest, and I'm able to query the TPM in the guest using
commands like tpm_version, tpm_getpubek, etc.  However, there are no
measurements recorded in
/sys/kernel/security/tpm0/ascii_bios_measurements.  Additionally,
though I can view the PCR list from /sys/class/tpm/tpm0/device/pcrs,
the list contains the same values that I saw when looking at the same
file on my host before enabling TPM passthrough.  So, it appears
measurement is not happening on the guest, and I wanted to check here
to see if that's a limitation of OVMF/TPM passthrough, or if I just
did something incorrectly when modifying the OVMF package.

Thanks,
David

___
edk2-devel mailing list
ed

[edk2] [patch] SecurityPkg: Fix one returned code issue in P7Verify Protocol

[Qin Long]

VerifyBuffer() in PKCS7 Verify Protocol should return EFI_UNSUPPORTED
when the embedded content is found in SignedData but InData is not NULL.
This patch is to comply with the spec definition.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 13c9138..07fdf55 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -833,6 +833,13 @@ VerifyBuffer (
 return EFI_UNSUPPORTED;
   }
   if (AttachedData != NULL) {
+if (InData != NULL) {
+  //
+  // The embedded content is found in SignedData but InData is not NULL
+  //
+  Status = EFI_UNSUPPORTED;
+  goto _Exit;
+}
 //
 // PKCS7-formatted signedData with attached content; Use the embedded
 // content for verification
-- 
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] MdeModulePkg/Xhci: Remove TDs from transfer ring when timeout happens

[Carsey, Jaben]

What about other error return values in *TransferResult?  Both places you check 
for a specific set of errors, but the set is not exhaustive and if a different 
error occurs, will the caller know?

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Tian, Feng
> Sent: Monday, August 24, 2015 8:17 PM
> To: Anbazhagan, Baraneedharan ; Zeng, Star
> 
> Cc: edk2-devel@lists.01.org; Tian, Feng 
> Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer
> ring when timeout happens
> 
> Thanks for your comments, Baranee. I updated the patch as below. Please
> review again.
> 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |  75 +++---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 193
> +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  80 +++
>  MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c   |  51 +--
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 235
> +++
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h |  81 ++-
>  6 files changed, 618 insertions(+), 97 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 390ca0a..39c28ab 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -905,17 +905,28 @@ XhcControlTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcControlTransfer:
> XhcDequeueTrbFromEndpoint failed\n"));
>  }
> -Status = EFI_DEVICE_ERROR;
>  goto FREE_URB;
>} else {
> -goto FREE_URB;
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcControlTransfer:
> XhcRecoverHaltedEndpoint failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
> +  goto FREE_URB;
> +} else {
> +  goto FREE_URB;
> +}
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1241,14 +1252,24 @@ XhcBulkTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint
> failed\n"));
> +}
> +  } else {
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
>  }
> -Status = EFI_DEVICE_ERROR;
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1538,14 +1559,24 @@ XhcSyncInterruptTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer:
> XhcRecoverHaltedEndpoint failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcSyncInterruptTransfer:
> XhcDequeueTrbFromEndpoint failed\n"));
> +}
> +  } else {
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (Recover

Re: [edk2] [patch] SecurityPkg: Fix one returned status code issue in P7Verify Protocol.

[Long, Qin]

Exactly. Thanks for catching this. 
Will send the new patch later.


Best Regards & Thanks,
LONG, Qin

> -Original Message-
> From: Zhang, Chao B
> Sent: Tuesday, August 25, 2015 8:17 PM
> To: Long, Qin; edk2-devel@lists.01.org
> Subject: RE: [patch] SecurityPkg: Fix one returned status code issue in
> P7Verify Protocol.
> 
> Qin:
>   In this case, Should AttachedData be freed before return UNSUPPORTED
> 
> 
> 
> 
> 
> Thanks & Best regards
> Chao Zhang
> 
> -Original Message-
> From: Long, Qin
> Sent: Tuesday, August 25, 2015 4:53 PM
> To: Zhang, Chao B; edk2-devel@lists.01.org
> Subject: [patch] SecurityPkg: Fix one returned status code issue in P7Verify
> Protocol.
> 
> VerifyBuffer() in PKCS7 Verify Protocol should return EFI_UNSUPPORTED
> when signed data is embedded in SignedData but InData is not NULL.
> This patch is to comply with the spec definition.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long 
> ---
>  SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> index 13c9138..8bf1f1f 100644
> --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> @@ -833,6 +833,12 @@ VerifyBuffer (
>  return EFI_UNSUPPORTED;
>}
>if (AttachedData != NULL) {
> +if (InData != NULL) {
> +  //
> +  // Signed data embedded in SignedData but InData is not NULL
> +  //
> +  return EFI_UNSUPPORTED;
> +}
>  //
>  // PKCS7-formatted signedData with attached content; Use the
> embedded
>  // content for verification
> --
> 1.9.5.msysgit.1

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

[edk2] [PATCH] ArmPkg: Add isb when setting SCR.

[Evan Lloyd]

Some updates to SCR can cause a problem which manifests as an undefined opcode 
exception.
This may be when a speculative secure instruction fetch happens after the NS 
bit is set.
An isb is required to make the register change take effect fully.

Contributed-under: Tianocore Contribution Agreement 1.0
Signed-off-by: Evan Lloyd 
Reviewed-by: Sami Mujawar 
---
diff --git a/ArmPkg/Library/ArmLib/Common/AArch64/ArmLibSupport.S 
b/ArmPkg/Library/ArmLib/Common/AArch64/ArmLibSupport.S
index 28db98b..50faef4 100644
--- a/ArmPkg/Library/ArmLib/Common/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Common/AArch64/ArmLibSupport.S
@@ -184,6 +184,7 @@ ASM_PFX(ArmWriteCptr):

 ASM_PFX(ArmWriteScr):
   msr scr_el3, x0// Secure configuration register EL3
+  isb
   ret

 ASM_PFX(ArmWriteMVBar):
diff --git a/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.S 
b/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.S
index 43842f3..ffeaed3 100644
--- a/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.S
@@ -153,6 +153,7 @@ ASM_PFX(ArmReadScr):

 ASM_PFX(ArmWriteScr):
   mcr p15, 0, r0, c1, c1, 0
+  isb
   bx  lr

 ASM_PFX(ArmReadHVBar):
diff --git a/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.asm 
b/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.asm
index 8864c16..15bfb6e 100644
--- a/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.asm
+++ b/ArmPkg/Library/ArmLib/Common/Arm/ArmLibSupport.asm
@@ -153,6 +153,7 @@ ArmReadScr

 ArmWriteScr
   mcr p15, 0, r0, c1, c1, 0
+  isb
   bx  lr

 ArmReadHVBar



-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered 
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782

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

Re: [edk2] [minnowboard] How/where to insert Realtek HDA Codec Verb Table?

[Gerard Bucas]

Forgot to insert the code snipped from AzaliaVerbTable.h that I referred to
below. Now added...


-Original Message-
From: Gerard Bucas [mailto:gerar...@tekmagic.net] 
Sent: Tuesday, August 25, 2015 10:47 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [minnowboard] How/where to insert Realtek HDA Codec Verb
Table?

Hi All,

We are using the Minnowboard MAX 0.82 firmware as the basis for the firmware
for a new Intel Baytrail (E3845) based product we have developed. 

It uses the Realtek ALC262 HDA Audio Codec & we are having some problems
activating the analog audio out headphone.

Realtek has sent us the "HDA Codec Subsystem ID verb-table" to insert in our
BIOS/firmware. However, it is not clear how/where this needs to be inserted.
Does anypne have an example of how/where such a verb table (see attached)
needs to be inserted in the firmware code?

I noticed the following line in the code snipped below in the
"AzaliaVerbTable.h" file, which has this line in the ALC262 entry:

0 // Pointer to verb table data, need to be inited in the
code. 

{
//
// VerbTable: (RealTek ALC262)
//  Revision ID = 0xFF, support all steps
//  Codec Verb Table For AZALIA
//  Codec Address: CAd value (0/1/2)
//  Codec Vendor:  0x10EC0262
//
{
  0x10EC0262, // Vendor ID/Device ID
  0x, // SubSystem ID
  0xFF,   // Revision ID
  0x01,   // Front panel support (1=yes, 2=no)
  0x000B, // Number of Rear Jacks = 11
  0x0002  // Number of Front Jacks = 2
},
0 // Pointer to verb table data, need to be inited in
the code.
  }

Not clear to us what the value of this pointer should be and where the
attached Realtek verb table should be inserted?

Any examples or help would be greatly appreciated!

Best Regards

Gerard Bucas
;===
;
;   Realtek Semiconductor Corp.
;
;===

;Realtek High Definition Audio Configuration - Version : 5.0.3.0
;Realtek HD Audio Codec : ALC262-VC
;PCI PnP ID : PCI\VEN_8086&DEV_2668&SUBSYS_
;HDA Codec PnP ID : HDAUDIO\FUNC_01&VEN_10EC&DEV_0262&SUBSYS_
;The number of verb command block : 15

;NID 0x12 : 0x4000
;NID 0x14 : 0x41F0
;NID 0x15 : 0x03214010
;NID 0x16 : 0x41F0
;NID 0x18 : 0x03A19120
;NID 0x19 : 0x41F0
;NID 0x1A : 0x41F0
;NID 0x1B : 0x41F0
;NID 0x1C : 0x41F0
;NID 0x1D : 0x40530605
;NID 0x1E : 0x41F0
;NID 0x1F : 0x41F0


;= HDA Codec Subsystem ID Verb-table =
;HDA Codec Subsystem ID  : 0x
dd 00172000h
dd 00172100h
dd 00172200h
dd 00172300h


;= Pin Widget Verb-table =
;Widget node 0x01 :
dd 0017FF00h
dd 0017FF00h
dd 0017FF00h
dd 0017FF00h
;Pin widget 0x12 - DMIC
dd 01271C00h
dd 01271D00h
dd 01271E00h
dd 01271F40h
;Pin widget 0x14 - LINE-OUT (Port-D)
dd 01471CF0h
dd 01471D11h
dd 01471E11h
dd 01471F41h
;Pin widget 0x15 - HP-OUT (Port-A)
dd 01571C10h
dd 01571D40h
dd 01571E21h
dd 01571F03h
;Pin widget 0x16 - MONO-OUT
dd 01671CF0h
dd 01671D11h
dd 01671E11h
dd 01671F41h
;Pin widget 0x18 - MIC1 (Port-B)
dd 01871C20h
dd 01871D91h
dd 01871EA1h
dd 01871F03h
;Pin widget 0x19 - MIC2 (Port-F)
dd 01971CF0h
dd 01971D11h
dd 01971E11h
dd 01971F41h
;Pin widget 0x1A - LINE1 (Port-C)
dd 01A71CF0h
dd 01A71D11h
dd 01A71E11h
dd 01A71F41h
;Pin widget 0x1B - LINE2 (Port-E)
dd 01B71CF0h
dd 01B71D11h
dd 01B71E11h
dd 01B71F41h
;Pin widget 0x1C - CD-IN
dd 01C71CF0h
dd 01C71D11h
dd 01C71E11h
dd 01C71F41h
;Pin widget 0x1D - BEEP-IN
dd 01D71C05h
dd 01D71D06h
dd 01D71E53h
dd 01D71F40h
;Pin widget 0x1E - S/PDIF-OUT
dd 01E71CF0h
dd 01E71D11h
dd 01E71E11h
dd 01E71F41h
;Pin widget 0x1F - S/PDIF-IN
dd 01F71CF0h
dd 01F71D11h
dd 01F71E11h
dd 01F71F41h
;Widget node 0x20 :
dd 02050007h
dd 02040040h
dd 02050007h
dd 02040040h
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ShellPkg: use UEFI_APPLICATION module type for Shell components

[Ard Biesheuvel]

On 25 August 2015 at 16:31, Carsey, Jaben  wrote:
> Reviewed-by: Jaben Carsey 
>

Thanks

Committed as SVN r18310

>>-Original Message-
>>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>Sent: Tuesday, August 25, 2015 6:04 AM
>>To: edk2-devel@lists.01.org; Carsey, Jaben; Qiu, Shumin
>>Cc: Ard Biesheuvel
>>Subject: [PATCH] ShellPkg: use UEFI_APPLICATION module type for Shell 
>>components
>>Importance: High
>>
>>Some of the libraries under ShellPkg/Library/ are only intended to be
>>loaded into the Shell by means of a NULL LibraryClass resolution, and
>>serve no other purpose. Since the Shell itself is a UEFI_APPLICATION, it
>>makes sense to set the module type of those libraries to UEFI_APPLICATION
>>as well.
>>
>>This allows us to use different compiler flags for the Shell application
>>itself but also for the majority of its constituent parts that are built
>>separately via these libraries.
>>
>>Contributed-under: TianoCore Contribution Agreement 1.0
>>Signed-off-by: Ard Biesheuvel 
>>---
>> .../Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf   | 2 
>> +-
>> .../Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf | 2 
>> +-
>> .../Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf   | 2 
>> +-
>> .../Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf   | 2 
>> +-
>> .../Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf   | 2 
>> +-
>> .../UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf   | 2 
>> +-
>> ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf| 2 
>> +-
>> 7 files changed, 7 insertions(+), 7 deletions(-)
>>
>>diff --git 
>>a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>index d8451c397652..55371eb37194 100644
>>--- 
>>a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>+++ 
>>b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>@@ -16,7 +16,7 @@ [Defines]
>>   INF_VERSION= 0x00010006
>>   BASE_NAME  = UefiShellDebug1CommandsLib
>>   FILE_GUID  = 90330D51-A99B-4cc8-A2EB-AE22542A3F45
>>-  MODULE_TYPE= UEFI_DRIVER
>>+  MODULE_TYPE= UEFI_APPLICATION
>>   VERSION_STRING = 1.0
>>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>>   CONSTRUCTOR= UefiShellDebug1CommandsLibConstructor
>>diff --git 
>>a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>index a813b3b614f9..f2a2e4c581b0 100644
>>--- 
>>a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>+++ 
>>b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>@@ -16,7 +16,7 @@ [Defines]
>>   INF_VERSION= 0x00010006
>>   BASE_NAME  = UefiShellDriver1CommandsLib
>>   FILE_GUID  = 313D3674-3ED4-48fd-BF97-7DB35D4190D1
>>-  MODULE_TYPE= UEFI_DRIVER
>>+  MODULE_TYPE= UEFI_APPLICATION
>>   VERSION_STRING = 1.0
>>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>>   CONSTRUCTOR= UefiShellDriver1CommandsLibConstructor
>>diff --git 
>>a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>>b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>>index d21ad41ca9f3..4d3713b46092 100644
>>--- 
>>a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>>+++ 
>>b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>>@@ -16,7 +16,7 @@ [Defines]
>>   INF_VERSION= 0x00010006
>>   BASE_NAME  = UefiShellLevel1CommandsLib
>>   FILE_GUID  = 50cb6037-1102-47af-b2dd-9944b6eb1abe
>>-  MODULE_TYPE= UEFI_DRIVER
>>+  MODULE_TYPE= UEFI_APPLICATION
>>   VERSION_STRING = 1.0
>>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>>   CONSTRUCTOR= ShellLevel1CommandsLibConstructor
>>diff --git 
>>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>>index 324d210d31ca..76540fb3a364 100644
>>--- 
>>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>>+++ 
>>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>>@@ -17,7 +17,7 @@ [Defines]
>>   INF_VERSION= 0x00010006
>>   BASE_NAME  = UefiShellLevel2CommandsLib
>>   FILE_GUID  = CBF3931C-A2DF-40e5-B77E-CCA9555E9755
>>-  MOD

[edk2] [minnowboard] How/where to insert Realtek HDA Codec Verb Table?

[Gerard Bucas]

Hi All,

We are using the Minnowboard MAX 0.82 firmware as the basis for the firmware
for a new Intel Baytrail (E3845) based product we have developed. 

It uses the Realtek ALC262 HDA Audio Codec & we are having some problems
activating the analog audio out headphone.

Realtek has sent us the "HDA Codec Subsystem ID verb-table" to insert in our
BIOS/firmware. However, it is not clear how/where this needs to be inserted.
Does anypne have an example of how/where such a verb table (see attached)
needs to be inserted in the firmware code?

I noticed the following code in the "AzaliaVerbTable.h" file, which has this
line in the ALC262 entry:

0 // Pointer to verb table data, need to be inited in the
code. 

Not clear to us what the value of this pointer should be and where the
attached Realtek verb table should be inserted?

Any examples or help would be greatly appreciated!

Best Regards

Gerard Bucas
;===
;
;   Realtek Semiconductor Corp.
;
;===

;Realtek High Definition Audio Configuration - Version : 5.0.3.0
;Realtek HD Audio Codec : ALC262-VC
;PCI PnP ID : PCI\VEN_8086&DEV_2668&SUBSYS_
;HDA Codec PnP ID : HDAUDIO\FUNC_01&VEN_10EC&DEV_0262&SUBSYS_
;The number of verb command block : 15

;NID 0x12 : 0x4000
;NID 0x14 : 0x41F0
;NID 0x15 : 0x03214010
;NID 0x16 : 0x41F0
;NID 0x18 : 0x03A19120
;NID 0x19 : 0x41F0
;NID 0x1A : 0x41F0
;NID 0x1B : 0x41F0
;NID 0x1C : 0x41F0
;NID 0x1D : 0x40530605
;NID 0x1E : 0x41F0
;NID 0x1F : 0x41F0


;= HDA Codec Subsystem ID Verb-table =
;HDA Codec Subsystem ID  : 0x
dd 00172000h
dd 00172100h
dd 00172200h
dd 00172300h


;= Pin Widget Verb-table =
;Widget node 0x01 :
dd 0017FF00h
dd 0017FF00h
dd 0017FF00h
dd 0017FF00h
;Pin widget 0x12 - DMIC
dd 01271C00h
dd 01271D00h
dd 01271E00h
dd 01271F40h
;Pin widget 0x14 - LINE-OUT (Port-D)
dd 01471CF0h
dd 01471D11h
dd 01471E11h
dd 01471F41h
;Pin widget 0x15 - HP-OUT (Port-A)
dd 01571C10h
dd 01571D40h
dd 01571E21h
dd 01571F03h
;Pin widget 0x16 - MONO-OUT
dd 01671CF0h
dd 01671D11h
dd 01671E11h
dd 01671F41h
;Pin widget 0x18 - MIC1 (Port-B)
dd 01871C20h
dd 01871D91h
dd 01871EA1h
dd 01871F03h
;Pin widget 0x19 - MIC2 (Port-F)
dd 01971CF0h
dd 01971D11h
dd 01971E11h
dd 01971F41h
;Pin widget 0x1A - LINE1 (Port-C)
dd 01A71CF0h
dd 01A71D11h
dd 01A71E11h
dd 01A71F41h
;Pin widget 0x1B - LINE2 (Port-E)
dd 01B71CF0h
dd 01B71D11h
dd 01B71E11h
dd 01B71F41h
;Pin widget 0x1C - CD-IN
dd 01C71CF0h
dd 01C71D11h
dd 01C71E11h
dd 01C71F41h
;Pin widget 0x1D - BEEP-IN
dd 01D71C05h
dd 01D71D06h
dd 01D71E53h
dd 01D71F40h
;Pin widget 0x1E - S/PDIF-OUT
dd 01E71CF0h
dd 01E71D11h
dd 01E71E11h
dd 01E71F41h
;Pin widget 0x1F - S/PDIF-IN
dd 01F71CF0h
dd 01F71D11h
dd 01F71E11h
dd 01F71F41h
;Widget node 0x20 :
dd 02050007h
dd 02040040h
dd 02050007h
dd 02040040h
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 2/3] MdePkg/Library/UefiFileHandleLib: Make the FileHandleGetFileName return the file name in right format.

[Carsey, Jaben]

I like the comments.  Thanks for updating them!

>-Original Message-
>From: Qiu, Shumin
>Sent: Monday, August 24, 2015 11:39 PM
>To: edk2-devel@lists.01.org
>Cc: Qiu, Shumin; Carsey, Jaben; Gao, Liming
>Subject: [PATCH v3 2/3] MdePkg/Library/UefiFileHandleLib: Make the 
>FileHandleGetFileName return the file name in right format.
>Importance: High
>
>1. If the file is not a directory remove the redundant '\' char in file name 
>string returned from FileHandleGetFileName.
>2. Update function comments.
>
>Cc: Jaben Carsey 
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Qiu Shumin 
>Reviewed-by: Jaben Carsey 
>---
> MdePkg/Include/Library/FileHandleLib.h   |  4 +++-
> MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 12 +++-
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Include/Library/FileHandleLib.h 
>b/MdePkg/Include/Library/FileHandleLib.h
>index fdbdc3a..00706f9 100644
>--- a/MdePkg/Include/Library/FileHandleLib.h
>+++ b/MdePkg/Include/Library/FileHandleLib.h
>@@ -356,7 +356,9 @@ FileHandleSetSize (
>
> /**
>   Function to get a full filename given a EFI_FILE_HANDLE somewhere lower on 
> the
>-  directory 'stack'.
>+  directory 'stack'. If the file is a directory, then append the '\' char at 
>the
>+  end of name string. If it's not a directory, then the last '\' should not be
>+  added.
>
>   @param[in] Handle Handle to the Directory or File to create 
> path to.
>   @param[out] FullFileName  Pointer to pointer to generated full file 
> name.  It
>diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c 
>b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>index a31d12b..04a2f18 100644
>--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>@@ -769,7 +769,9 @@ StrnCatGrowLeft (
>
> /**
>   Function to get a full filename given a EFI_FILE_HANDLE somewhere lower on 
> the
>-  directory 'stack'.
>+  directory 'stack'. If the file is a directory, then append the '\' char at 
>the
>+  end of name string. If it's not a directory, then the last '\' should not be
>+  added.
>
>   if Handle is NULL, return EFI_INVALID_PARAMETER
>
>@@ -856,6 +858,14 @@ FileHandleGetFileName (
> *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0);
>   }
>
>+  if (*FullFileName != NULL &&
>+  (*FullFileName)[StrLen(*FullFileName) - 1] == L'\\' &&
>+  StrLen(*FullFileName) > 1 &&
>+  FileHandleIsDirectory(Handle) == EFI_NOT_FOUND
>+ ) {
>+(*FullFileName)[StrLen(*FullFileName) - 1] = CHAR_NULL;
>+  }
>+
>   if (CurrentHandle != NULL) {
> CurrentHandle->Close (CurrentHandle);
>   }
>--
>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] ShellPkg: use UEFI_APPLICATION module type for Shell components

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

>-Original Message-
>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>Sent: Tuesday, August 25, 2015 6:04 AM
>To: edk2-devel@lists.01.org; Carsey, Jaben; Qiu, Shumin
>Cc: Ard Biesheuvel
>Subject: [PATCH] ShellPkg: use UEFI_APPLICATION module type for Shell 
>components
>Importance: High
>
>Some of the libraries under ShellPkg/Library/ are only intended to be
>loaded into the Shell by means of a NULL LibraryClass resolution, and
>serve no other purpose. Since the Shell itself is a UEFI_APPLICATION, it
>makes sense to set the module type of those libraries to UEFI_APPLICATION
>as well.
>
>This allows us to use different compiler flags for the Shell application
>itself but also for the majority of its constituent parts that are built
>separately via these libraries.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ard Biesheuvel 
>---
> .../Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf   | 2 +-
> .../Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf | 2 +-
> .../Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf   | 2 +-
> .../Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf   | 2 +-
> .../Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf   | 2 +-
> .../UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf   | 2 +-
> ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf| 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git 
>a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>index d8451c397652..55371eb37194 100644
>--- 
>a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>+++ 
>b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>@@ -16,7 +16,7 @@ [Defines]
>   INF_VERSION= 0x00010006
>   BASE_NAME  = UefiShellDebug1CommandsLib
>   FILE_GUID  = 90330D51-A99B-4cc8-A2EB-AE22542A3F45
>-  MODULE_TYPE= UEFI_DRIVER
>+  MODULE_TYPE= UEFI_APPLICATION
>   VERSION_STRING = 1.0
>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>   CONSTRUCTOR= UefiShellDebug1CommandsLibConstructor
>diff --git 
>a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>index a813b3b614f9..f2a2e4c581b0 100644
>--- 
>a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>+++ 
>b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>@@ -16,7 +16,7 @@ [Defines]
>   INF_VERSION= 0x00010006
>   BASE_NAME  = UefiShellDriver1CommandsLib
>   FILE_GUID  = 313D3674-3ED4-48fd-BF97-7DB35D4190D1
>-  MODULE_TYPE= UEFI_DRIVER
>+  MODULE_TYPE= UEFI_APPLICATION
>   VERSION_STRING = 1.0
>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>   CONSTRUCTOR= UefiShellDriver1CommandsLibConstructor
>diff --git 
>a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>index d21ad41ca9f3..4d3713b46092 100644
>--- 
>a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>+++ 
>b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>@@ -16,7 +16,7 @@ [Defines]
>   INF_VERSION= 0x00010006
>   BASE_NAME  = UefiShellLevel1CommandsLib
>   FILE_GUID  = 50cb6037-1102-47af-b2dd-9944b6eb1abe
>-  MODULE_TYPE= UEFI_DRIVER
>+  MODULE_TYPE= UEFI_APPLICATION
>   VERSION_STRING = 1.0
>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
>   CONSTRUCTOR= ShellLevel1CommandsLibConstructor
>diff --git 
>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>index 324d210d31ca..76540fb3a364 100644
>--- 
>a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>+++ 
>b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>@@ -17,7 +17,7 @@ [Defines]
>   INF_VERSION= 0x00010006
>   BASE_NAME  = UefiShellLevel2CommandsLib
>   FILE_GUID  = CBF3931C-A2DF-40e5-B77E-CCA9555E9755
>-  MODULE_TYPE= UEFI_DRIVER
>+  MODULE_TYPE= UEFI_APPLICATION
>   VERSION_STRING = 1.0
>   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
> 

Re: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer ring when timeout happens

[Zeng, Star]

On 2015/8/25 11:16, Tian, Feng wrote:

Thanks for your comments, Baranee. I updated the patch as below. Please review 
again.

---
  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |  75 +++---
  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 193 +
  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  80 +++
  MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c   |  51 +--
  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 235 +++
  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h |  81 ++-
  6 files changed, 618 insertions(+), 97 deletions(-)


Reviewed-by: Star Zeng 



diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 390ca0a..39c28ab 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -905,17 +905,28 @@ XhcControlTransfer (
*TransferResult = Urb->Result;
*DataLength = Urb->Completed;

-  if (*TransferResult == EFI_USB_NOERROR) {
-Status = EFI_SUCCESS;
-  } else if (*TransferResult == EFI_USB_ERR_STALL) {
-RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
-if (EFI_ERROR (RecoveryStatus)) {
-  DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  if (Status == EFI_TIMEOUT) {
+//
+// The transfer timed out. Abort the transfer by dequeueing of the TD.
+//
+RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
+if (EFI_ERROR(RecoveryStatus)) {
+  DEBUG((EFI_D_ERROR, "XhcControlTransfer: XhcDequeueTrbFromEndpoint 
failed\n"));
  }
-Status = EFI_DEVICE_ERROR;
  goto FREE_URB;
} else {
-goto FREE_URB;
+if (*TransferResult == EFI_USB_NOERROR) {
+  Status = EFI_SUCCESS;
+} else if (*TransferResult == EFI_USB_ERR_STALL) {
+  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+  if (EFI_ERROR (RecoveryStatus)) {
+DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  }
+  Status = EFI_DEVICE_ERROR;
+  goto FREE_URB;
+} else {
+  goto FREE_URB;
+}
}

Xhc->PciIo->Flush (Xhc->PciIo);
@@ -1241,14 +1252,24 @@ XhcBulkTransfer (
*TransferResult = Urb->Result;
*DataLength = Urb->Completed;

-  if (*TransferResult == EFI_USB_NOERROR) {
-Status = EFI_SUCCESS;
-  } else if (*TransferResult == EFI_USB_ERR_STALL) {
-RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
-if (EFI_ERROR (RecoveryStatus)) {
-  DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  if (Status == EFI_TIMEOUT) {
+//
+// The transfer timed out. Abort the transfer by dequeueing of the TD.
+//
+RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
+if (EFI_ERROR(RecoveryStatus)) {
+  DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint 
failed\n"));
+}
+  } else {
+if (*TransferResult == EFI_USB_NOERROR) {
+  Status = EFI_SUCCESS;
+} else if (*TransferResult == EFI_USB_ERR_STALL) {
+  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+  if (EFI_ERROR (RecoveryStatus)) {
+DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  }
+  Status = EFI_DEVICE_ERROR;
  }
-Status = EFI_DEVICE_ERROR;
}

Xhc->PciIo->Flush (Xhc->PciIo);
@@ -1538,14 +1559,24 @@ XhcSyncInterruptTransfer (
*TransferResult = Urb->Result;
*DataLength = Urb->Completed;

-  if (*TransferResult == EFI_USB_NOERROR) {
-Status = EFI_SUCCESS;
-  } else if (*TransferResult == EFI_USB_ERR_STALL) {
-RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
-if (EFI_ERROR (RecoveryStatus)) {
-  DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  if (Status == EFI_TIMEOUT) {
+//
+// The transfer timed out. Abort the transfer by dequeueing of the TD.
+//
+RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
+if (EFI_ERROR(RecoveryStatus)) {
+  DEBUG((EFI_D_ERROR, "XhcSyncInterruptTransfer: XhcDequeueTrbFromEndpoint 
failed\n"));
+}
+  } else {
+if (*TransferResult == EFI_USB_NOERROR) {
+  Status = EFI_SUCCESS;
+} else if (*TransferResult == EFI_USB_ERR_STALL) {
+  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+  if (EFI_ERROR (RecoveryStatus)) {
+DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: XhcRecoverHaltedEndpoint 
failed\n"));
+  }
+  Status = EFI_DEVICE_ERROR;
  }
-Status = EFI_DEVICE_ERROR;
}

Xhc->PciIo->Flush (Xhc->PciIo);
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 05cd616..c25342d 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -645,12 +645,8 @@ XhcRecoverHaltedEndpoint (
)
  {
EFI_STATUS  Status;
-  EVT_TRB_COMMAND_COMPLETION  *EvtTrb;
-  CMD_TRB_RESET_ENDPOINT  CmdTrbResetED;
-  CMD_SET_TR_DEQ_POINTER  CmdSetTRDeq;
UI

Re: [edk2] PCI and non-1:1 mapping of MMIO space

[Andrew Fish]

> On Aug 25, 2015, at 6:10 AM, Benjamin Herrenschmidt 
>  wrote:
> 
> Hi !
> 
> I have a situation where the mapping from the CPU address space to the
> PCI MMIO space isn't 1:1
> 
> To give an example, I have, let's say, a 2G MMIO window to PCI, which
> can generate PCI addresses 0x8000_..0x_ by accessing CPU
> address 0x100_8000_..0x100__.
> 
> So far, I have done the translation inside my Pci Root IO protocol. IE,
> all the resources are in term of PCI addresses, and when doing Read()
> or Write(), my implementation will apply the appropriate offset to the
> actual MMIO access.
> 
> So far it works fine for everything ... except when confronted to a
> driver that uses GetBarAttributes() from the PCI IO protocol.
> 
> This is typically the case of GOP drivers which use this in order to
> get the framebuffer address.
> 
> In that case, the PciBusDxe returns a resource containing the PCI
> address, which isn't usable as a CPU address directly and thus things
> break horribly.
> 
> Sadly it doesn't seem like the UEFI spec caters well to that non-1:1
> mapping (which is quite common outside of the x86 world), and doesn't
> really sat what GetBarAttributes() is supposed to return. However,
> practically, for GOP drivers (and possibly others) to work, we need to
> return a CPU physical address, so we need to *translate* the BAR value.
> 

I think you have it backwards. The UEFI spec abstracts PCI properly, and 
drivers are implemented incorrectly, but they work due to x86 PC assumptions. 

GetBarAttributes() returns the BAR. That is the correct thing to do. If that 
Bar was accessed via 
EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI_PCI_IO_PROTOCOL.CopyMem()
  then it is being accessed relative to the BAR and everything should work. 
Casting a pointer to the value returned in GetBarAttributes() is not guaranteed 
to work. 

There have  also been lots of bugs in regards to DMA. Drivers don’t use 
Map()/Unmap()/AllocateBuffer() correctly, but since x86 makes DMA cache 
coherent it all seems to work. When we did the 1st port to ARM even the edk2 
USB stack had issues not doing DMA correctly (On ARM you end up doing DMA into 
non-cached buffers, so you need to follow the rules).

> Unfortunately, I haven't found any way for my root bridge to intercept
> this and perform the translation. The Root IO protocol is standard so
> cannot be easily modified to handle this. The resource allocation
> protocol is not, so could be extended, but by the time we get those
> GetBarAttributes() call, there is no remaining reference to that
> protocol around that I can find (but I may have missed something).
> Ideally, if it stuck around, it would be handy to have a hook in there
> to "adjust" the resource returned by GetBarAttribtues().
> 
> Another way would be to actually use the AddressTranslationOffset field
> of the ACPI resource descriptor. IE, the PciBusDxe resource allocation
> code could make use of that field to maintain an offset associated with
> the BAR value internally in the PCI_IO_DEVICE to be able to perform
> such adjustments.
> 
> Unfortunately, it appears that the current implementation already
> hijacks that field for user uses.
> 
> Note that just playing with the MMU to punch a hole at 8000_ in the
> CPU address space and route that to PCI won't fly very high unless I'm
> ready to limit the 32-bit space drastically. The machines I'm playing
> with can have 3 or 4 PCI Host Bridge per chip, each of them having a
> completely separate aperture.
> 
> Any recommendation on what is the best approach here ? I'd rather not
> completely duplicate & hack the PciBusDxe, so the less invasive change
> would be the best here…
> 

Adding hacks to work around broken drivers is just going to cause chaos. 

You are going to need to work with the driver writers to get their driver fixed.

http://tianocore.sourceforge.net/wiki/UEFI_Driver_Writer's_Guide 

The PCI I/O Protocol provides services that allow a PCI driver to easily access 
the resources of the PCI controllers it is currently managing. These services 
hide platform- specific implementation details and prevent a PCI driver from 
inadvertently accessing resources of the motherboard or other PCI controllers. 
The PCI I/O Protocol has also been designed to simplify the implementation of 
PCI drivers. For example, a PCI driver should never read the BARs in the PCI 
configuration header. Instead, the PCI driver passes in a BarIndex and Offset 
into the PCI I/O Protocol services. The PCI bus driver is responsible for 
managing the PCI controller’s BARs.

Thanks,

Andrew Fish

> Cheers,
> Ben.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_mailman_listinfo_edk2-2Ddevel&d=BQICAg&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw&m=QE9d1rL3ij

[edk2] [PATCH] ArmVirtPkg: revert unintended change to ArmVirt.dsc.inc

[Ard Biesheuvel]

The recent changes to ArmVExpress-FVP-AArch64 in SVN r18309
contained a hunk to ArmVirt.dsc.inc which was included in
error. So remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 5 -
 1 file changed, 5 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 4b4e70b7ce91..8372c5813cb3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -239,11 +239,6 @@ [LibraryClasses.ARM]
   # Add support for GCC stack protector
   NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  # OpenSSL requires floating point for the RNG code
-  NULL|StdLib/LibC/Softfloat/Softfloat.inf
-!endif
-
 [LibraryClasses.AARCH64]
   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
 
-- 
1.9.1

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

[edk2] PCI and non-1:1 mapping of MMIO space

[Benjamin Herrenschmidt]

Hi !

I have a situation where the mapping from the CPU address space to the
PCI MMIO space isn't 1:1

To give an example, I have, let's say, a 2G MMIO window to PCI, which
can generate PCI addresses 0x8000_..0x_ by accessing CPU
address 0x100_8000_..0x100__.

So far, I have done the translation inside my Pci Root IO protocol. IE,
all the resources are in term of PCI addresses, and when doing Read()
or Write(), my implementation will apply the appropriate offset to the
actual MMIO access.

So far it works fine for everything ... except when confronted to a
driver that uses GetBarAttributes() from the PCI IO protocol.

This is typically the case of GOP drivers which use this in order to
get the framebuffer address.

In that case, the PciBusDxe returns a resource containing the PCI
address, which isn't usable as a CPU address directly and thus things
break horribly.

Sadly it doesn't seem like the UEFI spec caters well to that non-1:1
mapping (which is quite common outside of the x86 world), and doesn't
really sat what GetBarAttributes() is supposed to return. However,
practically, for GOP drivers (and possibly others) to work, we need to
return a CPU physical address, so we need to *translate* the BAR value.

Unfortunately, I haven't found any way for my root bridge to intercept
this and perform the translation. The Root IO protocol is standard so
cannot be easily modified to handle this. The resource allocation
protocol is not, so could be extended, but by the time we get those
GetBarAttributes() call, there is no remaining reference to that
protocol around that I can find (but I may have missed something).
Ideally, if it stuck around, it would be handy to have a hook in there
to "adjust" the resource returned by GetBarAttribtues().

Another way would be to actually use the AddressTranslationOffset field
of the ACPI resource descriptor. IE, the PciBusDxe resource allocation
code could make use of that field to maintain an offset associated with
the BAR value internally in the PCI_IO_DEVICE to be able to perform
such adjustments.

Unfortunately, it appears that the current implementation already
hijacks that field for user uses.

Note that just playing with the MMU to punch a hole at 8000_ in the
CPU address space and route that to PCI won't fly very high unless I'm
ready to limit the 32-bit space drastically. The machines I'm playing
with can have 3 or 4 PCI Host Bridge per chip, each of them having a
completely separate aperture.

Any recommendation on what is the best approach here ? I'd rather not
completely duplicate & hack the PciBusDxe, so the less invasive change
would be the best here...

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

[edk2] [PATCH] ShellPkg: use UEFI_APPLICATION module type for Shell components

[Ard Biesheuvel]

Some of the libraries under ShellPkg/Library/ are only intended to be
loaded into the Shell by means of a NULL LibraryClass resolution, and
serve no other purpose. Since the Shell itself is a UEFI_APPLICATION, it
makes sense to set the module type of those libraries to UEFI_APPLICATION
as well.

This allows us to use different compiler flags for the Shell application
itself but also for the majority of its constituent parts that are built
separately via these libraries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 .../Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf   | 2 +-
 .../Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf | 2 +-
 .../Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf   | 2 +-
 .../Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf   | 2 +-
 .../Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf   | 2 +-
 .../UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf   | 2 +-
 ShellPkg/Library/UefiShellTftpCommandLib/UefiShellTftpCommandLib.inf| 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
index d8451c397652..55371eb37194 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
@@ -16,7 +16,7 @@ [Defines]
   INF_VERSION= 0x00010006
   BASE_NAME  = UefiShellDebug1CommandsLib
   FILE_GUID  = 90330D51-A99B-4cc8-A2EB-AE22542A3F45
-  MODULE_TYPE= UEFI_DRIVER
+  MODULE_TYPE= UEFI_APPLICATION
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR= UefiShellDebug1CommandsLibConstructor
diff --git 
a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf 
b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
index a813b3b614f9..f2a2e4c581b0 100644
--- 
a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
+++ 
b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
@@ -16,7 +16,7 @@ [Defines]
   INF_VERSION= 0x00010006
   BASE_NAME  = UefiShellDriver1CommandsLib
   FILE_GUID  = 313D3674-3ED4-48fd-BF97-7DB35D4190D1
-  MODULE_TYPE= UEFI_DRIVER
+  MODULE_TYPE= UEFI_APPLICATION
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR= UefiShellDriver1CommandsLibConstructor
diff --git 
a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf 
b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
index d21ad41ca9f3..4d3713b46092 100644
--- a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
@@ -16,7 +16,7 @@ [Defines]
   INF_VERSION= 0x00010006
   BASE_NAME  = UefiShellLevel1CommandsLib
   FILE_GUID  = 50cb6037-1102-47af-b2dd-9944b6eb1abe
-  MODULE_TYPE= UEFI_DRIVER
+  MODULE_TYPE= UEFI_APPLICATION
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR= ShellLevel1CommandsLibConstructor
diff --git 
a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
index 324d210d31ca..76540fb3a364 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
@@ -17,7 +17,7 @@ [Defines]
   INF_VERSION= 0x00010006
   BASE_NAME  = UefiShellLevel2CommandsLib
   FILE_GUID  = CBF3931C-A2DF-40e5-B77E-CCA9555E9755
-  MODULE_TYPE= UEFI_DRIVER
+  MODULE_TYPE= UEFI_APPLICATION
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = NULL|UEFI_APPLICATION UEFI_DRIVER
   CONSTRUCTOR= ShellLevel2CommandsLibConstructor
diff --git 
a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf 
b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
index 7cc49605ee86..a13cd9babf55 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comm

[edk2] [PATCH v2] NetworkPkg: Update HttpDxe to consume EFI_HTTP_UTILITIES_PROTOCOL

[Jiaxin Wu]

v2:
* Register a notification function to be executed for Http utilities protocol
in the drivers entry points.

Since we add EFI_HTTP_UTILITIES_PROTOCOL support, HttpDxe driver should
be updated to remove internal http utilities functions and consume this
protocol directly.

Cc: Ye Ting 
Cc: Siyuan Fu 
Cc: Samer El-Haj-Mahmoud 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpDriver.c|  54 +++-
 NetworkPkg/HttpDxe/HttpDriver.h|   4 +-
 NetworkPkg/HttpDxe/HttpDxe.inf |   3 +-
 NetworkPkg/HttpDxe/HttpImpl.c  |  20 +-
 NetworkPkg/HttpDxe/HttpProto.c |  30 +-
 NetworkPkg/HttpDxe/HttpUtilities.c | 622 -
 NetworkPkg/HttpDxe/HttpUtilities.h |  82 -
 7 files changed, 94 insertions(+), 721 deletions(-)
 delete mode 100644 NetworkPkg/HttpDxe/HttpUtilities.c
 delete mode 100644 NetworkPkg/HttpDxe/HttpUtilities.h

diff --git a/NetworkPkg/HttpDxe/HttpDriver.c b/NetworkPkg/HttpDxe/HttpDriver.c
index 43f42e2..bd1d04e 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.c
+++ b/NetworkPkg/HttpDxe/HttpDriver.c
@@ -13,10 +13,12 @@
 
 **/
 
 #include "HttpDriver.h"
 
+EFI_HTTP_UTILITIES_PROTOCOL *mHttpUtilities = NULL;
+
 ///
 /// Driver Binding Protocol instance
 ///
 EFI_DRIVER_BINDING_PROTOCOL gHttpDxeDriverBinding = {
   HttpDxeDriverBindingSupported,
@@ -99,10 +101,39 @@ HttpCleanService (
   );
   }
 }
 
 /**
+  The event process routine when the http utilities protocol is installed
+  in the system.
+
+  @param[in] Event Not used.
+  @param[in] Context   The pointer to the IP4 config2 instance data.
+
+**/
+VOID
+EFIAPI
+HttpUtilitiesInstalledCallback (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  gBS->LocateProtocol (
+ &gEfiHttpUtilitiesProtocolGuid, 
+ NULL, 
+ (VOID **) &mHttpUtilities
+ );
+
+  //
+  // Close the event if Http utilities protocol is loacted.
+  //
+  if (mHttpUtilities != NULL && Event != NULL) {
+ gBS->CloseEvent (Event);
+  }
+}
+
+/**
   This is the declaration of an EFI image entry point. This entry point is
   the same for UEFI Applications, UEFI OS Loaders, and UEFI Drivers including
   both device drivers and bus drivers.
 
   @param  ImageHandle   The firmware allocated handle for the UEFI 
image.
@@ -116,11 +147,32 @@ EFI_STATUS
 EFIAPI
 HttpDxeDriverEntryPoint (
   IN EFI_HANDLEImageHandle,
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
-{
+{ 
+  VOID   *Registration;
+
+  gBS->LocateProtocol (
+ &gEfiHttpUtilitiesProtocolGuid, 
+ NULL, 
+ (VOID **) &mHttpUtilities
+ );
+
+  if (mHttpUtilities == NULL) {
+//
+// No Http utilities protocol, register a notify.
+//
+EfiCreateProtocolNotifyEvent (
+  &gEfiHttpUtilitiesProtocolGuid,
+  TPL_CALLBACK,
+  HttpUtilitiesInstalledCallback,
+  NULL,
+  &Registration
+  );
+  }
+
   //
   // Install UEFI Driver Model protocol(s).
   //
   return EfiLibInstallDriverBindingComponentName2 (
ImageHandle,
diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 5bad705..d95b05b 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -39,10 +39,11 @@
 #include 
 
 //
 // Consumed Protocols
 //
+#include 
 #include 
 #include 
 #include 
 
 //
@@ -60,18 +61,19 @@
 //
 extern EFI_DRIVER_BINDING_PROTOCOL  gHttpDxeDriverBinding;
 extern EFI_COMPONENT_NAME2_PROTOCOL gHttpDxeComponentName2;
 extern EFI_COMPONENT_NAME_PROTOCOL  gHttpDxeComponentName;
 
+extern EFI_HTTP_UTILITIES_PROTOCOL  *mHttpUtilities;
+
 //
 // Include files with function prototypes
 //
 #include "ComponentName.h"
 #include "HttpImpl.h"
 #include "HttpProto.h"
 #include "HttpDns.h"
-#include "HttpUtilities.h"
 
 typedef struct {
   EFI_SERVICE_BINDING_PROTOCOL  *ServiceBinding;
   UINTN NumberOfChildren;
   EFI_HANDLE*ChildHandleBuffer;
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 4632934..d9652b3 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -36,12 +36,10 @@
   HttpDriver.c
   HttpImpl.h
   HttpImpl.c
   HttpProto.h
   HttpProto.c
-  HttpUtilities.h
-  HttpUtilities.c
 
 [LibraryClasses]
   UefiDriverEntryPoint
   UefiBootServicesTableLib
   MemoryAllocationLib
@@ -52,10 +50,11 @@
   HttpLib
 
 [Protocols]
   gEfiHttpServiceBindingProtocolGuid   ## BY_START
   gEfiHttpProtocolGuid ## BY_START
+  gEfiHttpUtilitiesProtocolGuid## TO_START
   gEfiTcp4ServiceBindingProtocolGuid   ## TO_START
   gEfiTcp4ProtocolGuid ## TO_START
   gEfiDns4ServiceBindingProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiDns4ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiIp4Config2ProtocolGuid   

Re: [edk2] [patch] SecurityPkg: Fix one returned status code issue in P7Verify Protocol.

[Zhang, Chao B]

Qin:
  In this case, Should AttachedData be freed before return UNSUPPORTED 





Thanks & Best regards
Chao Zhang

-Original Message-
From: Long, Qin 
Sent: Tuesday, August 25, 2015 4:53 PM
To: Zhang, Chao B; edk2-devel@lists.01.org
Subject: [patch] SecurityPkg: Fix one returned status code issue in P7Verify 
Protocol.

VerifyBuffer() in PKCS7 Verify Protocol should return EFI_UNSUPPORTED when 
signed data is embedded in SignedData but InData is not NULL.
This patch is to comply with the spec definition.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 13c9138..8bf1f1f 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -833,6 +833,12 @@ VerifyBuffer (
 return EFI_UNSUPPORTED;
   }
   if (AttachedData != NULL) {
+if (InData != NULL) {
+  //
+  // Signed data embedded in SignedData but InData is not NULL
+  //
+  return EFI_UNSUPPORTED;
+}
 //
 // PKCS7-formatted signedData with attached content; Use the embedded
 // content for verification
--
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] MdeModulePkg/Xhci: Remove TDs from transfer ring when timeout happens

[Anbazhagan, Baraneedharan]

Looks good to me.

Reviewed-by: Baraneedharan Anbazhagan 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tian, 
> Feng
> Sent: Monday, August 24, 2015 10:17 PM
> To: Anbazhagan, Baraneedharan ; Zeng, Star
> 
> Cc: edk2-devel@lists.01.org; Tian, Feng 
> Subject: Re: [edk2] [patch] MdeModulePkg/Xhci: Remove TDs from transfer ring 
> when
> timeout happens
> 
> Thanks for your comments, Baranee. I updated the patch as below. Please review
> again.
> 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |  75 +++---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 193 +
> MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  80 +++
>  MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c   |  51 +--
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c | 235 +++-
> ---  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.h |  81 ++-
>  6 files changed, 618 insertions(+), 97 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 390ca0a..39c28ab 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -905,17 +905,28 @@ XhcControlTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcControlTransfer:
> + XhcDequeueTrbFromEndpoint failed\n"));
>  }
> -Status = EFI_DEVICE_ERROR;
>  goto FREE_URB;
>} else {
> -goto FREE_URB;
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcControlTransfer: XhcRecoverHaltedEndpoint
> failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
> +  goto FREE_URB;
> +} else {
> +  goto FREE_URB;
> +}
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1241,14 +1252,24 @@ XhcBulkTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint 
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcBulkTransfer: XhcDequeueTrbFromEndpoint
> failed\n"));
> +}
> +  } else {
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint 
> failed\n"));
> +  }
> +  Status = EFI_DEVICE_ERROR;
>  }
> -Status = EFI_DEVICE_ERROR;
>}
> 
>Xhc->PciIo->Flush (Xhc->PciIo);
> @@ -1538,14 +1559,24 @@ XhcSyncInterruptTransfer (
>*TransferResult = Urb->Result;
>*DataLength = Urb->Completed;
> 
> -  if (*TransferResult == EFI_USB_NOERROR) {
> -Status = EFI_SUCCESS;
> -  } else if (*TransferResult == EFI_USB_ERR_STALL) {
> -RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> -if (EFI_ERROR (RecoveryStatus)) {
> -  DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: 
> XhcRecoverHaltedEndpoint
> failed\n"));
> +  if (Status == EFI_TIMEOUT) {
> +//
> +// The transfer timed out. Abort the transfer by dequeueing of the TD.
> +//
> +RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
> +if (EFI_ERROR(RecoveryStatus)) {
> +  DEBUG((EFI_D_ERROR, "XhcSyncInterruptTransfer: 
> XhcDequeueTrbFromEndpoint
> failed\n"));
> +}
> +  } else {
> +if (*TransferResult == EFI_USB_NOERROR) {
> +  Status = EFI_SUCCESS;
> +} else if (*TransferResult == EFI_USB_ERR_STALL) {
> +  RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
> +  if (EFI_ERROR (RecoveryStatus)) {
> +DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: 
> XhcRecoverHaltedEndpoint
> failed\n"));
> +  }
> +

Re: [edk2] Question about memory reservation from PrePi to DXE

[Benjamin Herrenschmidt]

On Tue, 2015-08-25 at 06:13 +, Gao, Liming wrote:> Ben:
>   I agree this is an issue. PI spec has no limitation to create 
> memory allocation hob. Memory may be allocated in either the HO aprod
> ucer phase memory area or other areas of present and initialized 
> system memory. DxeCore shouldn't assume the memory allocation hob are 
> all created by PEI AllocatePages() service. I attach my patch to add 
> the check in CoreInitializeMemoryServices(). Could you help reviewand
> try first?

Ok, I'll have a close look tomorrow.

Your approach does mean however that if you have a single descriptor
covering all of memory you will not consider it at all if it contains a
single allocation Hob.

In that case we should at least consider the PEI memory between free
bottom/top as pointed by the PHIT as a valid fallback.

My gut feeling is that overall, we need very little memory to
initialize the DXE core before we can properly handle the allocation
HOBs and that maybe we are trying a bit "too hard" here.

We might be better off with a bit of BSS to use as initial memory
(maybe set via a PCD) or maybe restricting ourselves to the free memory
left by PEI as pointed by the PHIT and be done with it (ie, require
that the PEI phase leaves enough free there to do that early DXE init).

Otherwise, we might end up writing a really complex algorithm to try to
find a chunk of free memory not covered by an allocation hob.

If you really prefer going down that path, I would suggest doing as
follow:

 - Once you have a candidate region (either after-PHIT in the PHIT
-owning descriptor or another descriptor higher up) call a function to
"adjust" it. That candidate region however is only as big as the min
memory needed to init DXE, don't use the enitre descriptor size.

 - That function scans for allocation hobs. If there's a collision, try
again moving your start address to the end of the allocation hob as
long as you are still inside the same descriptor

Rince & repeat until you have no collision.

Cheers,
Ben.

> Limin
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> Of Benjamin Herrenschmidt
> Sent: Saturday, August 22, 2015 12:35 PM
> To: Tian, Feng
> Cc: Kinney, Michael D; edk2-devel@lists.01.org; Laszlo Ersek; Andrew
> Fish
> Subject: Re: [edk2] Question about memory reservation from PrePi to
> DXE
> 
> On Mon, 2015-08-10 at 16:18 -0700, Andrew Fish wrote:
> 
> > Laszlo and I are not the maintainers for this code. 
> > https://github.com/tianocore/edk2/blob/master/Maintainers.txt < 
> > https://github.com/tianocore/edk2/blob/master/Maintainers.txt>;
> > 
> > But it does feel like the assumption in the DXE Core is not implied
> > by 
> > the PI Specifications.
> 
> Hi Feng !
> 
> You never replied to that thread ... my problem is that the code in
> CoreInitializeMemoryServices() will essentially pickup a piece of
> memory out of *any* memory descriptor HOB completely ignoring the
> memory allocation HOBs.
> 
> That means that if Sec or Peim (or in my case, a special PrePei that
> transfers controls to DXE from a different firmware layer) has marked
> some memory as reserved via some memory allocation HOBs, there is
> still a chance that this memory gets overwriten early during DXE
> initialization, because CoreInitializeMemoryServices() ignores those
> allocations completely to pick up the initial core memory, which it
> then makes available to the allocator with CoreAddMemoryDescriptor().
> 
> It appears that the Gcd code is generally trying to honor the
> existing allocation HOBs since later on, CoreInitializeGcdServices()
> will iterate them and allocates them, but this happens after the
> above initialization and some allocations do happen in between.
> 
> Is there a non-documented (or maybe I missed the documentation)
> requirement that all the Memory resource descriptor HOBs passed to
> DXE have been modified to *remove* all allocated space from them to
> avoid that problem or is this mainly a bug ?
> 
> It does make the transition from a Flat Device Tree to DXE tricky,
> since in the FDT world, we have on one hand the "memory" modes that
> give us the regions of physical memory in the system, which would
> nicely translate to memory resource descriptors, and separately we
> have a reserve map which describes ranges of addresses that are
> reserved, either by some firmware layer or in some cases by the HW
> itself.
> 
> Unless I missed something, while the spec allows us to match this to
> memory resource descriptors and memory allocation HOBs respectively,
> the DXE implementation will fail to honor such overlap properly.
> 
> Am I correct ?
> 
> Cheers,
> Ben.
> 
> ___
> 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] MdeModulePkg PiSmmCore: Prevent overlap for gSmmCorePrivate and CommBuffer

[Star Zeng]

InternalIsBufferOverlapped() is created to check overlap between buffers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 44 +++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c 
b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 55dcf31..496638a 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -379,6 +379,37 @@ SmmEndOfDxeHandler (
 }
 
 /**
+  Determine if two buffers overlap in memory.
+
+  @param[in] Buff1  Pointer to first buffer
+  @param[in] Size1  Size of Buff1
+  @param[in] Buff2  Pointer to second buffer
+  @param[in] Size2  Size of Buff2
+
+  @retval TRUE  Buffers overlap in memory.
+  @retval FALSE Buffer doesn't overlap.
+
+**/
+BOOLEAN
+InternalIsBufferOverlapped (
+  IN UINT8  *Buff1,
+  IN UINTN  Size1,
+  IN UINT8  *Buff2,
+  IN UINTN  Size2
+  )
+{
+  //
+  // If buff1's end is less than the start of buff2, then it's ok.
+  // Also, if buff1's start is beyond buff2's end, then it's ok.
+  //
+  if (((Buff1 + Size1) <= Buff2) || (Buff1 >= (Buff2 + Size2))) {
+return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
   The main entry point to SMM Foundation.
 
   Note: This function is only used by SMRAM invocation.  It is never used by 
DXE invocation.
@@ -396,6 +427,7 @@ SmmEntryPoint (
   EFI_STATUS  Status;
   EFI_SMM_COMMUNICATE_HEADER  *CommunicateHeader;
   BOOLEAN InLegacyBoot;
+  BOOLEAN IsOverlapped;
 
   PERF_START (NULL, "SMM", NULL, 0) ;
 
@@ -427,9 +459,17 @@ SmmEntryPoint (
   //
   // Synchronous SMI for SMM Core or request from Communicate protocol
   //
-  if (!SmmIsBufferOutsideSmmValid 
((UINTN)gSmmCorePrivate->CommunicationBuffer, gSmmCorePrivate->BufferSize)) {
+  IsOverlapped = InternalIsBufferOverlapped (
+   (UINT8 *) gSmmCorePrivate->CommunicationBuffer,
+   gSmmCorePrivate->BufferSize,
+   (UINT8 *) gSmmCorePrivate,
+   sizeof (*gSmmCorePrivate)
+   );
+  if (!SmmIsBufferOutsideSmmValid 
((UINTN)gSmmCorePrivate->CommunicationBuffer, gSmmCorePrivate->BufferSize) || 
IsOverlapped) {
 //
-// If CommunicationBuffer is not in valid address scope, return 
EFI_INVALID_PARAMETER
+// If CommunicationBuffer is not in valid address scope,
+// or there is overlap between gSmmCorePrivate and CommunicationBuffer,
+// return EFI_INVALID_PARAMETER
 //
 gSmmCorePrivate->CommunicationBuffer = NULL;
 gSmmCorePrivate->ReturnStatus = 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

Re: [edk2] [Patch] Nt32Pkg: Keep boot behavior using new BDS almost same as that using old BDS

[Gao, Liming]

Reviewed-by: Liming Gao 

-Original Message-
From: Ni, Ruiyu 
Sent: Tuesday, August 25, 2015 5:02 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu; Gao, Liming
Subject: [Patch] Nt32Pkg: Keep boot behavior using new BDS almost same as that 
using old BDS

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Liming Gao 
---
 .../Library/PlatformBootManagerLib/MemoryTest.c| 1086 
 .../PlatformBootManagerLib/PlatformBootManager.c   |  137 +--
 .../PlatformBootManagerLib/PlatformBootManager.h   |   76 +-
 .../PlatformBootManagerLib.inf |   18 +
 Nt32Pkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3732 bytes
 5 files changed, 1235 insertions(+), 82 deletions(-)  create mode 100644 
Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
 create mode 100644 Nt32Pkg/Library/PlatformBootManagerLib/Strings.uni

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c 
b/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
new file mode 100644
index 000..9f57466
--- /dev/null
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
@@ -0,0 +1,1086 @@
+/** @file
+  Perform the platform memory test
+
+Copyright (c) 2004 - 2015, 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 "PlatformBootManager.h"
+
+EFI_HII_HANDLE gStringPackHandle  = NULL;
+EFI_GUID   mPlatformBootManagerStringPackGuid = {
+  0x154dd51, 0x9079, 0x4a10, { 0x89, 0x5c, 0x9c, 0x7, 0x72, 0x81, 0x57, 
+0x88 }
+  };
+// extern UINT8  BdsDxeStrings[];
+
+//
+// BDS Platform Functions
+//
+/**
+
+  Show progress bar with title above it. It only works in Graphics mode.
+
+
+  @param TitleForeground Foreground color for Title.
+  @param TitleBackground Background color for Title.
+  @param Title   Title above progress bar.
+  @param ProgressColor   Progress bar color.
+  @param ProgressProgress (0-100)
+  @param PreviousValue   The previous value of the progress.
+
+  @retval  EFI_STATUS   Success update the progress bar
+
+**/
+EFI_STATUS
+PlatformBootManagerShowProgress (
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL TitleForeground,
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL TitleBackground,
+  IN CHAR16*Title,
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL ProgressColor,
+  IN UINTN Progress,
+  IN UINTN PreviousValue
+  )
+{
+  EFI_STATUS Status;
+  EFI_GRAPHICS_OUTPUT_PROTOCOL   *GraphicsOutput;
+  EFI_UGA_DRAW_PROTOCOL  *UgaDraw;
+  UINT32 SizeOfX;
+  UINT32 SizeOfY;
+  UINT32 ColorDepth;
+  UINT32 RefreshRate;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
+  UINTN  BlockHeight;
+  UINTN  BlockWidth;
+  UINTN  BlockNum;
+  UINTN  PosX;
+  UINTN  PosY;
+  UINTN  Index;
+
+  if (Progress > 100) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  UgaDraw = NULL;
+  Status = gBS->HandleProtocol (
+  gST->ConsoleOutHandle,
+  &gEfiGraphicsOutputProtocolGuid,
+  (VOID **) &GraphicsOutput
+  );
+  if (EFI_ERROR (Status) && FeaturePcdGet (PcdUgaConsumeSupport)) {
+GraphicsOutput = NULL;
+
+Status = gBS->HandleProtocol (
+gST->ConsoleOutHandle,
+&gEfiUgaDrawProtocolGuid,
+(VOID **) &UgaDraw
+);
+  }
+  if (EFI_ERROR (Status)) {
+return EFI_UNSUPPORTED;
+  }
+
+  SizeOfX = 0;
+  SizeOfY = 0;
+  if (GraphicsOutput != NULL) {
+SizeOfX = GraphicsOutput->Mode->Info->HorizontalResolution;
+SizeOfY = GraphicsOutput->Mode->Info->VerticalResolution;
+  } else if (UgaDraw != NULL) {
+Status = UgaDraw->GetMode (
+UgaDraw,
+&SizeOfX,
+&SizeOfY,
+&ColorDepth,
+&RefreshRate
+);
+if (EFI_ERROR (Status)) {
+  return EFI_UNSUPPORTED;
+}
+  } else {
+return EFI_UNSUPPORTED;
+  }
+
+  BlockWidth  = SizeOfX / 100;
+  BlockHeight = SizeOfY / 50;
+
+  BlockNum= Progress;
+
+  PosX= 0;
+  PosY= SizeOfY * 48 / 50;
+
+  if (BlockNum == 0) {
+//
+// Clear progress area
+//
+SetMem (&Color, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0);
+
+if (GraphicsOutput != NULL) {
+  

[edk2] [Patch] Nt32Pkg: Keep boot behavior using new BDS almost same as that using old BDS

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Liming Gao 
---
 .../Library/PlatformBootManagerLib/MemoryTest.c| 1086 
 .../PlatformBootManagerLib/PlatformBootManager.c   |  137 +--
 .../PlatformBootManagerLib/PlatformBootManager.h   |   76 +-
 .../PlatformBootManagerLib.inf |   18 +
 Nt32Pkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3732 bytes
 5 files changed, 1235 insertions(+), 82 deletions(-)
 create mode 100644 Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
 create mode 100644 Nt32Pkg/Library/PlatformBootManagerLib/Strings.uni

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c 
b/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
new file mode 100644
index 000..9f57466
--- /dev/null
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/MemoryTest.c
@@ -0,0 +1,1086 @@
+/** @file
+  Perform the platform memory test
+
+Copyright (c) 2004 - 2015, 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 "PlatformBootManager.h"
+
+EFI_HII_HANDLE gStringPackHandle  = NULL;
+EFI_GUID   mPlatformBootManagerStringPackGuid = {
+  0x154dd51, 0x9079, 0x4a10, { 0x89, 0x5c, 0x9c, 0x7, 0x72, 0x81, 0x57, 0x88 }
+  };
+// extern UINT8  BdsDxeStrings[];
+
+//
+// BDS Platform Functions
+//
+/**
+
+  Show progress bar with title above it. It only works in Graphics mode.
+
+
+  @param TitleForeground Foreground color for Title.
+  @param TitleBackground Background color for Title.
+  @param Title   Title above progress bar.
+  @param ProgressColor   Progress bar color.
+  @param ProgressProgress (0-100)
+  @param PreviousValue   The previous value of the progress.
+
+  @retval  EFI_STATUS   Success update the progress bar
+
+**/
+EFI_STATUS
+PlatformBootManagerShowProgress (
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL TitleForeground,
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL TitleBackground,
+  IN CHAR16*Title,
+  IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL ProgressColor,
+  IN UINTN Progress,
+  IN UINTN PreviousValue
+  )
+{
+  EFI_STATUS Status;
+  EFI_GRAPHICS_OUTPUT_PROTOCOL   *GraphicsOutput;
+  EFI_UGA_DRAW_PROTOCOL  *UgaDraw;
+  UINT32 SizeOfX;
+  UINT32 SizeOfY;
+  UINT32 ColorDepth;
+  UINT32 RefreshRate;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL  Color;
+  UINTN  BlockHeight;
+  UINTN  BlockWidth;
+  UINTN  BlockNum;
+  UINTN  PosX;
+  UINTN  PosY;
+  UINTN  Index;
+
+  if (Progress > 100) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  UgaDraw = NULL;
+  Status = gBS->HandleProtocol (
+  gST->ConsoleOutHandle,
+  &gEfiGraphicsOutputProtocolGuid,
+  (VOID **) &GraphicsOutput
+  );
+  if (EFI_ERROR (Status) && FeaturePcdGet (PcdUgaConsumeSupport)) {
+GraphicsOutput = NULL;
+
+Status = gBS->HandleProtocol (
+gST->ConsoleOutHandle,
+&gEfiUgaDrawProtocolGuid,
+(VOID **) &UgaDraw
+);
+  }
+  if (EFI_ERROR (Status)) {
+return EFI_UNSUPPORTED;
+  }
+
+  SizeOfX = 0;
+  SizeOfY = 0;
+  if (GraphicsOutput != NULL) {
+SizeOfX = GraphicsOutput->Mode->Info->HorizontalResolution;
+SizeOfY = GraphicsOutput->Mode->Info->VerticalResolution;
+  } else if (UgaDraw != NULL) {
+Status = UgaDraw->GetMode (
+UgaDraw,
+&SizeOfX,
+&SizeOfY,
+&ColorDepth,
+&RefreshRate
+);
+if (EFI_ERROR (Status)) {
+  return EFI_UNSUPPORTED;
+}
+  } else {
+return EFI_UNSUPPORTED;
+  }
+
+  BlockWidth  = SizeOfX / 100;
+  BlockHeight = SizeOfY / 50;
+
+  BlockNum= Progress;
+
+  PosX= 0;
+  PosY= SizeOfY * 48 / 50;
+
+  if (BlockNum == 0) {
+//
+// Clear progress area
+//
+SetMem (&Color, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0);
+
+if (GraphicsOutput != NULL) {
+  Status = GraphicsOutput->Blt (
+  GraphicsOutput,
+  &Color,
+  EfiBltVideoFill,
+  0,
+  0,
+  0,
+ 

[edk2] [patch] SecurityPkg: Fix one returned status code issue in P7Verify Protocol.

[Qin Long]

VerifyBuffer() in PKCS7 Verify Protocol should return EFI_UNSUPPORTED
when signed data is embedded in SignedData but InData is not NULL.
This patch is to comply with the spec definition.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 13c9138..8bf1f1f 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -833,6 +833,12 @@ VerifyBuffer (
 return EFI_UNSUPPORTED;
   }
   if (AttachedData != NULL) {
+if (InData != NULL) {
+  //
+  // Signed data embedded in SignedData but InData is not NULL
+  //
+  return EFI_UNSUPPORTED;
+}
 //
 // PKCS7-formatted signedData with attached content; Use the embedded
 // content for verification
-- 
1.9.5.msysgit.1

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