date:20181026

Re: [edk2] [PATCH] MdeModulePkg/Core: correct one coding style

2018-10-26 Thread Bi, Dandan

Reviewed-by: Bi Dandan 

Thanks,
Dandan

> -Original Message-
> From: Wang, Jian J
> Sent: Saturday, October 27, 2018 10:13 AM
> To: edk2-devel@lists.01.org
> Cc: Bi, Dandan ; Zeng, Star 
> Subject: [PATCH] MdeModulePkg/Core: correct one coding style
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1284
> 
> Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
> 
> Cc: Dandan Bi 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> index 521e0d7b2a..9f89df8d8f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> @@ -1559,7 +1559,7 @@ PromoteGuardedFreePages (
>  }
>}
> 
> -  if (AvailablePages) {
> +  if (AvailablePages != 0) {
>  DEBUG ((DEBUG_INFO, "Promoted pages: %lX (%lx)\r\n", Start,
> (UINT64)AvailablePages));
>  ClearGuardedMemoryBits (Start, AvailablePages);
> 
> --
> 2.19.0.windows.1

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

[edk2] [PATCH] MdeModulePkg/Core: correct one coding style

2018-10-26 Thread Jian J Wang

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1284

Non-Boolean comparisons should use a compare operator
(==, !=, >, < >=, <=)

Cc: Dandan Bi 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 521e0d7b2a..9f89df8d8f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -1559,7 +1559,7 @@ PromoteGuardedFreePages (
 }
   }
 
-  if (AvailablePages) {
+  if (AvailablePages != 0) {
 DEBUG ((DEBUG_INFO, "Promoted pages: %lX (%lx)\r\n", Start, 
(UINT64)AvailablePages));
 ClearGuardedMemoryBits (Start, AvailablePages);
 
-- 
2.19.0.windows.1

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

[edk2] Test message. Please ignore.

2018-10-26 Thread Chris Co via edk2-devel

Test message. Checking for DMARC bounces...

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

[edk2] Example of email Phabricator can be configured to send on svn commit

2018-10-26 Thread Rebecca Cran

Since we've been discussing Phabricator for code reviews, I thought 
people might be interested in an example of the emails it can be 
configured to send. This one's from the FreeBSD project and its instance 
at https://reviews.freebsd.org when I committed a changeset to the SVN repo.


This revision was automatically updated to reflect the committed changes.
Closed by commit rS339796: Simplify the EFI delay() function by calling 
BS-Stall() (authored by bcran, committed by ).

CHANGED PRIOR TO COMMIT
  https://reviews.freebsd.org/D16753?vs=46794=49670#toc

REPOSITORY
  rS FreeBSD src repository

CHANGES SINCE LAST UPDATE
  https://reviews.freebsd.org/D16753?vs=46794=49670

CHANGES SINCE LAST ACTION
  https://reviews.freebsd.org/D16753/new/

REVISION DETAIL
  https://reviews.freebsd.org/D16753

AFFECTED FILES
  head/stand/efi/libefi/delay.c

CHANGE DETAILS

diff --git a/head/stand/efi/libefi/delay.c b/head/stand/efi/libefi/delay.c
--- a/head/stand/efi/libefi/delay.c
+++ b/head/stand/efi/libefi/delay.c
@@ -33,15 +33,5 @@
 void
 delay(int usecs)
 {
-   static EFI_EVENT ev = 0;
-   UINTN junk;
-
-   if (!ev) {
-   if (BS->CreateEvent(EVT_TIMER, TPL_APPLICATION, 0, 0, )
-   != EFI_SUCCESS)
-   return;
-   }
-
-   BS->SetTimer(ev, TimerRelative, usecs * 10);
-   BS->WaitForEvent(1, , );
+   BS->Stall(usecs);
 }



EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: bcran, imp, kevans, manu, eadler


D16753.49670.patch

diff --git a/head/stand/efi/libefi/delay.c b/head/stand/efi/libefi/delay.c
--- a/head/stand/efi/libefi/delay.c
+++ b/head/stand/efi/libefi/delay.c
@@ -33,15 +33,5 @@
 void
 delay(int usecs)
 {
-   static EFI_EVENT ev = 0;
-   UINTN junk;
-
-   if (!ev) {
-   if (BS->CreateEvent(EVT_TIMER, TPL_APPLICATION, 0, 0, )
-   != EFI_SUCCESS)
-   return;
-   }
-
-   BS->SetTimer(ev, TimerRelative, usecs * 10);
-   BS->WaitForEvent(1, , );
+   BS->Stall(usecs);
 }

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

[edk2] [Patch] BaseTools: Rollback Filter out unused structure pcds

2018-10-26 Thread BobCF

This reverts commit 51d17bb7b0da0d9c9e91c226f1982d7020f43795.
commit 51d17bb7b0da0d9c9e91c226f1982d7020f43795 adds new check
of Pcds in the platform unused library INF files.
It breaks the existing platform.
To avoid the impact, roll back this change first.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob C Feng 
Cc: Liming Gao 
---
 .../Source/Python/Workspace/DscBuildData.py   | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py 
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 9882a36df8..6d596b2b54 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -280,11 +280,10 @@ class DscBuildData(PlatformBuildClassObject):
 self._RFCLanguages  = None
 self._ISOLanguages  = None
 self._VpdToolGuid   = None
 self._MacroDict = None
 self.DefaultStores  = None
-self.UsedStructurePcd   = None
 
 ## handle Override Path of Module
 def _HandleOverridePath(self):
 RecordList = self._RawData[MODEL_META_DATA_COMPONENT, self._Arch]
 for Record in RecordList:
@@ -1468,11 +1467,10 @@ class DscBuildData(PlatformBuildClassObject):
 if str_pcd_obj.Type in 
[self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII], 
self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]]:
 str_pcd_obj_str.DefaultFromDSC = 
{skuname:{defaultstore: 
str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore, 
str_pcd_obj.SkuInfoList[skuname].HiiDefaultValue) for defaultstore in 
DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
 else:
 str_pcd_obj_str.DefaultFromDSC = 
{skuname:{defaultstore: 
str_pcd_obj.SkuInfoList[skuname].DefaultStoreDict.get(defaultstore, 
str_pcd_obj.SkuInfoList[skuname].DefaultValue) for defaultstore in 
DefaultStores} for skuname in str_pcd_obj.SkuInfoList}
 S_pcd_set[Pcd] = str_pcd_obj_str
-self.FilterStrcturePcd(S_pcd_set)
 if S_pcd_set:
 GlobalData.gStructurePcd[self.Arch] = S_pcd_set
 for stru_pcd in S_pcd_set.values():
 for skuid in SkuIds:
 if skuid in stru_pcd.SkuOverrideValues:
@@ -1564,27 +1562,10 @@ class DscBuildData(PlatformBuildClassObject):
 elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in 
pcd.SkuInfoList:
 del pcd.SkuInfoList[TAB_COMMON]
 
 map(self.FilterSkuSettings, [Pcds[pcdkey] for pcdkey in Pcds if 
Pcds[pcdkey].Type in DynamicPcdType])
 return Pcds
-#Filter the StrucutrePcd that is not used by any module in dsc file and 
fdf file.
-def FilterStrcturePcd(self, S_pcd_set):
-if not self.UsedStructurePcd:
-FdfInfList = []
-if GlobalData.gFdfParser:
-FdfInfList = GlobalData.gFdfParser.Profile.InfList
-FdfModuleList = [PathClass(NormPath(Inf), GlobalData.gWorkspace, 
Arch=self._Arch) for Inf in FdfInfList]
-AllModulePcds = set()
-ModuleSet = set(self._Modules.keys() + self.LibraryInstances + 
FdfModuleList)
-for ModuleFile in ModuleSet:
-ModuleData = self._Bdb[ModuleFile, self._Arch, self._Target, 
self._Toolchain]
-AllModulePcds = AllModulePcds | set(ModuleData.Pcds.keys())
-
-self.UsedStructurePcd = AllModulePcds
-UnusedStruPcds = set(S_pcd_set.keys()) - self.UsedStructurePcd
-for (Token, TokenSpaceGuid) in UnusedStruPcds:
-del S_pcd_set[(Token, TokenSpaceGuid)]
 
 ## Retrieve non-dynamic PCD settings
 #
 #   @param  TypePCD type
 #
-- 
2.18.0.windows.1

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

Re: [edk2] [PATCH] IntelFsp2Pkg: Fixed potentially NULL pointer accessing

2018-10-26 Thread Yao, Jiewen

Hi Chasel
Can we change "  if ((IdtDescriptor.Base == 0) && (IdtDescriptor.Limit == 
0x)) {" to "  if (IdtDescriptor.Base == 0) {" ?

That can simplify the logic.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Chasel, Chiu
> Sent: Friday, October 26, 2018 3:25 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [edk2] [PATCH] IntelFsp2Pkg: Fixed potentially NULL pointer
> accessing
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1280
> 
> When copying IDT table in SecMain, the pointer might be
> NULL so added the check to fix it.
> 
> Test: Verified on internal platform and boots successfully.
> 
> Cc: Jiewen Yao 
> Cc: Desimone Nathaniel L 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu 
> ---
>  IntelFsp2Pkg/FspSecCore/SecMain.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index f319c68cc5..aed8893ff0 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
> @@ -113,8 +113,14 @@ SecStartup (
>// ERROR: IDT table size from boot loader is larger than FSP can
> support, DeadLoop here!
>//
>CpuDeadLoop();
> +} else if (IdtDescriptor.Base == 0)  {
> +  //
> +  // ERROR: IDT table Base should not be zero, DeadLoop here!
> +  //
> +  CpuDeadLoop();
> +} else {
> +  CopyMem ((VOID *) (UINTN) , (VOID *)
> IdtDescriptor.Base, IdtSize);
>  }
> -CopyMem ((VOID *) (UINTN) , (VOID *)
> IdtDescriptor.Base, IdtSize);
>}
>IdtDescriptor.Base  = (UINTN) 
>IdtDescriptor.Limit = (UINT16)(IdtSize - 1);
> --
> 2.13.3.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

2018-10-26 Thread Zeng, Star


Hao,

On 2018/10/26 15:54, Hao Wu wrote:

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1279

The commit will add 3 types of checks for function ResolveSymlink():

A. Check for the value of 'Component Type' field within a Path Component

According to the ECMA-167 standard (3rd Edition - June 1997), Section
14.16.1.1, valid values are 1 to 5. All other values will be treated as a
corrupted volume.

B. Check for the content pointed by 'File'

Since content within 'File' is the output data for ResolveSymlink().
Checks is added to ensure the content in 'File' is valid. Otherwise,
possible null pointer dereference issue will occur during the subsequent
usage of the data returned by ResolveSymlink().

C. Check for possible memory double free/use after free case

For codes:

 if (CompareMem ((VOID *), (VOID *)Parent,
 sizeof (UDF_FILE_INFO)) != 0) {
   CleanupFileInformation ();
 }

 CopyMem ((VOID *), (VOID *)File, sizeof (UDF_FILE_INFO));

If the contents in 'PreviousFile' and 'File' are the same, call to
"CleanupFileInformation ();" will free the buffers in 'File'
as well. This will lead to potential memory double free/use after free
issues.


If 'PreviousFile' and 'File' are the same, the coping operation below 
also has no need to be done, right?


CopyMem ((VOID *), (VOID *)File, sizeof (UDF_FILE_INFO));

Thanks,
Star



Cc: Paulo Alcantara 
Cc: Paulo Alcantara 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
  .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 30 --
  1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c 
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index b9ebddfe62..a89e5ba9ff 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2145,6 +2145,8 @@ ResolveSymlink (
UINT8   CompressionId;
UDF_FILE_INFO   PreviousFile;
  
+  ZeroMem ((VOID *)File, sizeof (UDF_FILE_INFO));

+
//
// Symlink files on UDF volumes do not contain so much data other than
// Path Components which resolves to real filenames, so it's OK to read in
@@ -2257,6 +2259,13 @@ ResolveSymlink (
}
FileName[Index] = L'\0';
break;
+default:
+  //
+  // Accoring to the ECMA-167 standard (3rd Edition - June 1997), Section
+  // 14.16.1.1, all other values are reserved.
+  //
+  Status = EFI_VOLUME_CORRUPTED;
+  goto Error_Find_File;
  }
  
  //

@@ -2281,8 +2290,18 @@ ResolveSymlink (
break;
  }
  
-if (CompareMem ((VOID *), (VOID *)Parent,

-sizeof (UDF_FILE_INFO)) != 0) {
+//
+// Check the content in the file info pointed by File.
+//
+if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+  Status = EFI_VOLUME_CORRUPTED;
+  goto Error_Find_File;
+}
+
+if ((CompareMem ((VOID *), (VOID *)Parent,
+sizeof (UDF_FILE_INFO)) != 0) &&
+(CompareMem ((VOID *), (VOID *)File,
+sizeof (UDF_FILE_INFO)) != 0)) {
CleanupFileInformation ();
  }
  
@@ -2294,6 +2313,13 @@ ResolveSymlink (

//
FreePool (ReadFileInfo.FileData);
  
+  //

+  // Check the content in the resolved file info.
+  //
+  if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+return EFI_VOLUME_CORRUPTED;
+  }
+
return EFI_SUCCESS;
  
  Error_Find_File:




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

[edk2] Grub Error Marvell Board

2018-10-26 Thread Sven Auhagen

Hi,

I have a Marvell Board like the MCBin with an Marvell 8040 CPU.

EDK2 works fine, a Debian installer runs just fine.
The problem comes after the installation.

When I try to boot from the eMMC where the files were installed on grub seems 
not to find the partitions anymore:

Tianocore/EDK2 firmware version MARVELL_EFI
Press ESCAPE for boot options XhcCheckUrbResult: STALL_ERROR! Completecode = 6
XhcControlTransfer: error - Device Error, transfer - 2
XHCI: Don't support multi-TT feature for Hub now. (force to disable MTT)
Installed Fat filesystem on BE649F98
Installed Fat filesystem on BE80FE98
...[Bds]Booting Debian
Loading driver at 0x000B6732000 EntryPoint=0x000B6732400
Loading driver at 0x000B6732000 EntryPoint=0x000B6732400
error: no such partition.
Entering rescue mode...
grub rescue>
grub rescue> ls
(hd0) (hd1) (hd2) (hd3)
grub rescue> set
cmdpath=(hd1)/efi/debian
prefix=(hd1,gpt2)/boot/grub
root=hd1,gpt2
grub rescue>

Has anyone an idea why this is happening or what could be the cause?

Partitions are setup as GPT
1. EFI FAT
2. EXT4
3. SWAP

Best and thanks
Sven


Beste Grüße/Best Regards

Sven Auhagen
Dipl. Math. oec., M.Sc.

Voleatech GmbH
HRB: B 754643
USTID: DE303643180
Grathwohlstr. 5
72762 Reutlingen
Tel: +49 7121539550
Fax: +49 7121539551
E-Mail: sven.auha...@voleatech.de

www.voleatech.de
Diese Information ist ausschließlich für den Adressaten bestimmt und kann 
vertraulich oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht 
der bestimmungsgemäße Adressat sind, unterrichten Sie bitte den Absender und 
vernichten Sie diese Mail. Anderen als dem bestimmungsgemäßen Adressaten ist es 
untersagt, diese E-Mail zu lesen, zu speichern, weiterzuleiten oder ihren 
Inhalt auf welche Weise auch immer zu verwenden. Für den Adressaten sind die 
Informationen in dieser Mail nur zum persönlichen Gebrauch. Eine Weiterleitung 
darf nur nach Rücksprache mit dem Absender erfolgen. Wir verwenden aktuelle 
Virenschutzprogramme. Für Schäden, die dem Empfänger gleichwohl durch von uns 
zugesandte mit Viren befallene E-Mails entstehen, schließen wir jede Haftung 
aus.

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

Re: [edk2] Community Meeting Minutes #2

2018-10-26 Thread stephano


On 10/26/2018 9:46 AM, stephano wrote> Standard C Types


...We should have a code standard in
place to require the use of one or the other, but not both.


I should be more clear here. When editing code, one should follow the 
pattern currently being used. E.g. If EDK2 types are used the developer 
should not mix in standard C types.


If committing new code, like developing a new module, the developer 
should pick one of the two and stick with it.


That is how I interpreted the discussion but please feel free to comment 
here if I misunderstood.

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

Re: [edk2] [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

2018-10-26 Thread Zeng, Star

Good catch. I just posted V3 patches to cover it.

Thanks,
Star
-Original Message-
From: Wu, Hao A 
Sent: Friday, October 26, 2018 3:33 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Wang, Jian J ; Yao, 
Jiewen 
Subject: RE: [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new 
XhciInsertAsyncIntTransfer function

Hi Star,

The interface for function XhciInsertAsyncIntTransfer() does not match between 
XhciSched.c and XhciSched.h.

Other than that:
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: Zeng, Star
> Sent: Friday, October 26, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star; Ni, Ruiyu; Wu, Hao A; Wang, Jian J; Yao, Jiewen
> Subject: [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new 
> XhciInsertAsyncIntTransfer function
> 
> V2:
> Add the missing "FreePool (Data);".
> Remove the unnecessary indentation change.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> Extract new XhciInsertAsyncIntTransfer function from 
> XhcAsyncInterruptTransfer.
> 
> It is code preparation for following patch, no essential functional 
> change.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 18 +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 65
> 
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 28 ++
>  3 files changed, 94 insertions(+), 17 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index f1c60bef01c0..7f64f9c7c982 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -1346,7 +1346,6 @@ XhcAsyncInterruptTransfer (
>EFI_STATUS  Status;
>UINT8   SlotId;
>UINT8   Index;
> -  UINT8   *Data;
>EFI_TPL OldTpl;
> 
>//
> @@ -1413,36 +1412,21 @@ XhcAsyncInterruptTransfer (
>  goto ON_EXIT;
>}
> 
> -  Data = AllocateZeroPool (DataLength);
> -
> -  if (Data == NULL) {
> -DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to allocate
> buffer\n"));
> -Status = EFI_OUT_OF_RESOURCES;
> -goto ON_EXIT;
> -  }
> -
> -  Urb = XhcCreateUrb (
> +  Urb = XhciInsertAsyncIntTransfer (
>Xhc,
>DeviceAddress,
>EndPointAddress,
>DeviceSpeed,
>MaximumPacketLength,
> -  XHC_INT_TRANSFER_ASYNC,
> -  NULL,
> -  Data,
>DataLength,
>CallBackFunction,
>Context
>);
> -
>if (Urb == NULL) {
> -DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to create
> URB\n"));
> -FreePool (Data);
>  Status = EFI_OUT_OF_RESOURCES;
>  goto ON_EXIT;
>}
> 
> -  InsertHeadList (>AsyncIntTransfers, >UrbList);
>//
>// Ring the doorbell
>//
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 166c44bf5e66..75959ae08363 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -1411,6 +1411,71 @@ XhciDelAllAsyncIntTransfers (  }
> 
>  /**
> +  Insert a single asynchronous interrupt transfer for  the device and 
> + endpoint.
> +
> +  @param XhcThe XHCI Instance
> +  @param BusAddrThe logical device address assigned by UsbBus driver
> +  @param EpAddr Endpoint addrress
> +  @param DevSpeed   The device speed
> +  @param MaxPacket  The max packet length of the endpoint
> +  @param DataLenThe length of data buffer
> +  @param Callback   The function to call when data is transferred
> +  @param ContextThe context to the callback
> +
> +  @return Created URB or NULL
> +
> +**/
> +URB *
> +XhciInsertAsyncIntTransfer (
> +  IN USB_XHCI_INSTANCE  *Xhc,
> +  IN UINT8  BusAddr,
> +  IN UINT8  EpAddr,
> +  IN UINT8  DevSpeed,
> +  IN UINTN  MaxPacket,
> +  IN UINTN  DataLen,
> +  IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
> +  IN VOID   *Context
> +  )
> +{
> +  VOID  *Data;
> +  URB   *Urb;
> +
> +  Data = AllocateZeroPool (DataLen);
> +  if (Data == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n",
> __FUNCTION__));
> +return NULL;
> +  }
> +
> +  Urb = XhcCreateUrb (
> +  Xhc,
> +  BusAddr,
> +  EpAddr,
> +  DevSpeed,
> +  MaxPacket,
> +  XHC_INT_TRANSFER_ASYNC,
> +  NULL,
> +  Data,
> +  DataLen,
> +  Callback,
> +  Context
> +  );
> +  if (Urb == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
>

[edk2] [PATCH V3 2/4] MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function

2018-10-26 Thread Star Zeng

V3:
Match function parameter name and description between
EhciSched.c and EhciSched.h.

V2:
Add the missing "gBS->FreePool (Data);".

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

Extract new EhciInsertAsyncIntTransfer function from
EhcAsyncInterruptTransfer.

It is code preparation for following patch,
no essential functional change.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c  | 25 +--
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 76 
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.h | 35 ++-
 3 files changed, 111 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c 
b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 50b5598df4fb..5569f4f9618b 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -997,7 +997,6 @@ EhcAsyncInterruptTransfer (
   URB *Urb;
   EFI_TPL OldTpl;
   EFI_STATUS  Status;
-  UINT8   *Data;
 
   //
   // Validate parameters
@@ -1046,16 +1045,7 @@ EhcAsyncInterruptTransfer (
 
   EhcAckAllInterrupt (Ehc);
 
-  Data = AllocatePool (DataLength);
-
-  if (Data == NULL) {
-DEBUG ((EFI_D_ERROR, "EhcAsyncInterruptTransfer: failed to allocate 
buffer\n"));
-
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  }
-
-  Urb = EhcCreateUrb (
+  Urb = EhciInsertAsyncIntTransfer (
   Ehc,
   DeviceAddress,
   EndPointAddress,
@@ -1063,9 +1053,6 @@ EhcAsyncInterruptTransfer (
   *DataToggle,
   MaximumPacketLength,
   Translator,
-  EHC_INT_TRANSFER_ASYNC,
-  NULL,
-  Data,
   DataLength,
   CallBackFunction,
   Context,
@@ -1073,20 +1060,10 @@ EhcAsyncInterruptTransfer (
   );
 
   if (Urb == NULL) {
-DEBUG ((EFI_D_ERROR, "EhcAsyncInterruptTransfer: failed to create URB\n"));
-
-gBS->FreePool (Data);
 Status = EFI_OUT_OF_RESOURCES;
 goto ON_EXIT;
   }
 
-  //
-  // New asynchronous transfer must inserted to the head.
-  // Check the comments in EhcMoniteAsyncRequests
-  //
-  EhcLinkQhToPeriod (Ehc, Urb->Qh);
-  InsertHeadList (>AsyncIntTransfers, >UrbList);
-
 ON_EXIT:
   Ehc->PciIo->Flush (Ehc->PciIo);
   gBS->RestoreTPL (OldTpl);
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c 
b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
index 168280be81d7..ec8d796fab11 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
@@ -814,6 +814,82 @@ EhciDelAllAsyncIntTransfers (
   }
 }
 
+/**
+  Insert a single asynchronous interrupt transfer for
+  the device and endpoint.
+
+  @param  Ehc   The EHCI device.
+  @param  DevAddr   The device address.
+  @param  EpAddrEndpoint addrress & its direction.
+  @param  DevSpeed  The device speed.
+  @param  ToggleInitial data toggle to use.
+  @param  MaxPacket The max packet length of the endpoint.
+  @param  Hub   The transaction translator to use.
+  @param  DataLen   The length of data buffer.
+  @param  Callback  The function to call when data is transferred.
+  @param  Context   The context to the callback.
+  @param  Interval  The interval for interrupt transfer.
+
+  @return Created URB or NULL.
+
+**/
+URB *
+EhciInsertAsyncIntTransfer (
+  IN USB2_HC_DEV*Ehc,
+  IN UINT8  DevAddr,
+  IN UINT8  EpAddr,
+  IN UINT8  DevSpeed,
+  IN UINT8  Toggle,
+  IN UINTN  MaxPacket,
+  IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Hub,
+  IN UINTN  DataLen,
+  IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
+  IN VOID   *Context,
+  IN UINTN  Interval
+  )
+{
+  VOID  *Data;
+  URB   *Urb;
+
+  Data = AllocatePool (DataLen);
+
+  if (Data == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));
+return NULL;
+  }
+
+  Urb = EhcCreateUrb (
+  Ehc,
+  DevAddr,
+  EpAddr,
+  DevSpeed,
+  Toggle,
+  MaxPacket,
+  Hub,
+  EHC_INT_TRANSFER_ASYNC,
+  NULL,
+  Data,
+  DataLen,
+  Callback,
+  Context,
+  Interval
+  );
+
+  if (Urb == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
+gBS->FreePool (Data);
+return NULL;
+  }
+
+  //
+  // New asynchronous transfer must inserted to the head.
+  // Check the comments in EhcMoniteAsyncRequests
+  //
+  EhcLinkQhToPeriod (Ehc, Urb->Qh);
+  InsertHeadList (>AsyncIntTransfers, >UrbList);
+

[edk2] [PATCH V3 0/4] Remove unnecessary Map/Unmap in XhciDxe/EhciDxe for AsyncInterruptTransfer

2018-10-26 Thread Star Zeng

V3: Thanks for Hao's feedback.
1. Match function parameter name and description between
XhciSched.c and XhciSched.h, EhciSched.c and EhciSched.h.
2. Call XhcFreeUrb after XhcCreateTransferTrb fails in XhcCreateTrb.


V2: Thanks for Ray's feedback.
1. Add the missing "FreePool (Data);".
2. Remove the unnecessary indentation change.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

Please refer to the log message of each commit for more details.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Jiewen Yao 

Star Zeng (4):
  MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function
  MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function
  MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer
  MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c  |  28 +
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 114 +--
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.h |  35 +-
 MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c   |  38 ++-
 MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h   |  33 +++---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |  19 +---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 185 +--
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  55 +++--
 8 files changed, 296 insertions(+), 211 deletions(-)

-- 
2.7.0.windows.1

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

[edk2] [PATCH V3 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

2018-10-26 Thread Star Zeng

V3:
Call XhcFreeUrb after XhcCreateTransferTrb fails in XhcCreateTrb.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

In current code, XhcMonitorAsyncRequests (timer handler) will do
unmap and map operations for AsyncIntTransfers to "Flush data from
PCI controller specific address to mapped system memory address".
XhcMonitorAsyncRequests
  XhcFlushAsyncIntMap
PciIo->Unmap
  IoMmu->SetAttribute
PciIo->Map
  IoMmu->SetAttribute

This may impact the boot performance.

Since the data buffer for XhcMonitorAsyncRequests is internal
buffer, we can allocate common buffer by PciIo->AllocateBuffer
and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
then the unmap and map operations can be removed.

///
/// Provides both read and write access to system memory by
/// both the processor and a bus master. The buffer is coherent
/// from both the processor's and the bus master's point of view.
///
EfiPciIoOperationBusMasterCommonBuffer,

Test done:
USB KB works normally.
USB disk read/write works normally.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Ruiyu Ni 
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |   1 +
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 138 +++
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  27 +++---
 3 files changed, 63 insertions(+), 103 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 7f64f9c7c982..64855a4c158c 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -769,6 +769,7 @@ XhcTransfer (
   MaximumPacketLength,
   Type,
   Request,
+  FALSE,
   Data,
   *DataLength,
   NULL,
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 75959ae08363..9dd45e93a272 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -118,17 +118,18 @@ ON_EXIT:
 /**
   Create a new URB for a new transaction.
 
-  @param  Xhc   The XHCI Instance
-  @param  BusAddr   The logical device address assigned by UsbBus driver
-  @param  EpAddrEndpoint addrress
-  @param  DevSpeed  The device speed
-  @param  MaxPacket The max packet length of the endpoint
-  @param  Type  The transaction type
-  @param  Request   The standard USB request for control transfer
-  @param  Data  The user data to transfer
-  @param  DataLen   The length of data buffer
-  @param  Callback  The function to call when data is transferred
-  @param  Context   The context to the callback
+  @param  Xhc   The XHCI Instance
+  @param  BusAddr   The logical device address assigned by UsbBus 
driver
+  @param  EpAddrEndpoint addrress
+  @param  DevSpeed  The device speed
+  @param  MaxPacket The max packet length of the endpoint
+  @param  Type  The transaction type
+  @param  Request   The standard USB request for control transfer
+  @param  AllocateCommonBuffer  Indicate whether need to allocate common 
buffer for data transfer
+  @param  Data  The user data to transfer, NULL if 
AllocateCommonBuffer is TRUE
+  @param  DataLen   The length of data buffer
+  @param  Callback  The function to call when data is transferred
+  @param  Context   The context to the callback
 
   @return Created URB or NULL
 
@@ -142,6 +143,7 @@ XhcCreateUrb (
   IN UINTN  MaxPacket,
   IN UINTN  Type,
   IN EFI_USB_DEVICE_REQUEST *Request,
+  IN BOOLEANAllocateCommonBuffer,
   IN VOID   *Data,
   IN UINTN  DataLen,
   IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
@@ -169,8 +171,24 @@ XhcCreateUrb (
   Ep->Type  = Type;
 
   Urb->Request  = Request;
+  if (AllocateCommonBuffer) {
+ASSERT (Data == NULL);
+Status = Xhc->PciIo->AllocateBuffer (
+   Xhc->PciIo,
+   AllocateAnyPages,
+   EfiBootServicesData,
+   EFI_SIZE_TO_PAGES (DataLen),
+   ,
+   0
+   );
+if (EFI_ERROR (Status) || (Data == NULL)) {
+  FreePool (Urb);
+  return NULL;
+}
+  }
   Urb->Data = Data;
   Urb->DataLen  = DataLen;
+  Urb->AllocateCommonBuffer = AllocateCommonBuffer;
   Urb->Callback = Callback;
   Urb->Context  = Context;
 
@@ -178,7 +196,7 @@ XhcCreateUrb (
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "XhcCreateUrb: XhcCreateTransferTrb Failed, Status = 
%r\n", Status));
-FreePool (Urb);
+XhcFreeUrb (Xhc, Urb);
 Urb =

[edk2] [PATCH V3 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

2018-10-26 Thread Star Zeng

V3:
Match function parameter name and description between
XhciSched.c and XhciSched.h.

V2:
Add the missing "FreePool (Data);".
Remove the unnecessary indentation change.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

Extract new XhciInsertAsyncIntTransfer function from
XhcAsyncInterruptTransfer.

It is code preparation for following patch,
no essential functional change.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 18 +
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 65 
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 28 ++
 3 files changed, 94 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index f1c60bef01c0..7f64f9c7c982 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -1346,7 +1346,6 @@ XhcAsyncInterruptTransfer (
   EFI_STATUS  Status;
   UINT8   SlotId;
   UINT8   Index;
-  UINT8   *Data;
   EFI_TPL OldTpl;
 
   //
@@ -1413,36 +1412,21 @@ XhcAsyncInterruptTransfer (
 goto ON_EXIT;
   }
 
-  Data = AllocateZeroPool (DataLength);
-
-  if (Data == NULL) {
-DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to allocate 
buffer\n"));
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  }
-
-  Urb = XhcCreateUrb (
+  Urb = XhciInsertAsyncIntTransfer (
   Xhc,
   DeviceAddress,
   EndPointAddress,
   DeviceSpeed,
   MaximumPacketLength,
-  XHC_INT_TRANSFER_ASYNC,
-  NULL,
-  Data,
   DataLength,
   CallBackFunction,
   Context
   );
-
   if (Urb == NULL) {
-DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to create URB\n"));
-FreePool (Data);
 Status = EFI_OUT_OF_RESOURCES;
 goto ON_EXIT;
   }
 
-  InsertHeadList (>AsyncIntTransfers, >UrbList);
   //
   // Ring the doorbell
   //
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 166c44bf5e66..75959ae08363 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1411,6 +1411,71 @@ XhciDelAllAsyncIntTransfers (
 }
 
 /**
+  Insert a single asynchronous interrupt transfer for
+  the device and endpoint.
+
+  @param XhcThe XHCI Instance
+  @param BusAddrThe logical device address assigned by UsbBus driver
+  @param EpAddr Endpoint addrress
+  @param DevSpeed   The device speed
+  @param MaxPacket  The max packet length of the endpoint
+  @param DataLenThe length of data buffer
+  @param Callback   The function to call when data is transferred
+  @param ContextThe context to the callback
+
+  @return Created URB or NULL
+
+**/
+URB *
+XhciInsertAsyncIntTransfer (
+  IN USB_XHCI_INSTANCE  *Xhc,
+  IN UINT8  BusAddr,
+  IN UINT8  EpAddr,
+  IN UINT8  DevSpeed,
+  IN UINTN  MaxPacket,
+  IN UINTN  DataLen,
+  IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
+  IN VOID   *Context
+  )
+{
+  VOID  *Data;
+  URB   *Urb;
+
+  Data = AllocateZeroPool (DataLen);
+  if (Data == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));
+return NULL;
+  }
+
+  Urb = XhcCreateUrb (
+  Xhc,
+  BusAddr,
+  EpAddr,
+  DevSpeed,
+  MaxPacket,
+  XHC_INT_TRANSFER_ASYNC,
+  NULL,
+  Data,
+  DataLen,
+  Callback,
+  Context
+  );
+  if (Urb == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
+FreePool (Data);
+return NULL;
+  }
+
+  //
+  // New asynchronous transfer must inserted to the head.
+  // Check the comments in XhcMoniteAsyncRequests
+  //
+  InsertHeadList (>AsyncIntTransfers, >UrbList);
+
+  return Urb;
+}
+
+/**
   Update the queue head for next round of asynchronous transfer
 
   @param  Xhc The XHCI Instance.
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
index 097408828a1f..b5e192c3b589 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
@@ -853,6 +853,34 @@ XhciDelAllAsyncIntTransfers (
   );
 
 /**
+  Insert a single asynchronous interrupt transfer for
+  the device and endpoint.
+
+  @param XhcThe XHCI Instance
+  @param BusAddrThe logical device address assigned by UsbBus driver
+  @param EpAddr Endpoint addrress
+  @param DevSpeed   The device speed
+  @param MaxPacket  The

[edk2] [PATCH V3 4/4] MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

2018-10-26 Thread Star Zeng

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

In current code, EhcMonitorAsyncRequests (timer handler) will do
unmap and map operations for AsyncIntTransfers to "Flush data from
PCI controller specific address to mapped system memory address".
EhcMonitorAsyncRequests
  EhcFlushAsyncIntMap
PciIo->Unmap
  IoMmu->SetAttribute
PciIo->Map
  IoMmu->SetAttribute

This may impact the boot performance.

Since the data buffer for EhcMonitorAsyncRequests is internal
buffer, we can allocate common buffer by PciIo->AllocateBuffer
and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
then the unmap and map operations can be removed.

///
/// Provides both read and write access to system memory by
/// both the processor and a bus master. The buffer is coherent
/// from both the processor's and the bus master's point of view.
///
EfiPciIoOperationBusMasterCommonBuffer,

Test done:
USB KB works normally.
USB disk read/write works normally.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Ruiyu Ni 
Reviewed-by: Hao Wu 
---
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c  |  3 ++
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 78 +---
 MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c   | 38 ++--
 MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h   | 33 --
 4 files changed, 57 insertions(+), 95 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c 
b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 5569f4f9618b..764eeda58ba1 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -763,6 +763,7 @@ EhcControlTransfer (
   Translator,
   EHC_CTRL_TRANSFER,
   Request,
+  FALSE,
   Data,
   *DataLength,
   NULL,
@@ -906,6 +907,7 @@ EhcBulkTransfer (
   Translator,
   EHC_BULK_TRANSFER,
   NULL,
+  FALSE,
   Data[0],
   *DataLength,
   NULL,
@@ -1163,6 +1165,7 @@ EhcSyncInterruptTransfer (
   Translator,
   EHC_INT_TRANSFER_SYNC,
   NULL,
+  FALSE,
   Data,
   *DataLength,
   NULL,
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c 
b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
index ec8d796fab11..b067fd02d1ce 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
@@ -778,7 +778,6 @@ EhciDelAsyncIntTransfer (
   EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);
   RemoveEntryList (>UrbList);
 
-  gBS->FreePool (Urb->Data);
   EhcFreeUrb (Ehc, Urb);
   return EFI_SUCCESS;
 }
@@ -809,7 +808,6 @@ EhciDelAllAsyncIntTransfers (
 EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);
 RemoveEntryList (>UrbList);
 
-gBS->FreePool (Urb->Data);
 EhcFreeUrb (Ehc, Urb);
   }
 }
@@ -848,16 +846,8 @@ EhciInsertAsyncIntTransfer (
   IN UINTN  Interval
   )
 {
-  VOID  *Data;
   URB   *Urb;
 
-  Data = AllocatePool (DataLen);
-
-  if (Data == NULL) {
-DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));
-return NULL;
-  }
-
   Urb = EhcCreateUrb (
   Ehc,
   DevAddr,
@@ -868,7 +858,8 @@ EhciInsertAsyncIntTransfer (
   Hub,
   EHC_INT_TRANSFER_ASYNC,
   NULL,
-  Data,
+  TRUE,
+  NULL,
   DataLen,
   Callback,
   Context,
@@ -877,7 +868,6 @@ EhciInsertAsyncIntTransfer (
 
   if (Urb == NULL) {
 DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
-gBS->FreePool (Data);
 return NULL;
   }
 
@@ -892,60 +882,6 @@ EhciInsertAsyncIntTransfer (
 }
 
 /**
-  Flush data from PCI controller specific address to mapped system
-  memory address.
-
-  @param  EhcThe EHCI device.
-  @param  UrbThe URB to unmap.
-
-  @retval EFI_SUCCESSSuccess to flush data to mapped system memory.
-  @retval EFI_DEVICE_ERROR   Fail to flush data to mapped system memory.
-
-**/
-EFI_STATUS
-EhcFlushAsyncIntMap (
-  IN  USB2_HC_DEV *Ehc,
-  IN  URB *Urb
-  )
-{
-  EFI_STATUSStatus;
-  EFI_PHYSICAL_ADDRESS  PhyAddr;
-  EFI_PCI_IO_PROTOCOL_OPERATION MapOp;
-  EFI_PCI_IO_PROTOCOL   *PciIo;
-  UINTN Len;
-  VOID  *Map;
-
-  PciIo = Ehc->PciIo;
-  Len   = Urb->DataLen;
-
-  if (Urb->Ep.Direction == EfiUsbDataIn) {
-MapOp = EfiPciIoOperationBusMasterWrite;
-  } else {
-MapOp = EfiPciIoOperationBusMasterRead;
-  }
-
-  Status = PciIo->Unmap (PciIo, Urb->DataMap);
-  if (EFI_ERROR (Status)) {
-goto ON_ERROR;
-  }
-
-  Urb->DataMap = NULL;
-
-  Status = PciIo->Map (PciIo, MapOp, Urb->Data, , , );
-  if (EFI_ERROR (Status) || (Len != Urb->DataLen)) {
-goto ON_ERROR;
-  }
-
-  Urb->DataPhy  = (VOID *) ((UINTN) PhyAddr);
-

Re: [edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

2018-10-26 Thread Paulo Alcantara

Hao Wu  writes:

> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1279
>
> The commit will add 3 types of checks for function ResolveSymlink():
>
> A. Check for the value of 'Component Type' field within a Path Component
>
> According to the ECMA-167 standard (3rd Edition - June 1997), Section
> 14.16.1.1, valid values are 1 to 5. All other values will be treated as a
> corrupted volume.
>
> B. Check for the content pointed by 'File'
>
> Since content within 'File' is the output data for ResolveSymlink().
> Checks is added to ensure the content in 'File' is valid. Otherwise,
> possible null pointer dereference issue will occur during the subsequent
> usage of the data returned by ResolveSymlink().
>
> C. Check for possible memory double free/use after free case
>
> For codes:
>
> if (CompareMem ((VOID *), (VOID *)Parent,
> sizeof (UDF_FILE_INFO)) != 0) {
>   CleanupFileInformation ();
> }
>
> CopyMem ((VOID *), (VOID *)File, sizeof (UDF_FILE_INFO));
>
> If the contents in 'PreviousFile' and 'File' are the same, call to
> "CleanupFileInformation ();" will free the buffers in 'File'
> as well. This will lead to potential memory double free/use after free
> issues.
>
> Cc: Paulo Alcantara 
> Cc: Paulo Alcantara 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 30 
> --
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c 
> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> index b9ebddfe62..a89e5ba9ff 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> @@ -2145,6 +2145,8 @@ ResolveSymlink (
>UINT8   CompressionId;
>UDF_FILE_INFO   PreviousFile;
>  
> +  ZeroMem ((VOID *)File, sizeof (UDF_FILE_INFO));
> +
>//
>// Symlink files on UDF volumes do not contain so much data other than
>// Path Components which resolves to real filenames, so it's OK to read in
> @@ -2257,6 +2259,13 @@ ResolveSymlink (
>}
>FileName[Index] = L'\0';
>break;
> +default:
> +  //
> +  // Accoring to the ECMA-167 standard (3rd Edition - June 1997), Section
> +  // 14.16.1.1, all other values are reserved.
> +  //
> +  Status = EFI_VOLUME_CORRUPTED;
> +  goto Error_Find_File;
>  }
>  
>  //
> @@ -2281,8 +2290,18 @@ ResolveSymlink (
>break;
>  }
>  
> -if (CompareMem ((VOID *), (VOID *)Parent,
> -sizeof (UDF_FILE_INFO)) != 0) {
> +//
> +// Check the content in the file info pointed by File.
> +//
> +if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
> +  Status = EFI_VOLUME_CORRUPTED;
> +  goto Error_Find_File;
> +}
> +
> +if ((CompareMem ((VOID *), (VOID *)Parent,
> +sizeof (UDF_FILE_INFO)) != 0) &&
> +(CompareMem ((VOID *), (VOID *)File,
> +sizeof (UDF_FILE_INFO)) != 0)) {
>CleanupFileInformation ();
>  }
>  
> @@ -2294,6 +2313,13 @@ ResolveSymlink (
>//
>FreePool (ReadFileInfo.FileData);
>  
> +  //
> +  // Check the content in the resolved file info.
> +  //
> +  if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
> +return EFI_VOLUME_CORRUPTED;
> +  }
> +
>return EFI_SUCCESS;
>  
>  Error_Find_File:

Reviewed-by: Paulo Alcantara 

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

[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: optimize TLB flush operation

2018-10-26 Thread Jian J Wang

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1281

This optimization has two purpose:

  1. fix BZ#1281 which caused by flushing TLB for AP
  2. improve performance for SMM heap guard

The code change is simple: just flush TLB for current processor.

Since processor's (including AP) SMI entry code will always initialize
CR3, it looks like that there's no need to add extra code in AP handler,
called from SMI entry, to flush TLB again.

Let each processor itself guarantee the TLB integrity can improve memory
operations performance if Heap Guard is enabled. This has been proved
by CpuDxe driver. Please check following patches for details.

  41a9c3fd110bed93c4fdf088eea18412bb2dfcde
  0dbb0f1a5ce6a9ec5213c85e5d4244cf5b061417
Stop flush TLB for APs (DXE) upon change

  199de89677deff30eda7ad17793b30042cce
Let AP (DXE) flush TLB in its wake-up procedure

Tests:
  a. Verified that issue in BZ#1281 is gone
  b. Verified SMM heap guard works well on any processor
  c. OVMF boot (Fedora26, Ubuntu18.04, Windows 10)

Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 45 +++---
 1 file changed, 6 insertions(+), 39 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 684b14dc28..e0bf0cd5ac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -464,41 +464,6 @@ ConvertMemoryPageAttributes (
   return RETURN_SUCCESS;
 }
 
-/**
-  FlushTlb on current processor.
-
-  @param[in,out] Buffer  Pointer to private data buffer.
-**/
-VOID
-EFIAPI
-FlushTlbOnCurrentProcessor (
-  IN OUT VOID  *Buffer
-  )
-{
-  CpuFlushTlb ();
-}
-
-/**
-  FlushTlb for all processors.
-**/
-VOID
-FlushTlbForAll (
-  VOID
-  )
-{
-  UINTN   Index;
-
-  FlushTlbOnCurrentProcessor (NULL);
-
-  for (Index = 0; Index < gSmst->NumberOfCpus; Index++) {
-if (Index != gSmst->CurrentlyExecutingCpu) {
-  // Force to start up AP in blocking mode,
-  SmmBlockingStartupThisAp (FlushTlbOnCurrentProcessor, Index, NULL);
-  // Do not check return status, because AP might not be present in some 
corner cases.
-}
-  }
-}
-
 /**
   This function sets the attributes for the memory region specified by 
BaseAddress and
   Length from their current attributes to the attributes specified by 
Attributes.
@@ -538,9 +503,10 @@ SmmSetMemoryAttributesEx (
   if (!EFI_ERROR(Status)) {
 if (IsModified) {
   //
-  // Flush TLB as last step
+  // Flush TLB as last step. No need to do it for APs, which sould take 
care
+  // of it in the wake-up procedure.
   //
-  FlushTlbForAll();
+  CpuFlushTlb ();
 }
   }
 
@@ -586,9 +552,10 @@ SmmClearMemoryAttributesEx (
   if (!EFI_ERROR(Status)) {
 if (IsModified) {
   //
-  // Flush TLB as last step
+  // Flush TLB as last step. No need to do it for APs, which sould take 
care
+  // of it in the wake-up procedure.
   //
-  FlushTlbForAll();
+  CpuFlushTlb ();
 }
   }
 
-- 
2.16.2.windows.1

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

Re: [edk2] [PATCH v5 0/1] Add ArmPkg/Optee library APIs

2018-10-26 Thread Sumit Garg

Hi Leif,

Please let me know if you have any further review comment.

-Sumit

On Mon, 22 Oct 2018 at 12:00, Sumit Garg  wrote:
>
> Changes in v5:
> Define RFC4122_UUID struct using network byte order instead of 16 byte
> octects for passing Trusted Application UUID.
>
> Changes in v4:
> Replaced abbreviations with full name which are not defined in [1]. Also
> used EFI_GUID for Trusted Application UUIDs.
>
> [1] 
> https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/4_naming_conventions/#table-2-efi-supported-abbreviations
>
> Changes in v3:
> Removed GlobalPlatform TEE return codes (IndustryStandard/GlobalPlatform.h)
> that were rejected by EDK2 maintainers. Rather used custom ones for this
> OP-TEE driver.
>
> Changes in v2:
> 1. Separate patch for MdePkg/Include/IndustryStandard/GlobalPlatform.h.
> 2. Correct comments style for struct members.
> 3. Update commit message.
>
> Sumit Garg (1):
>   ArmPkg/OpteeLib: Add APIs to communicate with OP-TEE
>
>  ArmPkg/Library/OpteeLib/OpteeLib.inf |   2 +
>  ArmPkg/Include/Library/OpteeLib.h|  88 +
>  ArmPkg/Library/OpteeLib/OpteeSmc.h   |  53 +++
>  ArmPkg/Library/OpteeLib/Optee.c  | 392 
>  4 files changed, 535 insertions(+)
>  create mode 100644 ArmPkg/Library/OpteeLib/OpteeSmc.h
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] uefi-sct/SctPkg:Add conformance test cases for deprecated EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute.

2018-10-26 Thread Leif Lindholm

Laszlo, many thanks for this.

Cc:ing some new(ish) maintainers.

/
Leif

On Wed, Oct 17, 2018 at 02:04:01PM +0200, Laszlo Ersek wrote:
> Hi Leif,
> 
> On 10/16/18 04:28, Leif Lindholm wrote:
> 
> > Laszlo: a few years ago, you also posted a _really_ useful email about
> > the process of being a maintainer, and helpful workflows (like "sort
> > emails to review immediately on reception, even if you don't have time
> > to review now"). I have since failed to find it in my history (or via
> > google). Since you're very organised - do you have it lying around,
> > and if so would you be able to re-post it?
> 
> I'm sure I still have the email, if I sent it; however, I can't find it
> now, because I don't personally remember the *specific* email you refer
> to, and so I can't come up with good search terms, for the program that
> indexes my mailbox ("recoll").
> 
> Anyway, what I generally do is:
> 
> - I maintain a set of tagged emails that present work items. This set
> (of emails) exists in addition to bugzillas that are assigned to me.
> 
> - I process emails in batches. I sync my mailbox, and then turn off
> synching, until I have covered everything possible, at that point, that
> relate to my mailbox.
> 
> - In every new batch in my INBOX, I go over the emails as quickly as
> possible, trying to triage emails as I go. If I can take care of an
> email immediately, I do. If it needs more work (especially focused
> work), then I tag the email, and archive it at once. If the tagged email
> is private (not on any list), then I'll let it sit tagged in my personal
> archive folder. If the email is also on some list, then I might choose
> to tag the email in that folder instead, and archive the personal copy
> without tagging it.
> 
> - I go over all the new emails in my list folders as well (i.e. emails
> that I'm not personally CC'd on). Dependent on list, I dedicate
> different amounts of attention.
> 
> - Once I'm done triaging / tagging the new batch (and therefore I have
> no unread messages in my entire mailbox), I search my mailbox for all
> tagged messages. I usually list those hits in chronological order, but
> not always (e.g., sometimes I group them by containing folder). I work
> my way through these items slowly. Importantly, I shut out interruptions
> while I do this -- no more email synching, no phone, no IRC. And the
> FIFO processing order of the tagged messages mostly ensures good
> responsiveness from my side, despite this process being quite OK at
> throughput as well.
> 
> (Your present email fell in the "take care of it at once" category :) )
> 
> Thanks,
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Community Meeting Minutes #2

2018-10-26 Thread stephano


Improving the Patch System
--
We would like to streamline the workflow of our patches making it easier 
for current and future developers to contribute while discouraging 
careless code submissions. We will review several possibilities 
including Fabricator, Github, Gitlab, and Gerrit. We will review these 
solutions by examining case studies as well as community aided research. 
Key goals that we wish to achieve:

1. Offline search of both patches AND comments/conversations
2. Automation of tasks (e.g. lint, merge-fail detection)
3. Automatic notifications (e.g. patch reject, comment added, patch 
pushed to master)


It was made clear that these goals should be implicit in the software, 
not something that we need to script or maintain. Rebecca Cran 
volunteered to help with Fabricator evaluation (thank you), and I will 
be heading up research on the remaining (more volunteers welcome :) ).


Standard C Types

The only benefit we see to adding Standard C Types might be when we pull 
in modules like LZMA or FDT. We see no reason not to add them, but we 
are hesitant to remove our current type system as it works well and the 
community is comfortable using them. We should have a code standard in 
place to require the use of one or the other, but not both.


Using Submodules

Assuming our deviations are not large we should follow a pattern of 
using submodules rather than pulling in code directly.


Project Mu
--
https://microsoft.github.io/mu/

Project Mu is a way of laying out TianoCore into layers, submodules, and 
repos, allowing multiple architectures to share the same core, 
facilitating efficiency and speeding time to market.


https://github.com/Microsoft/mu_basecore

Basecore is the minimum package set that someone would need to build a 
UEFI product.


https://github.com/Microsoft/mu_plus

Mu Plus is an example of optional features, specifically for surface, 
building upon the basecore layer. The Mu Plus repo includes features 
like DSCI (management of remote UEFI settings, targeted at client 
devices). Other groups might add similar layers to enable different 
features.


In the documentation, please see the "MinPlatform Example" and the 
"Surface Example"


To facilitate creating a build environment, they have created a wrapper 
around the current build system.


Liming mentioned that there is currently an area for wrappers in 
BaseTools/Scripts:

https://github.com/tianocore/edk2/tree/master/BaseTools/Scripts

What is the difference between mu_basecore and edk2 master branch?
--addition of large features not yet upstreamed (in progress)
--small changes that need resources to upstream (time constraint)
--rearranging things into different repos to facilitate layering. See: 
https://microsoft.github.io/mu/WhatAndWhy/layout/



Community TODO List
---
Jeremiah: please check to see if the Mu wrapper script is publicly 
available.


Stephano: head up the review of patch systems and write up a report for 
each detailing the pro's & con's, and collect community feedback.



Thank you all for your participation. I will be creating a survey to 
determine the best day & time to meet, and will setup a recurring 
meeting. As always, please feel free to contact me directly with 
comments / questions.


Cheers,
Stephano


Stephano Cetola
TianoCore Community Manager
stephano.cet...@linux.intel.com


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

[edk2] [PATCH 1/1] BaseTools: Sync MdePkg/Library/UefiDevicePathLib/DevicePathToText.c code

2018-10-26 Thread Feng, YunhuaX

Sync MdePkg/Library/UefiDevicePathLib/DevicePathToText.c code

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/C/DevicePath/DevicePath.c |  2 +-
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 59 ++
 BaseTools/Source/C/Include/Protocol/DevicePath.h   |  2 +-
 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c 
b/BaseTools/Source/C/DevicePath/DevicePath.c
index 956bbffb5f..356f5f7e24 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -23,11 +23,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 // Utility version information
 //
 #define UTILITY_MAJOR_VERSION 0
 #define UTILITY_MINOR_VERSION 1
 
-EFI_GUID gEfiDebugPortDevicePathGuid = DEVICE_PATH_MESSAGING_DEBUGPORT;
+EFI_GUID gEfiDebugPortProtocolGuid = DEVICE_PATH_MESSAGING_DEBUGPORT;
 EFI_GUID gEfiPcAnsiGuid = EFI_PC_ANSI_GUID;
 EFI_GUID gEfiVT100Guid = EFI_VT_100_GUID;
 EFI_GUID gEfiVT100PlusGuid = EFI_VT_100_PLUS_GUID;
 EFI_GUID gEfiVTUTF8Guid = EFI_VT_UTF8_GUID;
 EFI_GUID gEfiUartDevicePathGuid = EFI_UART_DEVICE_PATH_GUID;
diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c 
b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
index bb74e2e170..555efa1acd 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
@@ -762,11 +762,20 @@ DevPathFromTextAcpiExp (
   ACPI_EXTENDED_DP,
   Length
   );
 
   AcpiEx->HID = EisaIdFromText (HIDStr);
-  AcpiEx->CID = EisaIdFromText (CIDStr);
+  //
+  // According to UEFI spec, the CID parametr is optional and has a default 
value of 0.
+  // So when the CID parametr is not specified or specified as 0 in the text 
device node.
+  // Set the CID to 0 in the ACPI extension device path structure.
+  //
+  if (*CIDStr == L'\0' || *CIDStr == L'0') {
+AcpiEx->CID = 0;
+  } else {
+AcpiEx->CID = EisaIdFromText (CIDStr);
+  }
   AcpiEx->UID = 0;
 
   AsciiStr = (CHAR8 *) ((UINT8 *)AcpiEx + sizeof 
(ACPI_EXTENDED_HID_DEVICE_PATH));
   //
   // HID string is NULL
@@ -1601,19 +1610,19 @@ DevPathFromTextEmmc (
 EFI_DEVICE_PATH_PROTOCOL *
 DevPathFromTextDebugPort (
CHAR16 *TextDeviceNode
   )
 {
-  VENDOR_DEFINED_MESSAGING_DEVICE_PATH  *Vend;
+  VENDOR_DEVICE_PATH  *Vend;
 
-  Vend = (VENDOR_DEFINED_MESSAGING_DEVICE_PATH *) CreateDeviceNode (
+  Vend = (VENDOR_DEVICE_PATH *) CreateDeviceNode (
 MESSAGING_DEVICE_PATH,
 MSG_VENDOR_DP,
-(UINT16) sizeof 
(VENDOR_DEFINED_MESSAGING_DEVICE_PATH)
+(UINT16) sizeof 
(VENDOR_DEVICE_PATH)
 );
 
-  CopyGuid (>Guid, );
+  CopyGuid (>Guid, );
 
   return (EFI_DEVICE_PATH_PROTOCOL *) Vend;
 }
 
 /**
@@ -1904,26 +1913,46 @@ ConvertFromTextUsbClass (
 
   VIDStr  = GetNextParamStr ();
   PIDStr  = GetNextParamStr ();
   if (UsbClassText->ClassExist) {
 ClassStr = GetNextParamStr ();
-UsbClass->DeviceClass = (UINT8) Strtoi (ClassStr);
+if (*ClassStr == L'\0') {
+  UsbClass->DeviceClass = 0xFF;
+} else {
+  UsbClass->DeviceClass = (UINT8) Strtoi (ClassStr);
+}
   } else {
 UsbClass->DeviceClass = UsbClassText->Class;
   }
   if (UsbClassText->SubClassExist) {
 SubClassStr = GetNextParamStr ();
-UsbClass->DeviceSubClass = (UINT8) Strtoi (SubClassStr);
+if (*SubClassStr == L'\0') {
+  UsbClass->DeviceSubClass = 0xFF;
+} else {
+  UsbClass->DeviceSubClass = (UINT8) Strtoi (SubClassStr);
+}
   } else {
 UsbClass->DeviceSubClass = UsbClassText->SubClass;
   }
 
   ProtocolStr = GetNextParamStr ();
 
-  UsbClass->VendorId= (UINT16) Strtoi (VIDStr);
-  UsbClass->ProductId   = (UINT16) Strtoi (PIDStr);
-  UsbClass->DeviceProtocol  = (UINT8) Strtoi (ProtocolStr);
+  if (*VIDStr == L'\0') {
+UsbClass->VendorId= 0x;
+  } else {
+UsbClass->VendorId= (UINT16) Strtoi (VIDStr);
+  }
+  if (*PIDStr == L'\0') {
+UsbClass->ProductId   = 0x;
+  } else {
+UsbClass->ProductId   = (UINT16) Strtoi (PIDStr);
+  }
+  if (*ProtocolStr == L'\0') {
+UsbClass->DeviceProtocol  = 0xFF;
+  } else {
+UsbClass->DeviceProtocol  = (UINT8) Strtoi (ProtocolStr);
+  }
 
   return (EFI_DEVICE_PATH_PROTOCOL *) UsbClass;
 }
 
 
@@ -3259,11 +3288,19 @@ DevPathFromTextSata (
 MESSAGING_DEVICE_PATH,
 MSG_SATA_DP,
 (UINT16) sizeof (SATA_DEVICE_PATH)

Re: [edk2] [PATCH edk2-platforms v5 21/28] Platform/Hisilicon/D06: Add PciHostBridgeLib

2018-10-26 Thread Ming Huang

Hi Ard & Laszlo,

Sorry for delay reply.
Should all host bridges use EISA_PNP_ID(0x0A03)?

Ming

On 10/12/2018 4:08 PM, Laszlo Ersek wrote:
> On 10/12/18 09:29, Ard Biesheuvel wrote:
>> Hello all,
>>
>> While grepping through the code in edk2-platforms, I noticed an issue
>> with this commit. Apologies for not spotting it earlier.
>>
>> On 31 August 2018 at 15:27, Ming Huang  wrote:
>>> PciHostBridgeLib which is need by PciHostBridgeDxe, provide
>>> root bridges and deal with resource conflict.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ming Huang 
>>> Reviewed-by: Leif Lindholm 
>>> ---
>>>  Platform/Hisilicon/D06/D06.dsc   |   2 
>>> +-
>>>  Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf |  36 
>>> ++
>>>  Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c   | 635 
>>> 
>>>  3 files changed, 672 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
>>> index 2659cb7e37..83dcbab6c4 100644
>>> --- a/Platform/Hisilicon/D06/D06.dsc
>>> +++ b/Platform/Hisilicon/D06/D06.dsc
>>> @@ -419,7 +419,7 @@
>>>  
>>>
>>> PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>>>PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
>>> -  
>>> PciHostBridgeLib|MdeModulePkg/Library/PciHostBridgeLibNull/PciHostBridgeLibNull.inf
>>> +  
>>> PciHostBridgeLib|Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>>>}
>>>
>>>MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>>> diff --git 
>>> a/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf 
>>> b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>>> new file mode 100644
>>> index 00..8a998681a3
>>> --- /dev/null
>>> +++ b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>>> @@ -0,0 +1,36 @@
>>> +## @file
>>> +#
>>> +#  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>>> +#  Copyright (c) 2018, Linaro Limited. 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.
>>> +#
>>> +#
>>> +##
>>> +
>>> +[Defines]
>>> +  INF_VERSION= 0x0001001A
>>> +  BASE_NAME  = PciHostBridgeLib
>>> +  FILE_GUID  = 61b7276a-fc67-11e5-82fd-47ea9896dd5d
>>> +  MODULE_TYPE= DXE_DRIVER
>>> +  VERSION_STRING = 1.0
>>> +  LIBRARY_CLASS  = PciHostBridgeLib|DXE_DRIVER
>>> +
>>> +[Sources]
>>> +  PciHostBridgeLib.c
>>> +
>>> +[Packages]
>>> +  MdeModulePkg/MdeModulePkg.dec
>>> +  MdePkg/MdePkg.dec
>>> +
>>> +[LibraryClasses]
>>> +  BaseLib
>>> +  DebugLib
>>> +  DevicePathLib
>>> +  MemoryAllocationLib
>>> +  UefiBootServicesTableLib
>>> diff --git 
>>> a/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c 
>>> b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c
>>> new file mode 100644
>>> index 00..d1a436d9bc
>>> --- /dev/null
>>> +++ b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c
>>> @@ -0,0 +1,635 @@
>>> +/** @file
>>> +
>>> +  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>>> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
>>> +
>>> +  This program and the accompanying materials
>>> +  are licensed and made available under the terms and conditions of the 
>>> BSD License
>>> +  which accompanies this distribution.  The full text of the license may 
>>> be found at
>>> +  http://opensource.org/licenses/bsd-license.php
>>> +
>>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>>> IMPLIED.
>>> +
>>> +**/
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#define ENUM_HB_NUM 8
>>> +
>>> +#define EFI_PCI_SUPPORT   (EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | \
>>> +   EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO | \
>>> +   EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | \
>>> +   EFI_PCI_ATTRIBUTE_ISA_IO_16  | \
>>> +   EFI_PCI_ATTRIBUTE_VGA_MEMORY | \
>>> +   EFI_PCI_ATTRIBUTE_VGA_IO_16  | \
>>> +   EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16)
>>> +
>>> +#define EFI_PCI_ATTRIBUTE  EFI_PCI_SUPPORT
>>> +
>>> +#pragma pack(1)
>>> +typedef struct {
>>> +

Re: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

2018-10-26 Thread Wu, Hao A

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wu, Hao A
> Sent: Friday, October 26, 2018 3:38 PM
> To: Zeng, Star; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Yao, Jiewen; Zeng, Star
> Subject: Re: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common
> buffer for AsyncInterruptTransfer
> 
> Reviewed-by: Hao Wu 

Sorry, missed one question for this patch.
Please see below for more detail:

> 
> Best Regards,
> Hao Wu
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Star Zeng
> > Sent: Friday, October 26, 2018 1:42 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu; Wu, Hao A; Yao, Jiewen; Zeng, Star
> > Subject: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common
> buffer
> > for AsyncInterruptTransfer
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> >
> > In current code, XhcMonitorAsyncRequests (timer handler) will do
> > unmap and map operations for AsyncIntTransfers to "Flush data from
> > PCI controller specific address to mapped system memory address".
> > XhcMonitorAsyncRequests
> >   XhcFlushAsyncIntMap
> > PciIo->Unmap
> >   IoMmu->SetAttribute
> > PciIo->Map
> >   IoMmu->SetAttribute
> >
> > This may impact the boot performance.
> >
> > Since the data buffer for XhcMonitorAsyncRequests is internal
> > buffer, we can allocate common buffer by PciIo->AllocateBuffer
> > and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
> > then the unmap and map operations can be removed.
> >
> > ///
> > /// Provides both read and write access to system memory by
> > /// both the processor and a bus master. The buffer is coherent
> > /// from both the processor's and the bus master's point of view.
> > ///
> > EfiPciIoOperationBusMasterCommonBuffer,
> >
> > Test done:
> > USB KB works normally.
> > USB disk read/write works normally.
> >
> > Cc: Ruiyu Ni 
> > Cc: Hao Wu 
> > Cc: Jian J Wang 
> > Cc: Jiewen Yao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Star Zeng 
> > Reviewed-by: Ruiyu Ni 
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |   1 +
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 141 +++---
> --
> > ---
> >  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  27 +++---
> >  3 files changed, 67 insertions(+), 102 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > index 7f64f9c7c982..64855a4c158c 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> > @@ -769,6 +769,7 @@ XhcTransfer (
> >MaximumPacketLength,
> >Type,
> >Request,
> > +  FALSE,
> >Data,
> >*DataLength,
> >NULL,
> > diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> > index 75959ae08363..d03a6681ce0d 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> > +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> > @@ -118,17 +118,18 @@ ON_EXIT:
> >  /**
> >Create a new URB for a new transaction.
> >
> > -  @param  Xhc   The XHCI Instance
> > -  @param  BusAddr   The logical device address assigned by UsbBus driver
> > -  @param  EpAddrEndpoint addrress
> > -  @param  DevSpeed  The device speed
> > -  @param  MaxPacket The max packet length of the endpoint
> > -  @param  Type  The transaction type
> > -  @param  Request   The standard USB request for control transfer
> > -  @param  Data  The user data to transfer
> > -  @param  DataLen   The length of data buffer
> > -  @param  Callback  The function to call when data is transferred
> > -  @param  Context   The context to the callback
> > +  @param  Xhc   The XHCI Instance
> > +  @param  BusAddr   The logical device address assigned by 
> > UsbBus
> > driver
> > +  @param  EpAddrEndpoint addrress
> > +  @param  DevSpeed  The device speed
> > +  @param  MaxPacket The max packet length of the endpoint
> > +  @param  Type  The transaction type
> > +  @param  Request   The standard USB request for control 
> > transfer
> > +  @param  AllocateCommonBuffer  Indicate whether need to allocate
> > common buffer for data transfer
> > +  @param  Data  The user data to transfer, NULL if
> > AllocateCommonBuffer is TRUE
> > +  @param  DataLen   The length of data buffer
> > +  @param  Callback  The function to call when data is 
> > transferred
> > +  @param  Context   The context to the callback
> >
> >@return Created URB or NULL
> >
> > @@ -142,6 +143,7 @@ XhcCreateUrb (
> >IN UINTN  MaxPacket,
> >IN UINTN  Type,
> >IN EFI_USB_DEVICE_REQUEST *Request,
> > +  IN BOOLEAN

Re: [edk2] [PATCH V2 4/4] MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

2018-10-26 Thread Wu, Hao A

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, October 26, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Wu, Hao A; Yao, Jiewen; Zeng, Star
> Subject: [edk2] [PATCH V2 4/4] MdeModulePkg EhciDxe: Use common buffer
> for AsyncInterruptTransfer
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> In current code, EhcMonitorAsyncRequests (timer handler) will do
> unmap and map operations for AsyncIntTransfers to "Flush data from
> PCI controller specific address to mapped system memory address".
> EhcMonitorAsyncRequests
>   EhcFlushAsyncIntMap
> PciIo->Unmap
>   IoMmu->SetAttribute
> PciIo->Map
>   IoMmu->SetAttribute
> 
> This may impact the boot performance.
> 
> Since the data buffer for EhcMonitorAsyncRequests is internal
> buffer, we can allocate common buffer by PciIo->AllocateBuffer
> and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
> then the unmap and map operations can be removed.
> 
> ///
> /// Provides both read and write access to system memory by
> /// both the processor and a bus master. The buffer is coherent
> /// from both the processor's and the bus master's point of view.
> ///
> EfiPciIoOperationBusMasterCommonBuffer,
> 
> Test done:
> USB KB works normally.
> USB disk read/write works normally.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> Reviewed-by: Ruiyu Ni 
> ---
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c  |  3 ++
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 78 +-
> --
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c   | 38 ++--
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h   | 33 --
>  4 files changed, 57 insertions(+), 95 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> index 5569f4f9618b..764eeda58ba1 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> @@ -763,6 +763,7 @@ EhcControlTransfer (
>Translator,
>EHC_CTRL_TRANSFER,
>Request,
> +  FALSE,
>Data,
>*DataLength,
>NULL,
> @@ -906,6 +907,7 @@ EhcBulkTransfer (
>Translator,
>EHC_BULK_TRANSFER,
>NULL,
> +  FALSE,
>Data[0],
>*DataLength,
>NULL,
> @@ -1163,6 +1165,7 @@ EhcSyncInterruptTransfer (
>Translator,
>EHC_INT_TRANSFER_SYNC,
>NULL,
> +  FALSE,
>Data,
>*DataLength,
>NULL,
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> index 2d202d439d1c..f1edcf20e342 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> @@ -778,7 +778,6 @@ EhciDelAsyncIntTransfer (
>EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);
>RemoveEntryList (>UrbList);
> 
> -  gBS->FreePool (Urb->Data);
>EhcFreeUrb (Ehc, Urb);
>return EFI_SUCCESS;
>  }
> @@ -809,7 +808,6 @@ EhciDelAllAsyncIntTransfers (
>  EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);
>  RemoveEntryList (>UrbList);
> 
> -gBS->FreePool (Urb->Data);
>  EhcFreeUrb (Ehc, Urb);
>}
>  }
> @@ -849,16 +847,8 @@ EhciInsertAsyncIntTransfer (
>IN UINTN  Interval
>)
>  {
> -  VOID  *Data;
>URB   *Urb;
> 
> -  Data = AllocatePool (DataLen);
> -
> -  if (Data == NULL) {
> -DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n",
> __FUNCTION__));
> -return NULL;
> -  }
> -
>Urb = EhcCreateUrb (
>Ehc,
>DevAddr,
> @@ -869,7 +859,8 @@ EhciInsertAsyncIntTransfer (
>Hub,
>EHC_INT_TRANSFER_ASYNC,
>NULL,
> -  Data,
> +  TRUE,
> +  NULL,
>DataLen,
>Callback,
>Context,
> @@ -878,7 +869,6 @@ EhciInsertAsyncIntTransfer (
> 
>if (Urb == NULL) {
>  DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
> -gBS->FreePool (Data);
>  return NULL;
>}
> 
> @@ -893,60 +883,6 @@ EhciInsertAsyncIntTransfer (
>  }
> 
>  /**
> -  Flush data from PCI controller specific address to mapped system
> -  memory address.
> -
> -  @param  EhcThe EHCI device.
> -  @param  UrbThe URB to unmap.
> -
> -  @retval EFI_SUCCESSSuccess to flush data to mapped system memory.
> -  @retval EFI_DEVICE_ERROR   Fail to flush data to mapped system memory.
> -
> -**/
> -EFI_STATUS
> -EhcFlushAsyncIntMap (
> -  IN  USB2_HC_DEV *Ehc,
> -  IN  URB *Urb
> -  )
> -{
> -  EFI_STATUSStatus;
> -  EFI_PHYSICAL_ADDRESS  PhyAddr;
> -

Re: [edk2] [PATCH] MdeModulePkg/Core: fix an IA32 build failure

2018-10-26 Thread Wang, Jian J

Thanks. Since this failure blocks others work, I'll check in this patch soon.

Regards,
Jian


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng,
> Star
> Sent: Friday, October 26, 2018 1:13 PM
> To: edk2-devel ; edk2-devel@lists.01.org
> Cc: Zeng, Star ; Gao, Liming 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/Core: fix an IA32 build failure
> 
> Reviewed-by: Star Zeng 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org]
> Sent: Friday, October 26, 2018 12:54 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> Subject: [edk2] [PATCH] MdeModulePkg/Core: fix an IA32 build failure
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1277
> 
> The failure is caused by data type conversion between UINTN and UINT64,
> which is checked in at 63ebde8ef6d4ff497d054ccc010904ecd4441198.
> 
> Cc: Star Zeng 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> index 449a022658..521e0d7b2a 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> @@ -463,7 +463,7 @@ IsGuardPage (
>IN EFI_PHYSICAL_ADDRESSAddress
>)
>  {
> -  UINTN   BitMap;
> +  UINT64BitMap;
> 
>//
>// There must be at least one guarded page before and/or after given
> @@ -1368,7 +1368,7 @@ GuardAllFreedPages (
>UINT64Address;
>UINT64GuardPage;
>INTN  Level;
> -  UINTN BitIndex;
> +  UINT64BitIndex;
>UINTN GuardPageNumber;
> 
>if (mGuardedMemoryMap == 0 ||
> @@ -1475,12 +1475,12 @@ MergeGuardPages (
>}
> 
>Bitmap = 0;
> -  Pages  = EFI_SIZE_TO_PAGES (MaxAddress - MemoryMapEntry->PhysicalStart);
> -  Pages -= MemoryMapEntry->NumberOfPages;
> +  Pages  = EFI_SIZE_TO_PAGES ((UINTN)(MaxAddress - MemoryMapEntry-
> >PhysicalStart));
> +  Pages -= (INTN)MemoryMapEntry->NumberOfPages;
>while (Pages > 0) {
>  if (Bitmap == 0) {
>EndAddress = MemoryMapEntry->PhysicalStart +
> -   EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages);
> +   EFI_PAGES_TO_SIZE ((UINTN)MemoryMapEntry->NumberOfPages);
>Bitmap = GetGuardedMemoryBits (EndAddress,
> GUARDED_HEAP_MAP_ENTRY_BITS);
>  }
> 
> --
> 2.19.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V2 2/4] MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function

2018-10-26 Thread Wu, Hao A

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, October 26, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Wu, Hao A; Yao, Jiewen; Zeng, Star
> Subject: [edk2] [PATCH V2 2/4] MdeModulePkg EhciDxe: Extract new
> EhciInsertAsyncIntTransfer function
> 
> V2:
> Add the missing "gBS->FreePool (Data);".
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> Extract new EhciInsertAsyncIntTransfer function from
> EhcAsyncInterruptTransfer.
> 
> It is code preparation for following patch,
> no essential functional change.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c  | 25 +--
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c | 77
> 
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.h | 36 ++-
>  3 files changed, 113 insertions(+), 25 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> index 50b5598df4fb..5569f4f9618b 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> @@ -997,7 +997,6 @@ EhcAsyncInterruptTransfer (
>URB *Urb;
>EFI_TPL OldTpl;
>EFI_STATUS  Status;
> -  UINT8   *Data;
> 
>//
>// Validate parameters
> @@ -1046,16 +1045,7 @@ EhcAsyncInterruptTransfer (
> 
>EhcAckAllInterrupt (Ehc);
> 
> -  Data = AllocatePool (DataLength);
> -
> -  if (Data == NULL) {
> -DEBUG ((EFI_D_ERROR, "EhcAsyncInterruptTransfer: failed to allocate
> buffer\n"));
> -
> -Status = EFI_OUT_OF_RESOURCES;
> -goto ON_EXIT;
> -  }
> -
> -  Urb = EhcCreateUrb (
> +  Urb = EhciInsertAsyncIntTransfer (
>Ehc,
>DeviceAddress,
>EndPointAddress,
> @@ -1063,9 +1053,6 @@ EhcAsyncInterruptTransfer (
>*DataToggle,
>MaximumPacketLength,
>Translator,
> -  EHC_INT_TRANSFER_ASYNC,
> -  NULL,
> -  Data,
>DataLength,
>CallBackFunction,
>Context,
> @@ -1073,20 +1060,10 @@ EhcAsyncInterruptTransfer (
>);
> 
>if (Urb == NULL) {
> -DEBUG ((EFI_D_ERROR, "EhcAsyncInterruptTransfer: failed to create
> URB\n"));
> -
> -gBS->FreePool (Data);
>  Status = EFI_OUT_OF_RESOURCES;
>  goto ON_EXIT;
>}
> 
> -  //
> -  // New asynchronous transfer must inserted to the head.
> -  // Check the comments in EhcMoniteAsyncRequests
> -  //
> -  EhcLinkQhToPeriod (Ehc, Urb->Qh);
> -  InsertHeadList (>AsyncIntTransfers, >UrbList);
> -
>  ON_EXIT:
>Ehc->PciIo->Flush (Ehc->PciIo);
>gBS->RestoreTPL (OldTpl);
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> index 168280be81d7..2d202d439d1c 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> @@ -814,6 +814,83 @@ EhciDelAllAsyncIntTransfers (
>}
>  }
> 
> +/**
> +  Insert a single asynchronous interrupt transfer for
> +  the device and endpoint.
> +
> +  @param  Ehc   The EHCI device.
> +  @param  DevAddr   The device address.
> +  @param  EpAddrEndpoint addrress & its direction.
> +  @param  DevSpeed  The device speed.
> +  @param  ToggleInitial data toggle to use.
> +  @param  MaxPacket The max packet length of the endpoint.
> +  @param  Hub   The transaction translator to use.
> +  @param  Data  The user data to transfer.
> +  @param  DataLen   The length of data buffer.
> +  @param  Callback  The function to call when data is transferred.
> +  @param  Context   The context to the callback.
> +  @param  Interval  The interval for interrupt transfer.
> +
> +  @return Created URB or NULL.
> +
> +**/
> +URB *
> +EhciInsertAsyncIntTransfer (
> +  IN USB2_HC_DEV*Ehc,
> +  IN UINT8  DevAddr,
> +  IN UINT8  EpAddr,
> +  IN UINT8  DevSpeed,
> +  IN UINT8  Toggle,
> +  IN UINTN  MaxPacket,
> +  IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Hub,
> +  IN UINTN  DataLen,
> +  IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
> +  IN VOID   *Context,
> +  IN UINTN  Interval
> +  )
> +{
> +  VOID  *Data;
> +  URB   *Urb;
> +
> +  Data = AllocatePool (DataLen);
> +
> +  if (Data == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n",
> __FUNCTION__));
> +return NULL;
> +  }
> +
> +  Urb = EhcCreateUrb (
> +  Ehc,
> +  DevAddr,
>

[edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

2018-10-26 Thread Hao Wu

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1279

The commit will add 3 types of checks for function ResolveSymlink():

A. Check for the value of 'Component Type' field within a Path Component

According to the ECMA-167 standard (3rd Edition - June 1997), Section
14.16.1.1, valid values are 1 to 5. All other values will be treated as a
corrupted volume.

B. Check for the content pointed by 'File'

Since content within 'File' is the output data for ResolveSymlink().
Checks is added to ensure the content in 'File' is valid. Otherwise,
possible null pointer dereference issue will occur during the subsequent
usage of the data returned by ResolveSymlink().

C. Check for possible memory double free/use after free case

For codes:

if (CompareMem ((VOID *), (VOID *)Parent,
sizeof (UDF_FILE_INFO)) != 0) {
  CleanupFileInformation ();
}

CopyMem ((VOID *), (VOID *)File, sizeof (UDF_FILE_INFO));

If the contents in 'PreviousFile' and 'File' are the same, call to
"CleanupFileInformation ();" will free the buffers in 'File'
as well. This will lead to potential memory double free/use after free
issues.

Cc: Paulo Alcantara 
Cc: Paulo Alcantara 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c 
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index b9ebddfe62..a89e5ba9ff 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -2145,6 +2145,8 @@ ResolveSymlink (
   UINT8   CompressionId;
   UDF_FILE_INFO   PreviousFile;
 
+  ZeroMem ((VOID *)File, sizeof (UDF_FILE_INFO));
+
   //
   // Symlink files on UDF volumes do not contain so much data other than
   // Path Components which resolves to real filenames, so it's OK to read in
@@ -2257,6 +2259,13 @@ ResolveSymlink (
   }
   FileName[Index] = L'\0';
   break;
+default:
+  //
+  // Accoring to the ECMA-167 standard (3rd Edition - June 1997), Section
+  // 14.16.1.1, all other values are reserved.
+  //
+  Status = EFI_VOLUME_CORRUPTED;
+  goto Error_Find_File;
 }
 
 //
@@ -2281,8 +2290,18 @@ ResolveSymlink (
   break;
 }
 
-if (CompareMem ((VOID *), (VOID *)Parent,
-sizeof (UDF_FILE_INFO)) != 0) {
+//
+// Check the content in the file info pointed by File.
+//
+if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+  Status = EFI_VOLUME_CORRUPTED;
+  goto Error_Find_File;
+}
+
+if ((CompareMem ((VOID *), (VOID *)Parent,
+sizeof (UDF_FILE_INFO)) != 0) &&
+(CompareMem ((VOID *), (VOID *)File,
+sizeof (UDF_FILE_INFO)) != 0)) {
   CleanupFileInformation ();
 }
 
@@ -2294,6 +2313,13 @@ ResolveSymlink (
   //
   FreePool (ReadFileInfo.FileData);
 
+  //
+  // Check the content in the resolved file info.
+  //
+  if ((File->FileEntry == NULL) || (File->FileIdentifierDesc == NULL)) {
+return EFI_VOLUME_CORRUPTED;
+  }
+
   return EFI_SUCCESS;
 
 Error_Find_File:
-- 
2.12.0.windows.1

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

Re: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

2018-10-26 Thread Wu, Hao A

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, October 26, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Wu, Hao A; Yao, Jiewen; Zeng, Star
> Subject: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer
> for AsyncInterruptTransfer
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> In current code, XhcMonitorAsyncRequests (timer handler) will do
> unmap and map operations for AsyncIntTransfers to "Flush data from
> PCI controller specific address to mapped system memory address".
> XhcMonitorAsyncRequests
>   XhcFlushAsyncIntMap
> PciIo->Unmap
>   IoMmu->SetAttribute
> PciIo->Map
>   IoMmu->SetAttribute
> 
> This may impact the boot performance.
> 
> Since the data buffer for XhcMonitorAsyncRequests is internal
> buffer, we can allocate common buffer by PciIo->AllocateBuffer
> and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
> then the unmap and map operations can be removed.
> 
> ///
> /// Provides both read and write access to system memory by
> /// both the processor and a bus master. The buffer is coherent
> /// from both the processor's and the bus master's point of view.
> ///
> EfiPciIoOperationBusMasterCommonBuffer,
> 
> Test done:
> USB KB works normally.
> USB disk read/write works normally.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> Reviewed-by: Ruiyu Ni 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  |   1 +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 141 +++-
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  27 +++---
>  3 files changed, 67 insertions(+), 102 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 7f64f9c7c982..64855a4c158c 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -769,6 +769,7 @@ XhcTransfer (
>MaximumPacketLength,
>Type,
>Request,
> +  FALSE,
>Data,
>*DataLength,
>NULL,
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 75959ae08363..d03a6681ce0d 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -118,17 +118,18 @@ ON_EXIT:
>  /**
>Create a new URB for a new transaction.
> 
> -  @param  Xhc   The XHCI Instance
> -  @param  BusAddr   The logical device address assigned by UsbBus driver
> -  @param  EpAddrEndpoint addrress
> -  @param  DevSpeed  The device speed
> -  @param  MaxPacket The max packet length of the endpoint
> -  @param  Type  The transaction type
> -  @param  Request   The standard USB request for control transfer
> -  @param  Data  The user data to transfer
> -  @param  DataLen   The length of data buffer
> -  @param  Callback  The function to call when data is transferred
> -  @param  Context   The context to the callback
> +  @param  Xhc   The XHCI Instance
> +  @param  BusAddr   The logical device address assigned by UsbBus
> driver
> +  @param  EpAddrEndpoint addrress
> +  @param  DevSpeed  The device speed
> +  @param  MaxPacket The max packet length of the endpoint
> +  @param  Type  The transaction type
> +  @param  Request   The standard USB request for control transfer
> +  @param  AllocateCommonBuffer  Indicate whether need to allocate
> common buffer for data transfer
> +  @param  Data  The user data to transfer, NULL if
> AllocateCommonBuffer is TRUE
> +  @param  DataLen   The length of data buffer
> +  @param  Callback  The function to call when data is transferred
> +  @param  Context   The context to the callback
> 
>@return Created URB or NULL
> 
> @@ -142,6 +143,7 @@ XhcCreateUrb (
>IN UINTN  MaxPacket,
>IN UINTN  Type,
>IN EFI_USB_DEVICE_REQUEST *Request,
> +  IN BOOLEANAllocateCommonBuffer,
>IN VOID   *Data,
>IN UINTN  DataLen,
>IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
> @@ -169,8 +171,24 @@ XhcCreateUrb (
>Ep->Type  = Type;
> 
>Urb->Request  = Request;
> +  if (AllocateCommonBuffer) {
> +ASSERT (Data == NULL);
> +Status = Xhc->PciIo->AllocateBuffer (
> +   Xhc->PciIo,
> +   AllocateAnyPages,
> +   EfiBootServicesData,
> +   EFI_SIZE_TO_PAGES (DataLen),
> +   ,
> +   0
> +

Re: [edk2] [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

2018-10-26 Thread Wu, Hao A

Hi Star,

The interface for function XhciInsertAsyncIntTransfer() does not match between
XhciSched.c and XhciSched.h.

Other than that:
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: Zeng, Star
> Sent: Friday, October 26, 2018 1:42 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star; Ni, Ruiyu; Wu, Hao A; Wang, Jian J; Yao, Jiewen
> Subject: [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new
> XhciInsertAsyncIntTransfer function
> 
> V2:
> Add the missing "FreePool (Data);".
> Remove the unnecessary indentation change.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274
> 
> Extract new XhciInsertAsyncIntTransfer function from
> XhcAsyncInterruptTransfer.
> 
> It is code preparation for following patch,
> no essential functional change.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c  | 18 +
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 65
> 
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 28 ++
>  3 files changed, 94 insertions(+), 17 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index f1c60bef01c0..7f64f9c7c982 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -1346,7 +1346,6 @@ XhcAsyncInterruptTransfer (
>EFI_STATUS  Status;
>UINT8   SlotId;
>UINT8   Index;
> -  UINT8   *Data;
>EFI_TPL OldTpl;
> 
>//
> @@ -1413,36 +1412,21 @@ XhcAsyncInterruptTransfer (
>  goto ON_EXIT;
>}
> 
> -  Data = AllocateZeroPool (DataLength);
> -
> -  if (Data == NULL) {
> -DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to allocate
> buffer\n"));
> -Status = EFI_OUT_OF_RESOURCES;
> -goto ON_EXIT;
> -  }
> -
> -  Urb = XhcCreateUrb (
> +  Urb = XhciInsertAsyncIntTransfer (
>Xhc,
>DeviceAddress,
>EndPointAddress,
>DeviceSpeed,
>MaximumPacketLength,
> -  XHC_INT_TRANSFER_ASYNC,
> -  NULL,
> -  Data,
>DataLength,
>CallBackFunction,
>Context
>);
> -
>if (Urb == NULL) {
> -DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: failed to create
> URB\n"));
> -FreePool (Data);
>  Status = EFI_OUT_OF_RESOURCES;
>  goto ON_EXIT;
>}
> 
> -  InsertHeadList (>AsyncIntTransfers, >UrbList);
>//
>// Ring the doorbell
>//
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 166c44bf5e66..75959ae08363 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -1411,6 +1411,71 @@ XhciDelAllAsyncIntTransfers (
>  }
> 
>  /**
> +  Insert a single asynchronous interrupt transfer for
> +  the device and endpoint.
> +
> +  @param XhcThe XHCI Instance
> +  @param BusAddrThe logical device address assigned by UsbBus driver
> +  @param EpAddr Endpoint addrress
> +  @param DevSpeed   The device speed
> +  @param MaxPacket  The max packet length of the endpoint
> +  @param DataLenThe length of data buffer
> +  @param Callback   The function to call when data is transferred
> +  @param ContextThe context to the callback
> +
> +  @return Created URB or NULL
> +
> +**/
> +URB *
> +XhciInsertAsyncIntTransfer (
> +  IN USB_XHCI_INSTANCE  *Xhc,
> +  IN UINT8  BusAddr,
> +  IN UINT8  EpAddr,
> +  IN UINT8  DevSpeed,
> +  IN UINTN  MaxPacket,
> +  IN UINTN  DataLen,
> +  IN EFI_ASYNC_USB_TRANSFER_CALLBACKCallback,
> +  IN VOID   *Context
> +  )
> +{
> +  VOID  *Data;
> +  URB   *Urb;
> +
> +  Data = AllocateZeroPool (DataLen);
> +  if (Data == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n",
> __FUNCTION__));
> +return NULL;
> +  }
> +
> +  Urb = XhcCreateUrb (
> +  Xhc,
> +  BusAddr,
> +  EpAddr,
> +  DevSpeed,
> +  MaxPacket,
> +  XHC_INT_TRANSFER_ASYNC,
> +  NULL,
> +  Data,
> +  DataLen,
> +  Callback,
> +  Context
> +  );
> +  if (Urb == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
> +FreePool (Data);
> +return NULL;
> +  }
> +
> +  //
> +  // New asynchronous transfer must inserted to the head.
> +  // Check the comments in XhcMoniteAsyncRequests
> +  //
> +  InsertHeadList (>AsyncIntTransfers, >UrbList);
> +
> +  return Urb;
> +}
> +
> +/**
>Update the queue head for next round of asynchronous

[edk2] [PATCH] IntelFsp2Pkg: Fixed potentially NULL pointer accessing

2018-10-26 Thread Chasel, Chiu

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1280

When copying IDT table in SecMain, the pointer might be
NULL so added the check to fix it.

Test: Verified on internal platform and boots successfully.

Cc: Jiewen Yao 
Cc: Desimone Nathaniel L 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu 
---
 IntelFsp2Pkg/FspSecCore/SecMain.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c 
b/IntelFsp2Pkg/FspSecCore/SecMain.c
index f319c68cc5..aed8893ff0 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -113,8 +113,14 @@ SecStartup (
   // ERROR: IDT table size from boot loader is larger than FSP can 
support, DeadLoop here!
   //
   CpuDeadLoop();
+} else if (IdtDescriptor.Base == 0)  {
+  //
+  // ERROR: IDT table Base should not be zero, DeadLoop here!
+  //
+  CpuDeadLoop();
+} else {
+  CopyMem ((VOID *) (UINTN) , (VOID *) 
IdtDescriptor.Base, IdtSize);
 }
-CopyMem ((VOID *) (UINTN) , (VOID *) 
IdtDescriptor.Base, IdtSize);
   }
   IdtDescriptor.Base  = (UINTN) 
   IdtDescriptor.Limit = (UINT16)(IdtSize - 1);
-- 
2.13.3.windows.1

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

Re: [edk2] [PATCH v2 0/4] SdMmcOverride extension

2018-10-26 Thread Wu, Hao A

> -Original Message-
> From: Marcin Wojtas [mailto:m...@semihalf.com]
> Sent: Thursday, October 25, 2018 8:43 PM
> To: Wu, Hao A
> Cc: edk2-devel-01; Tian, Feng; Kinney, Michael D; Gao, Liming; Leif Lindholm;
> Ard Biesheuvel; nad...@marvell.com; j...@semihalf.com; Tomasz Michalec
> Subject: Re: [PATCH v2 0/4] SdMmcOverride extension
> 
> Hi Hao,
> 
> Were you able to find time for evaluating my patchset?

Hi Marcin,

I plan to start looking into your series sometime next week.
I will give you some updates then.

Best Regards,
Hao Wu

> 
> Best regards,
> Marcin
> pt., 12 paź 2018 o 14:50 Marcin Wojtas  napisał(a):
> >
> > pt., 12 paź 2018 o 14:48 Wu, Hao A  napisał(a):
> > >
> > > > -Original Message-
> > > > From: Marcin Wojtas [mailto:m...@semihalf.com]
> > > > Sent: Friday, October 12, 2018 1:33 PM
> > > > To: Wu, Hao A
> > > > Cc: edk2-devel-01; Tian, Feng; Kinney, Michael D; Gao, Liming; Leif
> Lindholm;
> > > > Ard Biesheuvel; nad...@marvell.com; j...@semihalf.com; Tomasz
> Michalec
> > > > Subject: Re: [PATCH v2 0/4] SdMmcOverride extension
> > > >
> > > > Hi Hao,
> > > >
> > > > pt., 12 paź 2018 o 07:25 Wu, Hao A  napisał(a):
> > > > >
> > > > > Hi Marcin,
> > > > >
> > > > > Please grant me some time for this series.
> > > > >
> > > > > Since I found that the extension of the SdMmc override protocol
> (mainly
> > > > > the 3rd and 4th patch of the series) may have something overlaps
> with a
> > > > > (internal) request to configure the driver strength parameter and
> operating
> > > > > clock frequency of the SD/EMMC devices.
> > > > >
> > > > > For the (driver strength/operating freq) customize, we already have a
> > > > > proposal on the way. So I am wondering if you could grant me some
> time
> > > > to
> > > > > investigate whether both the cases can be addressed together based
> on
> > > > your
> > > > > proposed patch.
> > > > >
> > > >
> > > > Sure. I'm only wondering if it's not best to collect all remarks and
> > > > maybe update to v3 both edk2 and edk2-platforms sides (so far the
> > > > issues have been not critical, such as typos, parameters' names,
> > > > etc.). In the meantime you would be able to validate if the solution
> > > > is sufficient for you as well. What do you think? When do you expect
> > > > to be able to look at it vs your internal requirements more deeply?
> > > >
> > > > Best regards,
> > > > Marcin
> > >
> > > I think you can hold the new version of the patch if the feedbacks do not
> > > lead to considerable changes. At this moment, I can barely take time for
> > > the evaluation. I think I will be able to fully shift to this in about 2
> > > weeks. Does it sound acceptable to you with regard to the urgency level
> > > for the series?
> > >
> > > I will try my best to move up the process. Sorry again for the possible
> > > delay.
> >
> > I will proceed with other remaining items for my platforms and allow
> > myself to ping you about status around end of October :)
> >
> > Best regards,
> > Marcin
> >
> > >
> > > >
> > > >
> > > > > Thanks in advance.
> > > > >
> > > > > Best Regards,
> > > > > Hao Wu
> > > > >
> > > > >
> > > > > > -Original Message-
> > > > > > From: Marcin Wojtas [mailto:m...@semihalf.com]
> > > > > > Sent: Friday, October 05, 2018 9:25 PM
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Tian, Feng; Kinney, Michael D; Gao, Liming;
> leif.lindh...@linaro.org;
> > > > Wu,
> > > > > > Hao A; ard.biesheu...@linaro.org; nad...@marvell.com;
> > > > > > m...@semihalf.com; j...@semihalf.com; t...@semihalf.com
> > > > > > Subject: [PATCH v2 0/4] SdMmcOverride extension
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This is the second version of the patchset. Initial one was
> > > > > > interleaved with the fixes, which after split got already merged.
> > > > > > The biggest change is - resigning from the new callbacks
> > > > > > and extending parameter lists of both NotifyPhase and Capability
> > > > > > routines.
> > > > > >
> > > > > > Patches are available in the github:
> > > > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > > > platform/commits/sdmmc-override-upstream-r20181005
> > > > > >
> > > > > > Please note that extending SdMmcOverride protocol was impacting
> > > > > > so far the only user of it (Synquacer controller). In paralel
> > > > > > edk2-platforms patchset, a patch can be found:
> > > > > > ("Silicon/SynQuacer/PlatformDxe: adjust to updated
> SdMmcOverride")
> > > > > > which immunizes for above and future extensions of the protocol:
> > > > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > > > platform/commits/xenon-upstream-r20181005
> > > > > >
> > > > > > I'm looking forward to the comments and remarks.
> > > > > >
> > > > > > Best regards,
> > > > > > Marcin
> > > > > >
> > > > > > Changelog:
> > > > > > v1 -> v2
> > > > > > * Rebase onto newest master
> > > > > > * 1/4 [new patch] - preparation for extending NotifyPhase
> > > > > > * 2/4 -

Re: [edk2] [PATCH 1/1] BaseTools: Roll back code modify by commit 9e47e6f90880

2018-10-26 Thread Gao, Liming

Reviewed-by: Liming Gao 

Because some platform has been broken, I will push this patch soon. 

> -Original Message-
> From: Feng, YunhuaX
> Sent: Friday, October 26, 2018 2:08 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong ; Gao, Liming 
> Subject: [PATCH 1/1] BaseTools: Roll back code modify by commit 9e47e6f90880
> 
> Roll back code modify by commit 9e47e6f90880,
> if ForceRebase not False or True, the GenFv command not need add parameter
> 
> Cc: Liming Gao 
> Cc: Yonghong Zhu 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yunhua Feng 
> ---
>  BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py 
> b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
> index e867693d7c..ea61f723a7 100644
> --- a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
> +++ b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
> @@ -572,13 +572,13 @@ class GenFdsGlobalVariable:
> 
>  Cmd = ["GenFv"]
>  if BaseAddress:
>  Cmd += ("-r", BaseAddress)
> 
> -if not ForceRebase:
> +if ForceRebase == False:
>  Cmd += ("-F", "FALSE")
> -else:
> +elif ForceRebase == True:
>  Cmd += ("-F", "TRUE")
> 
>  if Capsule:
>  Cmd.append("-c")
>  if Dump:
> --
> 2.12.2.windows.2

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

[edk2] [PATCH 1/1] BaseTools: Roll back code modify by commit 9e47e6f90880

2018-10-26 Thread Feng, YunhuaX

Roll back code modify by commit 9e47e6f90880,
if ForceRebase not False or True, the GenFv command not need add parameter

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py 
b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
index e867693d7c..ea61f723a7 100644
--- a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
+++ b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
@@ -572,13 +572,13 @@ class GenFdsGlobalVariable:
 
 Cmd = ["GenFv"]
 if BaseAddress:
 Cmd += ("-r", BaseAddress)
 
-if not ForceRebase:
+if ForceRebase == False:
 Cmd += ("-F", "FALSE")
-else:
+elif ForceRebase == True:
 Cmd += ("-F", "TRUE")
 
 if Capsule:
 Cmd.append("-c")
 if Dump:
-- 
2.12.2.windows.2

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

Re: [edk2] [PATCH] MdeModulePkg/Core: correct one coding style

[edk2] [PATCH] MdeModulePkg/Core: correct one coding style

[edk2] Test message. Please ignore.

[edk2] Example of email Phabricator can be configured to send on svn commit

[edk2] [Patch] BaseTools: Rollback Filter out unused structure pcds

Re: [edk2] [PATCH] IntelFsp2Pkg: Fixed potentially NULL pointer accessing

Re: [edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

[edk2] Grub Error Marvell Board

Re: [edk2] Community Meeting Minutes #2

Re: [edk2] [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

[edk2] [PATCH V3 2/4] MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function

[edk2] [PATCH V3 0/4] Remove unnecessary Map/Unmap in XhciDxe/EhciDxe for AsyncInterruptTransfer

[edk2] [PATCH V3 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

[edk2] [PATCH V3 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

[edk2] [PATCH V3 4/4] MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

Re: [edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: optimize TLB flush operation

Re: [edk2] [PATCH v5 0/1] Add ArmPkg/Optee library APIs

Re: [edk2] [PATCH] uefi-sct/SctPkg:Add conformance test cases for deprecated EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute.

[edk2] Community Meeting Minutes #2

[edk2] [PATCH 1/1] BaseTools: Sync MdePkg/Library/UefiDevicePathLib/DevicePathToText.c code

Re: [edk2] [PATCH edk2-platforms v5 21/28] Platform/Hisilicon/D06: Add PciHostBridgeLib

Re: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

Re: [edk2] [PATCH V2 4/4] MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

Re: [edk2] [PATCH] MdeModulePkg/Core: fix an IA32 build failure

Re: [edk2] [PATCH V2 2/4] MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function

[edk2] [PATCH] MdeModulePkg/UdfDxe: Additional checks for ResolveSymlink()

Re: [edk2] [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

Re: [edk2] [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function

[edk2] [PATCH] IntelFsp2Pkg: Fixed potentially NULL pointer accessing

Re: [edk2] [PATCH v2 0/4] SdMmcOverride extension

Re: [edk2] [PATCH 1/1] BaseTools: Roll back code modify by commit 9e47e6f90880

[edk2] [PATCH 1/1] BaseTools: Roll back code modify by commit 9e47e6f90880

33 matches

Site Navigation

Mail list logo

Footer information