date:20170918

Re: [edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Udit Kumar

Thanks Vladimir, 
With your design, you did delayed write to eMMC due to sharing with OS.  But it 
works for you:)
Say if eMMC controllers offers you a status bit, if eMMC storage is being used 
for not. Then this 
could be possible to update at run time, both OS/UEFI needs to check and wait 
if controller is being used.
For sure,  some synchronization issues need to be ironed out (or maybe I am 
just dreaming here). 

On part 2) where you forked VariableRuntime driver , could we think of updating 
VariableRuntime driver, 
to support non-XIP or memory mapped devices. 

Thanks
Udit 

> -Original Message-
> From: Vladimir Olovyannikov [mailto:vladimir.olovyanni...@broadcom.com]
> Sent: Monday, September 18, 2017 10:22 PM
> To: Ard Biesheuvel ; Udit Kumar
> 
> Cc: grant.lik...@linaro.org; edk2-devel@lists.01.org; olivier.mar...@arm.com
> Subject: RE: [edk2] Storing Non volatile variables on SD/NAND
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Monday, September 18, 2017 8:43 AM
> > To: Udit Kumar
> > Cc: grant.lik...@linaro.org.; edk2-devel@lists.01.org;
> > olivier.mar...@arm.com
> > Subject: Re: [edk2] Storing Non volatile variables on SD/NAND
> >
> > On 18 September 2017 at 06:52, Udit Kumar  wrote:
> > > Hi EDK-2 Experts,
> > > I am looking to store NV variables on SD/NAND device.
> > >
> > > While browsing, I came across some old post at link,
> > > http://feishare.com/efimail/messages/20130319-1700-
> > Re__edk2__Regarding
> > > _storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html
> > >
> > > Looks like, this is possible easily.
> >
> > That's a bold statement dude :-)
> >
> > >>> What you need to support Non-Volatile UEFI variables is a
> Non-Volatile
> > Memory. And also a driver that implements the EFI Firmware Volume
> > Block protocol for this NVM device.
> > >
> > > But MdeModulePkg does Copymem from NV variable start memory to
> > some
> > > allocated buffers.  With SD/NAND Copymem is not possible, Is this
> > > something changes since 2013 or there are some other way to use
> > > SD/NAND
> > >
> >
> > No, SD/MMC cannot currently be used as the backing store for the EFI
> > variable store. The problem is that the variable protocols are
> architectural
> > protocols in PI that need to be present before any driver model
> > drivers
> are
> > dispatched, and so putting the variable store on block devices is not
> > something that the PI software architecture currently supports (unless
> you
> > reimplement the whole driver stack as DXE drivers).
> >
> > On top of that, it is almost impossible to share a block device that
> sits behind
> > a controller between the firmware and the OS at runtime (i.e., for
> > SetVariable() calls made by efibootmgr under Linux), because only a
> single
> > agent can take ownership of the controller at any given time. (You
> /could/
> > dedicate the SD/MMC to the firmware entirely, and boot from SATA or
> > USB, but this is out of the question on most platforms that need to
> > use
> SD/MMC
> > for that variable backing store, i.e., mobile platforms)
> >
> > The best thing would be for you to convince the hardware architects in
> your
> > company to design and implement dual-ported SD/MMC controllers that
> > allow a single SD/MMC to have two logical views that are independent
> > (although I'm unsure if that is even possible in the context of the
> SD/MMC
> > specifications)
> >
> > Thanks,
> > Ard.
> Udit,
> Ard is absolutely right.
> 
> Following design I had to implement variable store on the EMMC boot partition
> 1 (not exactly SD card, but close; the same is applied to NAND I guess).
> To do that I forked VariableRuntime driver and changed it to do cache writes
> into the store (just modifying store memory) at runtime.
> This is because you cannot share eMMC store between OS (Linux)  and the UEFI
> service at the same time, and you cannot predict when OS writes/reads to/from
> the EMMC device.
> So I do cache writes at runtime (just modifying store in memory), and flush 
> the
> store on reboot/shutdown.
> For this purpose my ResetSystemLib calls into the Variable service and forces
> flush. Variable service reinitiates eMMC controller and really writes into the
> store on the eMMC. Real flush is also performed on ExitBootServices().
> 
> As a big drawback an OS should either reset or shutdown (get into UEFI
> ResetSystemLib) in order for variable store to be updated physically; Physical
> flush is also forced on ExitBootServices - be it a reset command, or booting 
> an
> OS.
> Just pressing a reset button (example: Linux Kernel panic with no reset) would
> cause variable changes for this session to get lost.
> 
> Thank you,
> Vladimir
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> >

Re: [edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Udit Kumar

Thanks Ard / Jeremy. 

Say SD/NAND is just used for UEFI firmware then this is something doable. 

> > Having the firmware/variable store and OS root/boot on the same device
> > is fundamentally flawed.

I agree in case of SD/NAND, partitioning and synchronization with OS will be 
tough task. 
Even in case of NOR flash, if this is exposed as mtd device as well, then 
accessing from 
OS and UEFI poses same challenge .

Thanks
Udit

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, September 19, 2017 2:23 AM
> To: Jeremy Linton 
> Cc: Udit Kumar ; grant.lik...@linaro.org.
> ; edk2-devel@lists.01.org; olivier.mar...@arm.com
> Subject: Re: [edk2] Storing Non volatile variables on SD/NAND
> 
> On 18 September 2017 at 13:47, Jeremy Linton 
> wrote:
> > On 09/18/2017 10:43 AM, Ard Biesheuvel wrote:
> >>
> >> On 18 September 2017 at 06:52, Udit Kumar  wrote:
> >>>
> >>> Hi EDK-2 Experts,
> >>> I am looking to store NV variables on SD/NAND device.
> >>>
> >>> While browsing, I came across some old post at link,
> >>>
> >>> http://feishare.com/efimail/messages/20130319-1700-Re__edk2__Regardi
> >>> ng_storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.ht
> >>> ml
> >>>
> >>> Looks like, this is possible easily.
> >>
> >>
> >> That's a bold statement dude :-)
> >>
> > What you need to support Non-Volatile UEFI variables is a
> > Non-Volatile Memory. And also a driver that implements the EFI
> > Firmware Volume Block protocol for this NVM device.
> >>>
> >>>
> >>> But MdeModulePkg does Copymem from NV variable start memory to some
> >>> allocated buffers.  With SD/NAND Copymem is not possible, Is this
> >>> something changes since 2013 or there are some other way to use
> >>> SD/NAND
> >>>
> >>
> >> No, SD/MMC cannot currently be used as the backing store for the EFI
> >> variable store. The problem is that the variable protocols are
> >> architectural protocols in PI that need to be present before any
> >> driver model drivers are dispatched, and so putting the variable
> >> store on block devices is not something that the PI software
> >> architecture currently supports (unless you reimplement the whole
> >> driver stack as DXE drivers).
> >>
> >> On top of that, it is almost impossible to share a block device that
> >> sits behind a controller between the firmware and the OS at runtime
> >> (i.e., for SetVariable() calls made by efibootmgr under Linux),
> >> because only a single agent can take ownership of the controller at
> >> any given time. (You /could/ dedicate the SD/MMC to the firmware
> >> entirely, and boot from SATA or USB, but this is out of the question
> >> on most platforms that need to use SD/MMC for that variable backing
> >> store, i.e., mobile platforms)
> >>
> >> The best thing would be for you to convince the hardware architects
> >> in your company to design and implement dual-ported SD/MMC
> >> controllers that allow a single SD/MMC to have two logical views that
> >> are independent (although I'm unsure if that is even possible in the
> >> context of the SD/MMC specifications)
> >
> >
> > Which still has the problems of selecting "use whole disk" during an
> > OS install bricking the machine. Or similarly if the emmc layout isn't
> > just right having gparted automatically "fix" the partition table (as
> > it does with many of the hikey images) again bricking the machine.
> >
> > Having the firmware/variable store and OS root/boot on the same device
> > is fundamentally flawed. I've went down the path of simply disabling
> > the hikey/emmc for use beyond the firmware/variable storage on the
> > hikey. Of course I ran smack into the problem of making the block
> > device DXE's run-time safe which I've about concluded is far harder
> > than simply writing a monolithic variablestore->emmc driver.
> >
> > The ideal situation for the Hikey, is probably to solder a SPI flash
> > to the SPI controller and put the firmware/variable store on that, and
> > leave the eMMC entirely for linux's use.
> >
> 
> Oh, I completely agree that HiKey is a trainwreck in this regard. I asked for 
> a
> 96boards mezzanine board with a SPI NOR more than 2 years ago so we could
> prototype this stuff properly, but it never materialized.
> 
> But eMMC *can* potentially be used in a meaningful way, if we use a MMC boot
> partition for the UEFI image and the RPMB partition for the variable store 
> (which
> would arguably be the only way to get a truly tamper proof *and* rollback
> protected varstore, which is what you minimally need to implement UEFI secure
> boot)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] NetworkPkg/IScsiDxe: Remove redundant call to StrLen

2017-09-18 Thread Wu, Jiaxin

Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, September 19, 2017 10:12 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Fu, Siyuan ;
> Wu, Jiaxin 
> Subject: [PATCH] NetworkPkg/IScsiDxe: Remove redundant call to StrLen
> 
> The commits ultilizes a local variable to store the length of a string
> which will be used right after.
> 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  NetworkPkg/IScsiDxe/IScsiConfig.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c
> b/NetworkPkg/IScsiDxe/IScsiConfig.c
> index c0dd305ecf..52e51d6b31 100644
> --- a/NetworkPkg/IScsiDxe/IScsiConfig.c
> +++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
> @@ -766,8 +766,10 @@
> IScsiConvertAttemptConfigDataToIfrNvDataByKeyword (
> 
>*(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr)) = L'/';
>  }
> -if (StrLen (IfrNvData->ISCSIMacAddr) != 0) {
> -  *(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr) - 1) =
> L'\0';
> +
> +StringLen = StrLen (IfrNvData->ISCSIMacAddr);
> +if (StringLen > 0) {
> +  *(IfrNvData->ISCSIMacAddr + StringLen - 1) = L'\0';
>  }
>}
>  }
> --
> 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] NetworkPkg/IScsiDxe: Remove redundant call to StrLen

2017-09-18 Thread Fu, Siyuan

Reviewed-by: Fu Siyuan 

-Original Message-
From: Wu, Hao A 
Sent: Tuesday, September 19, 2017 10:12 AM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Fu, Siyuan ; Wu, 
Jiaxin 
Subject: [PATCH] NetworkPkg/IScsiDxe: Remove redundant call to StrLen

The commits ultilizes a local variable to store the length of a string
which will be used right after.

Cc: Fu Siyuan 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 NetworkPkg/IScsiDxe/IScsiConfig.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c 
b/NetworkPkg/IScsiDxe/IScsiConfig.c
index c0dd305ecf..52e51d6b31 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -766,8 +766,10 @@ IScsiConvertAttemptConfigDataToIfrNvDataByKeyword (
 
   *(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr)) = L'/';
 }
-if (StrLen (IfrNvData->ISCSIMacAddr) != 0) {
-  *(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr) - 1) = 
L'\0';
+
+StringLen = StrLen (IfrNvData->ISCSIMacAddr);
+if (StringLen > 0) {
+  *(IfrNvData->ISCSIMacAddr + StringLen - 1) = L'\0';
 }
   }
 }
-- 
2.12.0.windows.1

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

[edk2] [PATCH] CryptoPkg: Add new API to retrieve commonName of X.509 certificate

2017-09-18 Thread Long Qin

Add one new API (X509GetCommonName()) to retrieve the subject commonName
string from one X.509 certificate.

Cc: Ting Ye 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/Application/Cryptest/RsaVerify2.c| 17 
 CryptoPkg/Include/Library/BaseCryptLib.h   | 32 
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  | 93 ++
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c  | 32 
 .../Pk/CryptX509Null.c | 34 +++-
 5 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c 
b/CryptoPkg/Application/Cryptest/RsaVerify2.c
index 98b5aad900..f9b70d5794 100644
--- a/CryptoPkg/Application/Cryptest/RsaVerify2.c
+++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c
@@ -211,6 +211,9 @@ ValidateCryptRsa2 (
   UINTNSigSize;
   UINT8*Subject;
   UINTNSubjectSize;
+  CHAR8CommonName[64];
+  CHAR16   CommonNameUnicode[64];
+  UINTNCommonNameSize;
 
   Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: ");
 
@@ -286,6 +289,20 @@ ValidateCryptRsa2 (
 Print (L"[Pass]");
   }
 
+  //
+  // Get CommonName from X509 Certificate Subject
+  //
+  CommonNameSize = 64;
+  ZeroMem (CommonName, CommonNameSize);
+  Status = X509GetCommonName (TestCert, sizeof (TestCert), CommonName, 
);
+  if (!Status) {
+Print (L"\n  - Retrieving Common Name - [Fail]");
+return EFI_ABORTED;
+  } else {
+AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize);
+Print (L"\n  - Retrieving Common Name = \"%s\" (Size = %d)", 
CommonNameUnicode, CommonNameSize);
+  }
+
   //
   // X509 Certificate Verification.
   //
diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h 
b/CryptoPkg/Include/Library/BaseCryptLib.h
index 9c5ffcd9cf..d861be6725 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -2171,6 +2171,38 @@ X509GetSubjectName (
   IN OUT  UINTN*SubjectSize
   );
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]  CertPointer to the DER-encoded X509 certificate.
+  @param[in]  CertSizeSize of the X509 certificate in bytes.
+  @param[out] CommonName  Buffer to contain the retrieved certificate 
common
+  name string. At most CommonNameSize bytes 
will be
+  written and the string will be null 
terminated. May be
+  NULL in order to determine the size buffer 
needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer 
on input,
+  and the size of buffer returned CommonName 
on output.
+  if CommonName is NULL then the amount of 
space needed
+  in buffer (including the final null) is 
returned.
+
+  @retval  TRUE   The certificate CommonName retrieved successfully.
+  @retval  FALSE  Invalid certificate, or CommonNameSize is NULL,
+  or no CommonName entry exists.
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509GetCommonName (
+  IN  CONST UINT8  *Cert,
+  IN  UINTNCertSize,
+  OUT CHAR8*CommonName,
+  IN OUT  UINTN*CommonNameSize
+  );
+
 /**
   Verify one X509 certificate was issued by the trusted CA.
 
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 7d275977c5..e45c214bd1 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -297,6 +297,99 @@ _Exit:
   return Status;
 }
 
+/**
+  Retrieve the common name (CN) string from one X.509 certificate.
+
+  If Cert or CommonNameSize is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in]  CertPointer to the DER-encoded X509 certificate.
+  @param[in]  CertSizeSize of the X509 certificate in bytes.
+  @param[out] CommonName  Buffer to contain the retrieved certificate 
common
+  name string. At most CommonNameSize bytes 
will be
+  written and the string will be null 
terminated. May be
+  NULL in order to determine the size buffer 
needed.
+  @param[in,out]  CommonNameSize  The size in bytes of the CommonName buffer 
on input,
+  and the size of buffer returned CommonName 
on output.
+  if CommonName is NULL then the amount of 
space needed
+  in buffer (including the

Re: [edk2] [PATCH 0/7] MdeModulePkg/Udf: Code refinements

2017-09-18 Thread Wu, Hao A

Sure, I will keep an eye on the changes in UDF-related code.


Best Regards,
Hao Wu


> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, September 19, 2017 11:31 AM
> To: Paulo Alcantara; Wu, Hao A; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Dong, Eric; Bi, Dandan; Zeng, Star
> Subject: RE: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
> 
> Thanks for keeping improving the UDF code.
> Reviewed-by: Star Zeng 
> 
> Hao, you may push this patch series first, after that, could you help kindly 
> check
> whether are there similar issues with the new patch series at
> https://lists.01.org/pipermail/edk2-devel/2017-September/014791.html?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Saturday, September 16, 2017 5:47 AM
> To: Wu, Hao A ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Zeng, Star ; Dong,
> Eric ; Bi, Dandan 
> Subject: Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements
> 
> Hi Hao,
> 
> On 15/09/2017 01:57, Hao Wu wrote:
> > The series introduces the following code refinements for UdfDxe &
> > PartitionDxe:
> >
> > a. Add checks to ensure no possible NULL pointer dereference b.
> > Reslove operands of different size in bitwise operations c. Use
> > compare operator for non-boolean comparisons d. Refine function
> > description comments e. Refine local variable initialization f. Refine
> > enum members naming style
> >
> > Cc: Paulo Alcantara 
> > Cc: Ruiyu Ni 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Cc: Dandan Bi 
> >
> > Hao Wu (7):
> >MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
> >MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
> >MdeModulePkg/UdfDxe: Use compare operator for non-boolean
> comparisons
> >MdeModulePkg/Udf: Refine function description comments
> >MdeModulePkg/UdfDxe: Avoid short (single character) variable name
> >MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
> >MdeModulePkg/UdfDxe: Refine enum member naming style
> >
> >   MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c|  20 +-
> >   MdeModulePkg/Universal/Disk/UdfDxe/File.c |   5 +-
> >   MdeModulePkg/Universal/Disk/UdfDxe/FileName.c |  27 +-
> >   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 483
> +++-
> >   MdeModulePkg/Universal/Disk/UdfDxe/Udf.h  |  49 +-
> >   5 files changed, 432 insertions(+), 152 deletions(-)
> Looks good to me. Also tested it with OVMF X64. Thanks!
> 
> Reviewed-by: Paulo Alcantara 
> 
> Paulo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/7] MdeModulePkg/Udf: Code refinements

2017-09-18 Thread Zeng, Star

Thanks for keeping improving the UDF code.
Reviewed-by: Star Zeng 

Hao, you may push this patch series first, after that, could you help kindly 
check whether are there similar issues with the new patch series at 
https://lists.01.org/pipermail/edk2-devel/2017-September/014791.html?


Thanks,
Star
-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com] 
Sent: Saturday, September 16, 2017 5:47 AM
To: Wu, Hao A ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Zeng, Star ; Dong, 
Eric ; Bi, Dandan 
Subject: Re: [PATCH 0/7] MdeModulePkg/Udf: Code refinements

Hi Hao,

On 15/09/2017 01:57, Hao Wu wrote:
> The series introduces the following code refinements for UdfDxe &
> PartitionDxe:
> 
> a. Add checks to ensure no possible NULL pointer dereference b. 
> Reslove operands of different size in bitwise operations c. Use 
> compare operator for non-boolean comparisons d. Refine function 
> description comments e. Refine local variable initialization f. Refine 
> enum members naming style
> 
> Cc: Paulo Alcantara 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Dandan Bi 
> 
> Hao Wu (7):
>MdeModulePkg/UdfDxe: Add checks to ensure no possible NULL ptr deref
>MdeModulePkg/UdfDxe: Fix operands of different size in bitwise OP
>MdeModulePkg/UdfDxe: Use compare operator for non-boolean comparisons
>MdeModulePkg/Udf: Refine function description comments
>MdeModulePkg/UdfDxe: Avoid short (single character) variable name
>MdeModulePkg/Udf: Avoid declaring and initializing local GUID variable
>MdeModulePkg/UdfDxe: Refine enum member naming style
> 
>   MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c|  20 +-
>   MdeModulePkg/Universal/Disk/UdfDxe/File.c |   5 +-
>   MdeModulePkg/Universal/Disk/UdfDxe/FileName.c |  27 +-
>   MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 483 
> +++-
>   MdeModulePkg/Universal/Disk/UdfDxe/Udf.h  |  49 +-
>   5 files changed, 432 insertions(+), 152 deletions(-)
Looks good to me. Also tested it with OVMF X64. Thanks!

Reviewed-by: Paulo Alcantara 

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

Re: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change serial attributes

2017-09-18 Thread Zeng, Star

Pushed at 91cc526b15ffbbbdec5a57906596f37e059f80be :)

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pankaj 
Bansal
Sent: Tuesday, September 19, 2017 11:05 AM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Laszlo Ersek (ler...@redhat.com) 
Subject: Re: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Hi Zeng,

Thanks for suggestion.
I agree, that this looks nicer and gives more details.
Please push the patch with comment updates and signatures as you have mentioned.

Thanks & Regards,
Pankaj Bansal

-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com]
Sent: Tuesday, September 19, 2017 8:30 AM
To: Pankaj Bansal ; edk2-devel@lists.01.org
Cc: Zeng, Star ; Laszlo Ersek (ler...@redhat.com) 

Subject: RE: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Pankaj,

Thanks for your explanation at 
https://lists.01.org/pipermail/edk2-devel/2017-September/014832.html about the 
execute flow to reproduce the issue.
How about also include the explanation to the commit log like below?

==
Issue : When try to change serial attributes using sermode command, the default 
values are set with the execute flow as below.

The sermode command calls SerialSetAttributes, which sets H/W attributes of 
Serial device. After that the SerialIo protocol is reinstalled, which causes 
MdeModulePkg/Universal/Console/TerminalDxe
and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings to stop and 
then start. This in turn calls SerialReset, which undoes changes of 
SerialSetAttributes.

Cause : The SerialReset command resets the attributes' values to default.
Fix : Serial Reset command should set the attributes which have been changed by 
user after calling SerialSetAttributes.
==

Another minor comment, how about update "COM port" to be "Serial Port" in the 
change?

If you agree with above comments, you do not need to send an update patch and I 
will help push the patch with the updates I comment above and Reviewed-by: Star 
Zeng , and of course Regression-tested-by: Laszlo Ersek 
.

Thanks,
Star
-Original Message-
From: Zeng, Star
Sent: Monday, September 18, 2017 6:12 PM
To: Pankaj Bansal ; edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: RE: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Pankaj,

Thanks for the update.

I raised a question in the V1 patch review, could you help kindly provide the 
feedback?
"how you reproduce the issue by sermode command as I see sermode command only 
calls SerialIo->SetAttributes() but not SerialIo->Reset()"

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pankaj 
Bansal
Sent: Monday, September 18, 2017 3:43 PM
To: edk2-devel@lists.01.org
Cc: Pankaj Bansal 
Subject: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Issue : When try to change serial attributes using sermode command, the default 
values are set Cause : The SerialReset command resets the attributes' values to 
default Fix : Serial Reset command should set the attributes which have been 
changed by user after calling SerialSetAttributes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---
Changes in v2:
   - Modified Patch description

 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 -
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index 43d33db..dc7e13a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -220,7 +220,6 @@ SerialReset (
   )
 {
   EFI_STATUSStatus;
-  EFI_TPL   Tpl;

   Status = SerialPortInitialize ();
   if (EFI_ERROR (Status)) {
@@ -228,49 +227,17 @@ SerialReset (
   }

   //
-  // Set the Serial I/O mode and update the device path
-  //
-
-  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
-
-  //
-  // Set the Serial I/O mode
-  //
-  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
-  This->Mode->Timeout   = 1000 * 1000;
-  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
-  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
-  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
-  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
-
-  //
-  // Check if the device path has actually changed
-  //
-  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
-  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&

Re: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change serial attributes

2017-09-18 Thread Pankaj Bansal

Hi Zeng,

Thanks for suggestion.
I agree, that this looks nicer and gives more details.
Please push the patch with comment updates and signatures as you have mentioned.

Thanks & Regards,
Pankaj Bansal

-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com] 
Sent: Tuesday, September 19, 2017 8:30 AM
To: Pankaj Bansal ; edk2-devel@lists.01.org
Cc: Zeng, Star ; Laszlo Ersek (ler...@redhat.com) 

Subject: RE: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Pankaj,

Thanks for your explanation at 
https://lists.01.org/pipermail/edk2-devel/2017-September/014832.html about the 
execute flow to reproduce the issue.
How about also include the explanation to the commit log like below?

==
Issue : When try to change serial attributes using sermode command, the default 
values are set with the execute flow as below.

The sermode command calls SerialSetAttributes, which sets H/W attributes of 
Serial device. After that the SerialIo protocol is reinstalled, which causes 
MdeModulePkg/Universal/Console/TerminalDxe
and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings to stop and 
then start. This in turn calls SerialReset, which undoes changes of 
SerialSetAttributes.

Cause : The SerialReset command resets the attributes' values to default.
Fix : Serial Reset command should set the attributes which have been changed by 
user after calling SerialSetAttributes.
==

Another minor comment, how about update "COM port" to be "Serial Port" in the 
change?


If you agree with above comments, you do not need to send an update patch and I 
will help push the patch with the updates I comment above and Reviewed-by: Star 
Zeng , and of course Regression-tested-by: Laszlo Ersek 
.


Thanks,
Star
-Original Message-
From: Zeng, Star
Sent: Monday, September 18, 2017 6:12 PM
To: Pankaj Bansal ; edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: RE: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Pankaj,

Thanks for the update.

I raised a question in the V1 patch review, could you help kindly provide the 
feedback?
"how you reproduce the issue by sermode command as I see sermode command only 
calls SerialIo->SetAttributes() but not SerialIo->Reset()"


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pankaj 
Bansal
Sent: Monday, September 18, 2017 3:43 PM
To: edk2-devel@lists.01.org
Cc: Pankaj Bansal 
Subject: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Issue : When try to change serial attributes using sermode command, the default 
values are set Cause : The SerialReset command resets the attributes' values to 
default Fix : Serial Reset command should set the attributes which have been 
changed by user after calling SerialSetAttributes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---
Changes in v2:
   - Modified Patch description

 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 -
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index 43d33db..dc7e13a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -220,7 +220,6 @@ SerialReset (
   )
 {
   EFI_STATUSStatus;
-  EFI_TPL   Tpl;
 
   Status = SerialPortInitialize ();
   if (EFI_ERROR (Status)) {
@@ -228,49 +227,17 @@ SerialReset (
   }
 
   //
-  // Set the Serial I/O mode and update the device path
-  //
-
-  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
-
-  //
-  // Set the Serial I/O mode
-  //
-  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
-  This->Mode->Timeout   = 1000 * 1000;
-  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
-  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
-  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
-  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
-
-  //
-  // Check if the device path has actually changed
-  //
-  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
-  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&
-  mSerialDevicePath.Uart.Parity   == (UINT8) This->Mode->Parity &&
-  mSerialDevicePath.Uart.StopBits == (UINT8) This->Mode->StopBits
- ) {
-gBS->RestoreTPL (Tpl);
-return EFI_SUCCESS;
-  }
-
-  //
-  // Update the device path
+  // Go set the current attributes
   //
-  mSerialDevicePath.Uart.BaudRate = This->Mode->BaudRate;
-  mSerialDevicePath.Uart.DataBits = (UINT8) This->Mode->DataBits;
-

Re: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change serial attributes

2017-09-18 Thread Zeng, Star

Pankaj,

Thanks for your explanation at 
https://lists.01.org/pipermail/edk2-devel/2017-September/014832.html about the 
execute flow to reproduce the issue.
How about also include the explanation to the commit log like below?

==
Issue : When try to change serial attributes using sermode
command, the default values are set with the execute flow
as below.

The sermode command calls SerialSetAttributes, which sets H/W
attributes of Serial device. After that the SerialIo protocol is
reinstalled, which causes MdeModulePkg/Universal/Console/TerminalDxe
and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings
to stop and then start. This in turn calls SerialReset, which undoes
changes of SerialSetAttributes.

Cause : The SerialReset command resets the attributes' values
to default.
Fix : Serial Reset command should set the attributes which have
been changed by user after calling SerialSetAttributes.
==

Another minor comment, how about update "COM port" to be "Serial Port" in the 
change?


If you agree with above comments, you do not need to send an update patch and
I will help push the patch with the updates I comment above and Reviewed-by: 
Star Zeng ,
and of course Regression-tested-by: Laszlo Ersek .


Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Monday, September 18, 2017 6:12 PM
To: Pankaj Bansal ; edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: RE: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Pankaj,

Thanks for the update.

I raised a question in the V1 patch review, could you help kindly provide the 
feedback?
"how you reproduce the issue by sermode command as I see sermode command only 
calls SerialIo->SetAttributes() but not SerialIo->Reset()"


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pankaj 
Bansal
Sent: Monday, September 18, 2017 3:43 PM
To: edk2-devel@lists.01.org
Cc: Pankaj Bansal 
Subject: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Issue : When try to change serial attributes using sermode command, the default 
values are set Cause : The SerialReset command resets the attributes' values to 
default Fix : Serial Reset command should set the attributes which have been 
changed by user after calling SerialSetAttributes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---
Changes in v2:
   - Modified Patch description

 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 -
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index 43d33db..dc7e13a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -220,7 +220,6 @@ SerialReset (
   )
 {
   EFI_STATUSStatus;
-  EFI_TPL   Tpl;
 
   Status = SerialPortInitialize ();
   if (EFI_ERROR (Status)) {
@@ -228,49 +227,17 @@ SerialReset (
   }
 
   //
-  // Set the Serial I/O mode and update the device path
-  //
-
-  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
-
-  //
-  // Set the Serial I/O mode
-  //
-  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
-  This->Mode->Timeout   = 1000 * 1000;
-  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
-  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
-  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
-  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
-
-  //
-  // Check if the device path has actually changed
-  //
-  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
-  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&
-  mSerialDevicePath.Uart.Parity   == (UINT8) This->Mode->Parity &&
-  mSerialDevicePath.Uart.StopBits == (UINT8) This->Mode->StopBits
- ) {
-gBS->RestoreTPL (Tpl);
-return EFI_SUCCESS;
-  }
-
-  //
-  // Update the device path
+  // Go set the current attributes
   //
-  mSerialDevicePath.Uart.BaudRate = This->Mode->BaudRate;
-  mSerialDevicePath.Uart.DataBits = (UINT8) This->Mode->DataBits;
-  mSerialDevicePath.Uart.Parity   = (UINT8) This->Mode->Parity;
-  mSerialDevicePath.Uart.StopBits = (UINT8) This->Mode->StopBits;
-
-  Status = gBS->ReinstallProtocolInterface (
-  mSerialHandle,
-  ,
-  ,
-  
-  );
-
-  gBS->RestoreTPL (Tpl);
+  Status = This->SetAttributes (
+   This,
+   This->Mode->BaudRate,
+   This->Mode->ReceiveFifoDepth,
+   This->Mode->Timeout,
+   (EFI_PARITY_TYPE) This->Mode->Parity,
+

Re: [edk2] EFI_MEMORY_ATTRIBUTES_TABLE not in runtime memory

2017-09-18 Thread Ard Biesheuvel

On 18 September 2017 at 18:52, Zeng, Star  wrote:
> As I know according to UEFI spec, it has no requirement about the Table needs 
> to be in runtime memory, but only requires the list (holding Guid value and 
> Table pointer) needs to be in runtime memory.
>
> "The InstallConfigurationTable() function is used to maintain the list of 
> configuration tables that are stored in the EFI System Table. The list is 
> stored as an array of (GUID, Pointer) pairs. The list must be allocated from 
> pool memory with PoolType set to EfiRuntimeServicesData."
>
>
> Jiewen, you should know more detail about this feature.
>

Whether the list of configuration tables is in runtime memory and
whether the tables themselves are in runtime memory are completely
different things.

Runtime memory will get a mapping in the virtual tables created by the
OS, which is only necessary if the firmware itself refers to the
contents when runtime services are in progress. Using runtime memory
so that it will be automatically protected from allocation by the OS
implies that the memory is lost even if the OS does not understand the
table type in the first place. And if it does understand the table
type, it can reserve the region itself.

In summary, using runtime memory for configuration tables is a bad
idea, and the fact that it is recommended for SMbios (and there is
even a mention that no virtual mapping should be requested, which is
impossible) is a mistake IMO.

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

[edk2] [PATCH] NetworkPkg/IScsiDxe: Remove redundant call to StrLen

2017-09-18 Thread Hao Wu

The commits ultilizes a local variable to store the length of a string
which will be used right after.

Cc: Fu Siyuan 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 NetworkPkg/IScsiDxe/IScsiConfig.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c 
b/NetworkPkg/IScsiDxe/IScsiConfig.c
index c0dd305ecf..52e51d6b31 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -766,8 +766,10 @@ IScsiConvertAttemptConfigDataToIfrNvDataByKeyword (
 
   *(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr)) = L'/';
 }
-if (StrLen (IfrNvData->ISCSIMacAddr) != 0) {
-  *(IfrNvData->ISCSIMacAddr + StrLen (IfrNvData->ISCSIMacAddr) - 1) = 
L'\0';
+
+StringLen = StrLen (IfrNvData->ISCSIMacAddr);
+if (StringLen > 0) {
+  *(IfrNvData->ISCSIMacAddr + StringLen - 1) = L'\0';
 }
   }
 }
-- 
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] Fix not able to change serial attributes

2017-09-18 Thread Zeng, Star

Really appreciate the Regression-tested-by, I will pick up it when pushing the 
patch.

Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, September 18, 2017 11:55 PM
To: Zeng, Star ; Heyi Guo ; Pankaj 
Bansal ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Ard Biesheuvel 
Subject: Re: [edk2] [PATCH] Fix not able to change serial attributes

On 09/18/17 12:22, Zeng, Star wrote:
> Thanks for your good comments. :)
> Since there is no clear description for the behavior of Reset() :(, I prone 
> to align the behavior with MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c 
> and IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c, that means I agree 
> the fix.
> 
> 
> Laszlo and Gary, if you can help do some simple regression test with the 
> patch, that will be better.

Never used the SERMODE command before, so I didn't try it now either.

I tested the patch as follows: built ArmVirtQemu with it, and first checked 
normal use of the serial terminal, then issued a RECONNECT/CONNECT from the 
graphics console, and then verified if the serial terminal was working OK again 
(UEFI shell and Setup utility). I found no regression with this.

Regression-tested-by: Laszlo Ersek 

Thanks
Laszlo

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

Re: [edk2] EFI_MEMORY_ATTRIBUTES_TABLE not in runtime memory

2017-09-18 Thread Zeng, Star

As I know according to UEFI spec, it has no requirement about the Table needs 
to be in runtime memory, but only requires the list (holding Guid value and 
Table pointer) needs to be in runtime memory.

"The InstallConfigurationTable() function is used to maintain the list of 
configuration tables that are stored in the EFI System Table. The list is 
stored as an array of (GUID, Pointer) pairs. The list must be allocated from 
pool memory with PoolType set to EfiRuntimeServicesData."


Jiewen, you should know more detail about this feature.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Larry 
Cleeton
Sent: Tuesday, September 19, 2017 3:49 AM
To: edk2-devel@lists.01.org
Subject: [edk2] EFI_MEMORY_ATTRIBUTES_TABLE not in runtime memory

The EFI_MEMORY_ATTRIBUTES_TABLE constructed in 
MdeModulePkg/core/dxe/misc/MemoryAttributesTable.c is allocating the table 
memory using AllocatePool(). It seems it should be allocated using 
AllocateRuntimePool(). We've observed Linux kernels seeing a zeroed table, 
likely because those boot services memory pages have been reclaimed and zeroed. 
It appears that several other configuration tables are allocated with runtime 
memory. I haven't found any specific statements that this table should be in 
runtime memory. However it seems pragmatic and consistent that it be in runtime 
memory.
___
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] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Ard Biesheuvel

On 18 September 2017 at 13:47, Jeremy Linton  wrote:
> On 09/18/2017 10:43 AM, Ard Biesheuvel wrote:
>>
>> On 18 September 2017 at 06:52, Udit Kumar  wrote:
>>>
>>> Hi EDK-2 Experts,
>>> I am looking to store NV variables on SD/NAND device.
>>>
>>> While browsing, I came across some old post at link,
>>>
>>> http://feishare.com/efimail/messages/20130319-1700-Re__edk2__Regarding_storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html
>>>
>>> Looks like, this is possible easily.
>>
>>
>> That's a bold statement dude :-)
>>
> What you need to support Non-Volatile UEFI variables is a Non-Volatile
> Memory. And also a driver that implements the EFI Firmware Volume Block
> protocol for this NVM device.
>>>
>>>
>>> But MdeModulePkg does Copymem from NV variable start memory to some
>>> allocated buffers.  With SD/NAND Copymem is not possible, Is this something
>>> changes since 2013 or there are some other way to use SD/NAND
>>>
>>
>> No, SD/MMC cannot currently be used as the backing store for the EFI
>> variable store. The problem is that the variable protocols are
>> architectural protocols in PI that need to be present before any
>> driver model drivers are dispatched, and so putting the variable store
>> on block devices is not something that the PI software architecture
>> currently supports (unless you reimplement the whole driver stack as
>> DXE drivers).
>>
>> On top of that, it is almost impossible to share a block device that
>> sits behind a controller between the firmware and the OS at runtime
>> (i.e., for SetVariable() calls made by efibootmgr under Linux),
>> because only a single agent can take ownership of the controller at
>> any given time. (You /could/ dedicate the SD/MMC to the firmware
>> entirely, and boot from SATA or USB, but this is out of the question
>> on most platforms that need to use SD/MMC for that variable backing
>> store, i.e., mobile platforms)
>>
>> The best thing would be for you to convince the hardware architects in
>> your company to design and implement dual-ported SD/MMC controllers
>> that allow a single SD/MMC to have two logical views that are
>> independent (although I'm unsure if that is even possible in the
>> context of the SD/MMC specifications)
>
>
> Which still has the problems of selecting "use whole disk" during an OS
> install bricking the machine. Or similarly if the emmc layout isn't just
> right having gparted automatically "fix" the partition table (as it does
> with many of the hikey images) again bricking the machine.
>
> Having the firmware/variable store and OS root/boot on the same device is
> fundamentally flawed. I've went down the path of simply disabling the
> hikey/emmc for use beyond the firmware/variable storage on the hikey. Of
> course I ran smack into the problem of making the block device DXE's
> run-time safe which I've about concluded is far harder than simply writing a
> monolithic variablestore->emmc driver.
>
> The ideal situation for the Hikey, is probably to solder a SPI flash to the
> SPI controller and put the firmware/variable store on that, and leave the
> eMMC entirely for linux's use.
>

Oh, I completely agree that HiKey is a trainwreck in this regard. I
asked for a 96boards mezzanine board with a SPI NOR more than 2 years
ago so we could prototype this stuff properly, but it never
materialized.

But eMMC *can* potentially be used in a meaningful way, if we use a
MMC boot partition for the UEFI image and the RPMB partition for the
variable store (which would arguably be the only way to get a truly
tamper proof *and* rollback protected varstore, which is what you
minimally need to implement UEFI secure boot)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Jeremy Linton


On 09/18/2017 10:43 AM, Ard Biesheuvel wrote:

On 18 September 2017 at 06:52, Udit Kumar  wrote:

Hi EDK-2 Experts,
I am looking to store NV variables on SD/NAND device.

While browsing, I came across some old post at link,
http://feishare.com/efimail/messages/20130319-1700-Re__edk2__Regarding_storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html

Looks like, this is possible easily.


That's a bold statement dude :-)


What you need to support Non-Volatile UEFI variables is a Non-Volatile Memory. 
And also a driver that implements the EFI Firmware Volume Block protocol for 
this NVM device.


But MdeModulePkg does Copymem from NV variable start memory to some allocated 
buffers.  With SD/NAND Copymem is not possible, Is this something changes since 
2013 or there are some other way to use SD/NAND



No, SD/MMC cannot currently be used as the backing store for the EFI
variable store. The problem is that the variable protocols are
architectural protocols in PI that need to be present before any
driver model drivers are dispatched, and so putting the variable store
on block devices is not something that the PI software architecture
currently supports (unless you reimplement the whole driver stack as
DXE drivers).

On top of that, it is almost impossible to share a block device that
sits behind a controller between the firmware and the OS at runtime
(i.e., for SetVariable() calls made by efibootmgr under Linux),
because only a single agent can take ownership of the controller at
any given time. (You /could/ dedicate the SD/MMC to the firmware
entirely, and boot from SATA or USB, but this is out of the question
on most platforms that need to use SD/MMC for that variable backing
store, i.e., mobile platforms)

The best thing would be for you to convince the hardware architects in
your company to design and implement dual-ported SD/MMC controllers
that allow a single SD/MMC to have two logical views that are
independent (although I'm unsure if that is even possible in the
context of the SD/MMC specifications)


Which still has the problems of selecting "use whole disk" during an OS 
install bricking the machine. Or similarly if the emmc layout isn't just 
right having gparted automatically "fix" the partition table (as it does 
with many of the hikey images) again bricking the machine.


Having the firmware/variable store and OS root/boot on the same device 
is fundamentally flawed. I've went down the path of simply disabling the 
hikey/emmc for use beyond the firmware/variable storage on the hikey. Of 
course I ran smack into the problem of making the block device DXE's 
run-time safe which I've about concluded is far harder than simply 
writing a monolithic variablestore->emmc driver.


The ideal situation for the Hikey, is probably to solder a SPI flash to 
the SPI controller and put the firmware/variable store on that, and 
leave the eMMC entirely for linux's use.


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

[edk2] EFI_MEMORY_ATTRIBUTES_TABLE not in runtime memory

2017-09-18 Thread Larry Cleeton

The EFI_MEMORY_ATTRIBUTES_TABLE constructed in 
MdeModulePkg/core/dxe/misc/MemoryAttributesTable.c is allocating the table 
memory using AllocatePool(). It seems it should be allocated using 
AllocateRuntimePool(). We've observed Linux kernels seeing a zeroed table, 
likely because those boot services memory pages have been reclaimed and zeroed. 
It appears that several other configuration tables are allocated with runtime 
memory. I haven't found any specific statements that this table should be in 
runtime memory. However it seems pragmatic and consistent that it be in runtime 
memory.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/3] UDF partition driver fix

2017-09-18 Thread Paulo Alcantara


Hi Ruiyu,

On 9/18/2017 1:52 AM, Ni, Ruiyu wrote:

Paulo,
Could you please paste a "map -r" output on a CDROM which
contains Eltorito volume?
I want to confirm that the result is expected.


With UdfDxe driver disabled in OVMF:

>Mapping table
>  FS0: Alias(s):CD0c65535a1:;BLK2:
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/CDROM(0x1)
> BLK0: Alias(s):
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)
> BLK1: Alias(s):
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/CDROM(0x0)
> BLK3: Alias(s):
> 
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)


With UdfDxe driver enabled in OVMF:

> Mapping table
>  FS0: Alias(s):CD0c65535a1:;BLK2:
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/CDROM(0x1)
>  FS1: Alias(s):F0c65535a:;BLK3:
> 
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)

> BLK0: Alias(s):
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)
> BLK1: Alias(s):
>  PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0x,0x0)/CDROM(0x0)

Thanks!
Paulo



Thanks/Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 17, 2017 9:13 PM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Kinney, Michael D
; Gao, Liming ; Laszlo
Ersek ; Ni, Ruiyu ; Zeng, Star
; Yao, Jiewen 
Subject: [PATCH v2 0/3] UDF partition driver fix

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

Hi,

This patchset fixes a bug in Partition driver that UDF partitions occupied the
entire disk space instead of using LVD space only.

BTW, I've only tested it under OVMF and built it with GCC only. That would
be appreciable if someone could build with other toolchains and see if this
doesn't break.

I used a Windows 10 ISO image with UdfDxe disabled and enabled. The `map
-r` output seemed OK. No breakage when booting an OS off an ElTorito
partition from an UDF bridge disk.

v1->v2:
  - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
  - As I'm still waiting for Ruiyu and Star to test this fix, I took
advantage of it and did some code cleanups :-)

Repo:   https://github.com/pcacjr/edk2.git
Branch: udf-partition-fix-v2

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Reported-by: Ruiyu Ni 
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (3):
   MdePkg: Add UDF volume structure definitions
   MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
   MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 323
+++-
  MdeModulePkg/Universal/Disk/UdfDxe/File.c |  13 +-
  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515

  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c  |   7 -
  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h  |  88 +---
  MdePkg/Include/IndustryStandard/Udf.h |  63 +++
  6 files changed, 566 insertions(+), 443 deletions(-)

--
2.11.0




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

Re: [edk2] [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes

2017-09-18 Thread Paulo Alcantara




On 9/17/2017 10:00 PM, Ni, Ruiyu wrote:

Paulo,
With the change in partition driver, I suppose UdfDxe driver
only needs to take care of area covered by the partition descriptor.
But why StartMainVolumeDescriptorSequence() still reads
LVD, TD, and PD?


We still need the LVD again for a few reasons:

1. GetVolumeSize() needs to read LVID (Logical Volume Integrity 
Descriptor), which is located in LVD, in order to calculate free and 
used volume space.

2. To find FSD (File Set Descriptor), where the root directory is located.
3. To know which Partition Descriptor to use based on partition number 
from LVD->PartitionMaps field. I know UDF 2.60 spec says that there 
should be only one Partition Descriptor that covers the entire space of 
a single LVD, but it's good to make sure we're reading it correctly.


TD descriptor is used to know when stopping the Volume Descriptor 
Sequence and then we don't need to read all ExtentAd->ExtentLength 
blocks unnecessarily.


Thanks,
Paulo



I thought the UdfDxe driver's logic can be simplified a lot.
There should be no duplicated logic in Partition driver and udf driver.

Can you explain more?

Thanks/Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 17, 2017 9:13 PM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Dong, Eric ;
Ni, Ruiyu ; Zeng, Star ; Laszlo
Ersek 
Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support
PartitionDxe changes

This patch reworks the driver to support Partition driver changes.

Cc: Eric Dong 
Cc: Paulo Alcantara 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---
  MdeModulePkg/Universal/Disk/UdfDxe/File.c |  13 +-
  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515

  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c  |   7 -
  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h  |  88 +---
  4 files changed, 204 insertions(+), 419 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 01361141bb..f2c62967e8 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -100,6 +100,7 @@ UdfOpenVolume (
  >Volume,
  >Root
  );
+  ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
  goto Error_Find_Root_Dir;
}
@@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
CleanupFileInformation (>Root);

  Error_Find_Root_Dir:
-  CleanupVolumeInformation (>Volume);

  Error_Read_Udf_Volume:
  Error_Invalid_Params:
@@ -528,7 +528,6 @@ UdfClose (
EFI_TPL OldTpl;
EFI_STATUS  Status;
PRIVATE_UDF_FILE_DATA   *PrivFileData;
-  PRIVATE_UDF_SIMPLE_FS_DATA  *PrivFsData;

OldTpl = gBS->RaiseTPL (TPL_CALLBACK);

@@ -541,8 +540,6 @@ UdfClose (

PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);

-  PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-

SimpleFs);

-
if (!PrivFileData->IsRootDirectory) {
  CleanupFileInformation (>File);

@@ -551,10 +548,6 @@ UdfClose (
  }
}

-  if (--PrivFsData->OpenFiles == 0) {
-CleanupVolumeInformation (>Volume);
-  }
-
FreePool ((VOID *)PrivFileData);

  Exit:
@@ -787,7 +780,7 @@ UdfGetInfo (
} else if (CompareGuid (InformationType, )) {
  String = VolumeLabel;

-FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
+FileSetDesc = >Volume.FileSetDesc;

  OstaCompressed = >LogicalVolumeIdentifier[0];

@@ -846,7 +839,7 @@ UdfGetInfo (
  FileSystemInfo->Size= FileSystemInfoLength;
  FileSystemInfo->ReadOnly= TRUE;
  FileSystemInfo->BlockSize   =
-  LV_BLOCK_SIZE (>Volume, UDF_DEFAULT_LV_NUM);
+  PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
  FileSystemInfo->VolumeSize  = VolumeSize;
  FileSystemInfo->FreeSpace   = FreeSpaceSize;

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 4609580b30..63b643e60a 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (

  EFI_STATUS
  StartMainVolumeDescriptorSequence (
-  IN   EFI_BLOCK_IO_PROTOCOL *BlockIo,
-  IN   EFI_DISK_IO_PROTOCOL  *DiskIo,
-  IN   UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER  *AnchorPoint,
-  OUT  UDF_VOLUME_INFO   *Volume
+  IN   EFI_BLOCK_IO_PROTOCOL*BlockIo,
+  IN   EFI_DISK_IO_PROTOCOL *DiskIo,
+  OUT  UDF_VOLUME_INFO  *Volume
)
  {
-  EFI_STATUS

Re: [edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Vladimir Olovyannikov

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Monday, September 18, 2017 8:43 AM
> To: Udit Kumar
> Cc: grant.lik...@linaro.org.; edk2-devel@lists.01.org;
> olivier.mar...@arm.com
> Subject: Re: [edk2] Storing Non volatile variables on SD/NAND
>
> On 18 September 2017 at 06:52, Udit Kumar  wrote:
> > Hi EDK-2 Experts,
> > I am looking to store NV variables on SD/NAND device.
> >
> > While browsing, I came across some old post at link,
> > http://feishare.com/efimail/messages/20130319-1700-
> Re__edk2__Regarding
> > _storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html
> >
> > Looks like, this is possible easily.
>
> That's a bold statement dude :-)
>
> >>> What you need to support Non-Volatile UEFI variables is a
Non-Volatile
> Memory. And also a driver that implements the EFI Firmware Volume Block
> protocol for this NVM device.
> >
> > But MdeModulePkg does Copymem from NV variable start memory to
> some
> > allocated buffers.  With SD/NAND Copymem is not possible, Is this
> > something changes since 2013 or there are some other way to use
> > SD/NAND
> >
>
> No, SD/MMC cannot currently be used as the backing store for the EFI
> variable store. The problem is that the variable protocols are
architectural
> protocols in PI that need to be present before any driver model drivers
are
> dispatched, and so putting the variable store on block devices is not
> something that the PI software architecture currently supports (unless
you
> reimplement the whole driver stack as DXE drivers).
>
> On top of that, it is almost impossible to share a block device that
sits behind
> a controller between the firmware and the OS at runtime (i.e., for
> SetVariable() calls made by efibootmgr under Linux), because only a
single
> agent can take ownership of the controller at any given time. (You
/could/
> dedicate the SD/MMC to the firmware entirely, and boot from SATA or USB,
> but this is out of the question on most platforms that need to use
SD/MMC
> for that variable backing store, i.e., mobile platforms)
>
> The best thing would be for you to convince the hardware architects in
your
> company to design and implement dual-ported SD/MMC controllers that
> allow a single SD/MMC to have two logical views that are independent
> (although I'm unsure if that is even possible in the context of the
SD/MMC
> specifications)
>
> Thanks,
> Ard.
Udit,
Ard is absolutely right.

Following design I had to implement variable store on the EMMC boot
partition 1 (not exactly SD card, but close; the same is applied to NAND I
guess).
To do that I forked VariableRuntime driver and changed it to do cache
writes into the store (just modifying store memory) at runtime.
This is because you cannot share eMMC store between OS (Linux)  and the
UEFI service at the same time, and you cannot predict when OS writes/reads
to/from the EMMC device.
So I do cache writes at runtime (just modifying store in memory), and
flush the store on reboot/shutdown.
For this purpose my ResetSystemLib calls into the Variable service and
forces flush. Variable service reinitiates eMMC controller
and really writes into the store on the eMMC. Real flush is also performed
on ExitBootServices().

As a big drawback an OS should either reset or shutdown (get into UEFI
ResetSystemLib) in order for variable store to be updated physically;
Physical flush is also forced on ExitBootServices - be it a reset command,
or booting an OS.
Just pressing a reset button (example: Linux Kernel panic with no reset)
would cause variable changes for this session to get lost.

Thank you,
Vladimir
> ___
> 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/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition

2017-09-18 Thread Paulo Alcantara

Hi,

On 9/17/2017 9:54 PM, Ni, Ruiyu wrote:

Paulo,
Several comments:
1. Can below logic be removed in PartitionDxe/Udf.c?
while (!IsDevicePathEnd (DevicePathNode)) {
 //
 // Do not allow checking for UDF file systems in CDROM "El Torito"
 // partitions, and skip duplicate installation of UDF file system child
 // nodes.
 //
 if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
   if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
 return EFI_NOT_FOUND;
   }
   if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
 VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
  OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
 if (CompareGuid (VendorDefinedGuid, )) {
   return EFI_NOT_FOUND;
 }
   }
 }
 //
 // Try next device path node
 //
 DevicePathNode = NextDevicePathNode (DevicePathNode);
   }

I think it's no longer necessary, so I'll remove it.

2. Can you add function header comments for the newly added functions?

Sure.

3. Change the consuming code of IS_UDF_XXX macro after patch #1 is changed.

ACK.

4. Perhaps to add more checks for these numbers read from the UDF CDROM.
As Jiewen mentioned in another mail, secure code needs to validate all external 
inputs.
We shouldn't assume the UDF CDROM is a well-formatted CDROM.
A hacker may create a UDF CDROM to hack the system by using mechanism like
buffer overflow, integer overflow, etc.

OK, when we start the Main Volume Descriptor Sequence (MVDS), we rely on 
"ExtentAd->ExtentLocation" to know the LBA
where to start the sequence, and on "ExtentAd->ExtentLength" to know 
many LBAs the sequence has. We only care about
LVD, PD and TD descriptors. We currently check whether a TD descriptor 
exists, or the search exceed ExtentAd->ExtentLength blocks
to determine when stopping the sequence. That doesn't seem to be safe 
enough, but here are some thoughts:

a. "ExtentAd->ExtentLength" may point to a bad value. If it's greater 
than or equal to BlockIo->Media->LastBlock, and there is no
TD descriptor, we'll have to walk the entire disk and there will too 
much I/O overhead and then impacting bootup. Obviously,
when it's greater than LastBlock, DiksIo->ReadDisk() should not allow to 
read a non-existent LBA and then return

EFI_INVALID_PARAMETER, as per UEFI spec.

b. "ExtentAd->ExtentLocation" might point to a invalid LBA, although 
DiskIo->ReadDisk() would not allow it.

So, it's time to look at UDF 2.60 spec again:

> Volume Descriptor Sequence Extent
>
> Both the main and reserve volume descriptor sequence extents
> shall each have a minimum length of 16 logical sectors. The VDS
> Extent may be terminated by the extent length.

It doesn't tell us either a limit of logical blocks, or a maximum number.

Next:

> 6.9.2.3 Step 3. Volume Descriptor Sequence
> Read logical sectors:
> MVDS_Location through MVDS_Location + (MVDS_Length - 1) / SectorSize
> ...

MVDS_Length == ExtentAd->ExtentLength, so still insufficient.

Next:

> The data located in bytes 0-3 and 5 of the Anchor Volume Descriptor 
Pointer may be
> used for format verification if desired. Verifying the Tag Checksum 
in byte 4 and
> Descriptor CRC in bytes 8-11 are good additional verifications to 
perform.

> MVDS_Location and MVDS_Length are read from this structure.

Seems like a great check prior to reading ExtentAd->ExtentLocation and 
ExtentAd->ExtentLength contents. Would that be sufficient to you?

I haven't found a way to limit the number of blocks a Volume Descriptor 
Sequence might have.

Thanks,
Paulo

Thanks/Ray

-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 17, 2017 9:13 PM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Dong, Eric ;
Ni, Ruiyu ; Zeng, Star ; Laszlo
Ersek 
Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF
logical partition

Do not use entire block device size for the UDF logical partition, instead
reserve the appropriate space (LVD space) for it.

Cc: Eric Dong 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Reported-by: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---
  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
++--
  1 file changed, 299 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 7856b5dfc1..3e84882922 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
return EFI_VOLUME_CORRUPTED;
  }

-/**
-  Check if block

Re: [edk2] [PATCH] Fix not able to change serial attributes

2017-09-18 Thread Laszlo Ersek

On 09/18/17 12:22, Zeng, Star wrote:
> Thanks for your good comments. :)
> Since there is no clear description for the behavior of Reset() :(, I prone 
> to align the behavior with MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c 
> and IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c, that means I agree 
> the fix.
> 
> 
> Laszlo and Gary, if you can help do some simple regression test with the 
> patch, that will be better.

Never used the SERMODE command before, so I didn't try it now either.

I tested the patch as follows: built ArmVirtQemu with it, and first
checked normal use of the serial terminal, then issued a
RECONNECT/CONNECT from the graphics console, and then verified if the
serial terminal was working OK again (UEFI shell and Setup utility). I
found no regression with this.

Regression-tested-by: Laszlo Ersek 

Thanks
Laszlo

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

Re: [edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Ard Biesheuvel

On 18 September 2017 at 06:52, Udit Kumar  wrote:
> Hi EDK-2 Experts,
> I am looking to store NV variables on SD/NAND device.
>
> While browsing, I came across some old post at link,
> http://feishare.com/efimail/messages/20130319-1700-Re__edk2__Regarding_storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html
>
> Looks like, this is possible easily.

That's a bold statement dude :-)

>>> What you need to support Non-Volatile UEFI variables is a Non-Volatile 
>>> Memory. And also a driver that implements the EFI Firmware Volume Block 
>>> protocol for this NVM device.
>
> But MdeModulePkg does Copymem from NV variable start memory to some allocated 
> buffers.  With SD/NAND Copymem is not possible, Is this something changes 
> since 2013 or there are some other way to use SD/NAND
>

No, SD/MMC cannot currently be used as the backing store for the EFI
variable store. The problem is that the variable protocols are
architectural protocols in PI that need to be present before any
driver model drivers are dispatched, and so putting the variable store
on block devices is not something that the PI software architecture
currently supports (unless you reimplement the whole driver stack as
DXE drivers).

On top of that, it is almost impossible to share a block device that
sits behind a controller between the firmware and the OS at runtime
(i.e., for SetVariable() calls made by efibootmgr under Linux),
because only a single agent can take ownership of the controller at
any given time. (You /could/ dedicate the SD/MMC to the firmware
entirely, and boot from SATA or USB, but this is out of the question
on most platforms that need to use SD/MMC for that variable backing
store, i.e., mobile platforms)

The best thing would be for you to convince the hardware architects in
your company to design and implement dual-ported SD/MMC controllers
that allow a single SD/MMC to have two logical views that are
independent (although I'm unsure if that is even possible in the
context of the SD/MMC specifications)

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

Re: [edk2] [PATCH] ArmPkg: ARM v8.2 updates for detecting FP

2017-09-18 Thread Evan Lloyd



> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: 15 September 2017 17:06
> To: Evan Lloyd 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Matteo Carlini ; Sami Mujawar
> 
> Subject: Re: [PATCH] ArmPkg: ARM v8.2 updates for detecting FP
>
> On Thu, Sep 14, 2017 at 05:11:21PM +0100, evan.ll...@arm.com wrote:
> > From: Sami Mujawar 
>
> I have fixed Sami's email address up before pushing.

 [[Evan Lloyd]]
Thanks, Leif.  That was a problem caused by an out of date config in the
original repo where the change was created.
Sami has fixed any others in our internal repo, so it shouldn't happen again.

>
> > The ARMv8.2-FP16 extension introduces support for half precision
> > floating point and the processor ID registers have been updated to
> > enable detection of the implementation.
> >
> > The possible values for the FP bits in ID_AA64PFR0_EL1[19:16] are:
> >   -  : Floating-point is implemented.
> >   - 0001 : Floating-point including Half-precision support is
> >implemented.
> >   -  : Floating-point is not implemented.
> >   - All other values are reserved.
> >
> > Previously ArmEnableVFP() compared the FP bits with b to see if
> > the FP was implemented, before enabling FP. Modified this check to
> > enable the FP if the FP bits 19:16 are not b.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Sami Mujawar 
> > Signed-off-by: Evan Lloyd 
>
> So ... I think longer-term we should try to move a lot of this type of stuff 
> to c
> instead, but this is a clear fix - thanks!

 [[Evan Lloyd]] You are most welcome.  We agree about the move to c, as 
assembler
code doesn't buy anything here. However, if you are going to change that
you probably have to do all of it - not a task I'd want to take on.

>
> Reviewed-by: Leif Lindholm  Pushed as
> 2f16993c25
>
> > ---
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > index
> >
> dde6a756528f3abf1bd5a142448e42122a9bd8fa..2d136d242b943fe2f365bc82
> 4953
> > b7fe10c944b7 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > @@ -1,7 +1,7 @@
> >
> > #-
> > -
> >  #
> >  # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved. -#
> > Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> > +# Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
> >  # Copyright (c) 2016, Linaro Limited. All rights reserved.
> >  #
> >  # This program and the accompanying materials @@ -403,9 +403,11 @@
> > ASM_FUNC(ArmEnableVFP)
> >mov   x1, x30 // Save LR
> >blArmReadIdPfr0   // Read EL1 Processor Feature Register 
> > (PFR0)
> >mov   x30, x1 // Restore LR
> > -  ands  x0, x0, #AARCH64_PFR0_FP// Extract bits indicating VFP
> implementation
> > -  cmp   x0, #0  // VFP is implemented if '0'.
> > -  b.ne  4f  // Exit if VFP not implemented.
> > +  ubfx  x0, x0, #16, #4 // Extract the FP bits 16:19
> > +  cmp   x0, #0xF// Check if FP bits are 'b',
> > +// i.e. Floating Point not implemented
> > +  b.eq  4f  // Exit when VFP is not implemented.
> > +
> >// FVP is implemented.
> >// Make sure VFP exceptions are not trapped (to any exception level).
> >mrs   x0, cpacr_el1   // Read EL1 Coprocessor Access Control 
> > Register
> (CPACR)
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ShellPkg/dmpstore: Show name of known variable vendor GUID

2017-09-18 Thread Carsey, Jaben

Whats the difference between HEADER_LINE and HEADER_LINE2?  They look the same 
to me...

> -Original Message-
> From: Ni, Ruiyu
> Sent: Sunday, September 17, 2017 11:42 PM
> To: edk2-devel@lists.01.org
> Cc: Li, Huajing ; Carsey, Jaben
> 
> Subject: [PATCH] ShellPkg/dmpstore: Show name of known variable vendor
> GUID
> Importance: High
> 
> From: Huajing Li 
> 
> Change "dmpstore" to show name of known variable vendor GUID.
> The name is got from ShellProtocol.GetGuidName().
> 
> Cc: Jaben Carsey 
> Reviewed-by: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Huajing Li 
> ---
>  ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c  | 17
> +
>  .../UefiShellDebug1CommandsLib.uni  |  1 +
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> index aeffc89b19..062ab5dc3a 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> @@ -424,6 +424,7 @@ CascadeProcessVariables (
>CHAR16*AttrString;
>CHAR16*HexString;
>EFI_STATUSSetStatus;
> +  CHAR16*GuidName;
> 
>if (ShellGetExecutionBreakFlag()) {
>  return (SHELL_ABORTED);
> @@ -521,10 +522,18 @@ CascadeProcessVariables (
>  Status = EFI_OUT_OF_RESOURCES;
>}
>  } else {
> -  ShellPrintHiiEx (
> --1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> -AttrString, , FoundVarName, DataSize
> -);
> +  Status = gEfiShellProtocol->GetGuidName(,
> );
> +  if (EFI_ERROR (Status)) {
> +ShellPrintHiiEx (
> +  -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> +  AttrString, , FoundVarName, DataSize
> +  );
> +  } else {
> +ShellPrintHiiEx (
> +  -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE2),
> gShellDebug1HiiHandle,
> +  AttrString, GuidName, FoundVarName, DataSize
> +  );
> +  }
>DumpHex (2, 0, DataSize, DataBuffer);
>  }
>  SHELL_FREE_NON_NULL (AttrString);
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index f733a67f0b..b6a133a454 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> @@ -385,6 +385,7 @@
>  #string STR_DMPSTORE_LOAD_GEN_FAIL #language en-US "%H%s%N:
> Failed to set variable %H%s%N: %r.\r\n"
>  #string STR_DMPSTORE_LOAD_BAD_FILE #language en-US "%H%s%N:
> Incorrect file format.\r\n"
>  #string STR_DMPSTORE_HEADER_LINE   #language en-US "Variable
> %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
> +#string STR_DMPSTORE_HEADER_LINE2  #language en-US "Variable
> %H%s%N '%H%s%N:%H%s%N' DataSize = 0x%02x\r\n"
>  #string STR_DMPSTORE_DELETE_LINE   #language en-US "Delete variable
> '%H%g%N:%H%s%N': %r\r\n"
>  #string STR_DMPSTORE_NO_VAR_FOUND  #language en-US "%H%s%N:
> No matching variables found.\r\n"
>  #string STR_DMPSTORE_NO_VAR_FOUND_SFO  #language en-US
> "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
> --
> 2.12.2.windows.2

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

[edk2] [PATCH 0/2] Use PCD for Watchdog count

2017-09-18 Thread evan . lloyd

From: EvanLloyd 

Paired patches for edk2 and edk2-platforms that add and use a PCD
indicating the number of SBSA Watchdog timers present.  This makes
things more configurable, and is consistent with other examples of
platform characteristics. It may also help with debates over GTDT
inclusion (or not) of secure timers.

Sami Mujawar (1):
  ArmPlatformPkg: Add PCD for SBSA Watchdog Count

 ArmPlatformPkg/ArmPlatformPkg.dec   | 4 +++-
 ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Sami Mujawar (1):
  Platforms/ARM: SBSA Watchdog PCD and build option

 Platform/ARM/JunoPkg/ArmJuno.dsc |  7 ++-
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc |  7 ++-
 Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf   |  4 +++-
 Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf   |  4 +++-
 Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc|  6 +-
 Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc| 14 +++---
 6 files changed, 34 insertions(+), 8 deletions(-)

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

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

[edk2] [PATCH 2/2] [edk2-platforms] Platforms/ARM: SBSA Watchdog PCD and build option

2017-09-18 Thread evan . lloyd

From: Sami Mujawar 

Added PcdWatchdogCount to specify the number of Watchdog timers
that are available on Juno and FVP platform.

Also added DISABLE_SBSA_WATCHDOG option to disable the watchdog
timers if required for testing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar 
Signed-off-by: Evan Lloyd 
---
 Platform/ARM/JunoPkg/ArmJuno.dsc |  7 ++-
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc |  7 ++-
 Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf   |  4 +++-
 Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf   |  4 +++-
 Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc|  6 +-
 Platform/ARM/VExpressPkg/AcpiTables/Gtdt.aslc| 14 +++---
 6 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index 
72eab8b5be22a5516b243b2b5e70f6b919854707..54158d709ae836202254eef8b2e0eac8209ca9cc
 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -131,6 +131,11 @@ [PcdsFixedAtBuild.common]
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x1C17
 
+  ## SBSA Watchdog Count
+!ifndef DISABLE_SBSA_WATCHDOG
+  gArmPlatformTokenSpaceGuid.PcdWatchdogCount|2
+!endif
+
   # LAN9118 Ethernet Driver
   gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress|0x1800
   gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress|0x1215161822242628
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index 
e9f954d926ac25a2abd2f97a4141267927dfc0a3..51f0529db2516285641a8b6a49473db9d3d9224a
 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -135,6 +135,11 @@ [PcdsFixedAtBuild.common]
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x1C17
 
+  ## SBSA Watchdog Count
+!ifndef DISABLE_SBSA_WATCHDOG
+  gArmPlatformTokenSpaceGuid.PcdWatchdogCount|1
+!endif
+
 !ifdef EDK2_ENABLE_PL111
   ## PL111 Versatile Express Motherboard controller
   gArmPlatformTokenSpaceGuid.PcdPL111LcdBase|0x1C1F
diff --git a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf 
b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
index 
e099c02f39dad4bb19692c87a12d8d3d6f1da4b6..741ea191be3672db225b82dc0e182fceddec83f2
 100644
--- a/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
+++ b/Platform/ARM/JunoPkg/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
 #
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2014-2016, ARM Ltd. All rights reserved.
+#  Copyright (c) 2014-2017, ARM Ltd. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -61,3 +61,5 @@ [FixedPcd]
   gArmPlatformTokenSpaceGuid.PL011UartInterrupt
 
   gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase
+
+  gArmPlatformTokenSpaceGuid.PcdWatchdogCount
diff --git a/Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf 
b/Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf
index 
59d323840a3a5e32a1a870f2098bdb2588ae91e1..cc0f06f533236b3bf68bfc6eefc745b776fdad16
 100644
--- a/Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf
+++ b/Platform/ARM/VExpressPkg/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
 #
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2014-2016, ARM Ltd. All rights reserved.
+#  Copyright (c) 2014-2017, ARM Ltd. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -41,3 +41,5 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdGicDistributorBase
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
   gArmTokenSpaceGuid.PcdGicRedistributorsBase
+
+  gArmPlatformTokenSpaceGuid.PcdWatchdogCount
diff --git a/Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc 
b/Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc
index 
5e83802d576b11178a7fdb556086dea41ee0859f..d6ed58c3ae2d20ae6201f131d9f39c95e713b9d2
 100644
--- a/Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc
+++ b/Platform/ARM/JunoPkg/AcpiTables/Gtdt.aslc
@@ -1,7 +1,7 @@
 /** @file
 *  Generic Timer Description Table (GTDT)
 *
-*  Copyright (c) 2012 - 2016, ARM Limited. All rights reserved.
+*

[edk2] [PATCH 1/2] ArmPlatformPkg: Add PCD for SBSA Watchdog Count

2017-09-18 Thread evan . lloyd

From: Sami Mujawar 

The Juno and FVP platform implement SBSA Watchdog timers.
Added PcdWatchdogCount to specify the number of Watchdog timers
that are available.

This allows configurability and an option to disable the Watchdog
timers if required for testing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar 
Signed-off-by: Evan Lloyd 
---
 ArmPlatformPkg/ArmPlatformPkg.dec   | 4 +++-
 ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index 
b8a6b131632dc6788b73a034a41b9e3176dffafa..2d82ead7612ae05c6f16f76008bde605a80ae5b9
 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -1,6 +1,6 @@
 #/** @file
 #
-#  Copyright (c) 2011-2016, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
 #  Copyright (c) 2015, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
@@ -131,6 +131,8 @@ [PcdsFixedAtBuild.common,PcdsDynamic.common]
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x0024
   gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|3|UINT32|0x0022
 
+  gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x0033
+
 [PcdsFixedAtBuild.ARM]
   # Stack for CPU Cores in Secure Monitor Mode
   gArmPlatformTokenSpaceGuid.PcdCPUCoresSecMonStackBase|0|UINT64|0x0007
diff --git a/ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h 
b/ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h
index 
7eaa239b1cbd4a2f54aa2d71e4e3b0279c5527d1..fce445ea9fe48a1012c714e40f95f3052713152d
 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h
+++ b/ArmPlatformPkg/ArmJunoPkg/Include/ArmPlatform.h
@@ -1,6 +1,6 @@
 /** @file
 *
-*  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+*  Copyright (c) 2013-2017, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD 
License
@@ -108,7 +108,7 @@
   JunoRevision = ARM_VE_BOARD_SYS_ID_REV( SysId ); \
 }
 
-#define JUNO_WATCHDOG_COUNT  2
+#define JUNO_WATCHDOG_COUNT  FixedPcdGet32 (PcdWatchdogCount)
 
 // Define if the exported ACPI Tables are based on ACPI 5.0 spec or latest
 //#define ARM_JUNO_ACPI_5_0
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

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

Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

2017-09-18 Thread Paulo Alcantara


Hi,

On 9/17/2017 9:42 PM, Ni, Ruiyu wrote:

Continue reading the patch #2, I think we can change IS_PD to:
#define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)

Using the above way, we can avoid caller to supply an invalid buffer.


Good point. I'll fix it.

Thanks!
Paulo



Thanks/Ray


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
Ruiyu
Sent: Monday, September 18, 2017 8:29 AM
To: Paulo Alcantara ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Laszlo Ersek
; Gao, Liming 
Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
definitions

#define _GET_TAG_ID(_Pointer) \
   (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)

#define IS_PD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
#define IS_LVD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
#define IS_TD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))

#define IS_AVDP(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))

Paulo,
If you take a look at Pci22.h in the same directory, all macros are started
as "IS_PCI_".
How about changing the above IS_xxx to IS_UDF_xxx?
Shall we change AVDP to AVD, following the same naming style as PD, LVD
and TD?
How about changing _Pointer to _Tag or DescriptorTag?


Thanks/Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 17, 2017 9:13 PM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Kinney, Michael D
; Gao, Liming ;

Laszlo

Ersek ; Ni, Ruiyu 
Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

This patch adds a fewe more UDF structures in order to detect Logical
Volume and Partition descriptors during Main Volume Descriptor Sequence
in Partition driver.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---
  MdePkg/Include/IndustryStandard/Udf.h | 63 
  1 file changed, 63 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Udf.h
b/MdePkg/Include/IndustryStandard/Udf.h
index 0febb4bcda..6cc42f8543 100644
--- a/MdePkg/Include/IndustryStandard/Udf.h
+++ b/MdePkg/Include/IndustryStandard/Udf.h
@@ -27,9 +27,19 @@
  #define _GET_TAG_ID(_Pointer) \
(((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)

+#define IS_PD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
+
  #define IS_AVDP(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))

+#define LV_UDF_REVISION(_Lv) \
+  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
+
  #pragma pack(1)

  typedef struct {
@@ -55,6 +65,59 @@ typedef struct {
UINT8   Reserved[480];
  } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;

+typedef struct {
+  UINT8   CharacterSetType;
+  UINT8   CharacterSetInfo[63];
+} UDF_CHAR_SPEC;
+
+typedef struct {
+  UINT8   Flags;
+  UINT8   Identifier[23];
+  UINT8   IdentifierSuffix[8];
+} UDF_ENTITY_ID;
+
+typedef struct {
+  UINT32LogicalBlockNumber;
+  UINT16PartitionReferenceNumber;
+} UDF_LB_ADDR;
+
+typedef struct {
+  UINT32   ExtentLength;
+  UDF_LB_ADDR  ExtentLocation;
+  UINT8ImplementationUse[6];
+} UDF_LONG_ALLOCATION_DESCRIPTOR;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG  DescriptorTag;
+  UINT32  VolumeDescriptorSequenceNumber;
+  UDF_CHAR_SPEC   DescriptorCharacterSet;
+  UINT8   LogicalVolumeIdentifier[128];
+  UINT32  LogicalBlockSize;
+  UDF_ENTITY_ID   DomainIdentifier;
+  UDF_LONG_ALLOCATION_DESCRIPTOR  LogicalVolumeContentsUse;
+  UINT32  MapTableLength;
+  UINT32  NumberOfPartitionMaps;
+  UDF_ENTITY_ID   ImplementationIdentifier;
+  UINT8   ImplementationUse[128];
+  UDF_EXTENT_AD   IntegritySequenceExtent;
+  UINT8   PartitionMaps[6];
+} UDF_LOGICAL_VOLUME_DESCRIPTOR;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG DescriptorTag;
+  UINT32 VolumeDescriptorSequenceNumber;
+  UINT16 PartitionFlags;
+  UINT16 PartitionNumber;
+  UDF_ENTITY_ID  PartitionContents;
+  UINT8  PartitionContentsUse[128];
+  UINT32 AccessType;
+  UINT32 PartitionStartingLocation;
+

Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

2017-09-18 Thread Paulo Alcantara


Hi Ruiyu,

On 9/17/2017 9:28 PM, Ni, Ruiyu wrote:

#define _GET_TAG_ID(_Pointer) \
   (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)

#define IS_PD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
#define IS_LVD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
#define IS_TD(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))

#define IS_AVDP(_Pointer) \
   ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))

Paulo,
If you take a look at Pci22.h in the same directory, all macros are started
as "IS_PCI_".
How about changing the above IS_xxx to IS_UDF_xxx?


Looks good to me. I'll change them.


Shall we change AVDP to AVD, following the same naming style as PD, LVD and TD?


There is no "Anchor Volume Descriptor" but "Anchor Volume Descriptor 
Pointer", so we still need to keep 'P'.



How about changing _Pointer to _Tag or DescriptorTag?


I will change it to _Tag. _Pointer is really a bad name. Thanks! :-)

Paulo




Thanks/Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 17, 2017 9:13 PM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Kinney, Michael D
; Gao, Liming ; Laszlo
Ersek ; Ni, Ruiyu 
Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions

This patch adds a fewe more UDF structures in order to detect Logical
Volume and Partition descriptors during Main Volume Descriptor Sequence
in Partition driver.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---
  MdePkg/Include/IndustryStandard/Udf.h | 63 
  1 file changed, 63 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Udf.h
b/MdePkg/Include/IndustryStandard/Udf.h
index 0febb4bcda..6cc42f8543 100644
--- a/MdePkg/Include/IndustryStandard/Udf.h
+++ b/MdePkg/Include/IndustryStandard/Udf.h
@@ -27,9 +27,19 @@
  #define _GET_TAG_ID(_Pointer) \
(((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)

+#define IS_PD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
+
  #define IS_AVDP(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))

+#define LV_UDF_REVISION(_Lv) \
+  *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
+
  #pragma pack(1)

  typedef struct {
@@ -55,6 +65,59 @@ typedef struct {
UINT8   Reserved[480];
  } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;

+typedef struct {
+  UINT8   CharacterSetType;
+  UINT8   CharacterSetInfo[63];
+} UDF_CHAR_SPEC;
+
+typedef struct {
+  UINT8   Flags;
+  UINT8   Identifier[23];
+  UINT8   IdentifierSuffix[8];
+} UDF_ENTITY_ID;
+
+typedef struct {
+  UINT32LogicalBlockNumber;
+  UINT16PartitionReferenceNumber;
+} UDF_LB_ADDR;
+
+typedef struct {
+  UINT32   ExtentLength;
+  UDF_LB_ADDR  ExtentLocation;
+  UINT8ImplementationUse[6];
+} UDF_LONG_ALLOCATION_DESCRIPTOR;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG  DescriptorTag;
+  UINT32  VolumeDescriptorSequenceNumber;
+  UDF_CHAR_SPEC   DescriptorCharacterSet;
+  UINT8   LogicalVolumeIdentifier[128];
+  UINT32  LogicalBlockSize;
+  UDF_ENTITY_ID   DomainIdentifier;
+  UDF_LONG_ALLOCATION_DESCRIPTOR  LogicalVolumeContentsUse;
+  UINT32  MapTableLength;
+  UINT32  NumberOfPartitionMaps;
+  UDF_ENTITY_ID   ImplementationIdentifier;
+  UINT8   ImplementationUse[128];
+  UDF_EXTENT_AD   IntegritySequenceExtent;
+  UINT8   PartitionMaps[6];
+} UDF_LOGICAL_VOLUME_DESCRIPTOR;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG DescriptorTag;
+  UINT32 VolumeDescriptorSequenceNumber;
+  UINT16 PartitionFlags;
+  UINT16 PartitionNumber;
+  UDF_ENTITY_ID  PartitionContents;
+  UINT8  PartitionContentsUse[128];
+  UINT32 AccessType;
+  UINT32 PartitionStartingLocation;
+  UINT32 PartitionLength;
+  UDF_ENTITY_ID  ImplementationIdentifier;
+  UINT8  ImplementationUse[128];
+  UINT8  Reserved[156];
+} UDF_PARTITION_DESCRIPTOR;
+
  #pragma pack()

  #endif
--
2.11.0




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

[edk2] Storing Non volatile variables on SD/NAND

2017-09-18 Thread Udit Kumar

Hi EDK-2 Experts, 
I am looking to store NV variables on SD/NAND device. 

While browsing, I came across some old post at link, 
http://feishare.com/efimail/messages/20130319-1700-Re__edk2__Regarding_storing_Boot_Device_Config_in_persistent_memory-Olivier_Martin.html
 

Looks like, this is possible easily. 
>> What you need to support Non-Volatile UEFI variables is a Non-Volatile 
>> Memory. And also a driver that implements the EFI Firmware Volume Block 
>> protocol for this NVM device.

But MdeModulePkg does Copymem from NV variable start memory to some allocated 
buffers.  With SD/NAND Copymem is not possible, Is this something changes since 
2013 or there are some other way to use SD/NAND 

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

Re: [edk2] [PATCH] Fix not able to change serial attributes

2017-09-18 Thread Pankaj Bansal

Hi Laszlo, Zeng

Thanks for patch review.

@Zeng: The sermode command calls SerialSetAttributes, which sets H/W attributes 
of Serial device.
After that the SerialIo protocol is reinstalled, which causes 
MdeModulePkg/Universal/Console/TerminalDxe
and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings to stop and 
then start.
This in turn calls SerialReset, which undoes changes of SerialSetAttributes.

Thanks & Regards,
Pankaj Bansal
-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com] 
Sent: Monday, September 18, 2017 3:52 PM
To: Laszlo Ersek ; Heyi Guo ; Pankaj 
Bansal ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Zeng, Star 
Subject: RE: [edk2] [PATCH] Fix not able to change serial attributes

Thanks for your good comments. :)
Since there is no clear description for the behavior of Reset() :(, I prone to 
align the behavior with MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c and 
IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c, that means I agree the 
fix.

Laszlo and Gary, if you can help do some simple regression test with the patch, 
that will be better.

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Sunday, September 17, 2017 4:18 AM
To: Zeng, Star ; Pankaj Bansal ; 
edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: Re: [edk2] [PATCH] Fix not able to change serial attributes

On 09/16/17 03:21, Zeng, Star wrote:
> Pankaj,
> 
> Thanks for the contribution.
> Could you help update the title to be like MdeModulePkg SerialDxe: XX?
> 
> In fact, I agree the fix as it matches the code at 
> MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c and 
> IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c.
> But I am curious about how you reproduce the issue by sermode command as I 
> see sermode command only calls SerialIo->SetAttributes() but not 
> SerialIo->Reset().
> 
> 
> Ray, Laszlo,
> Do you have any comment?

Thanks for the CC.

The UEFI spec is very superficial on EFI_SERIAL_IO_PROTOCOL.Reset(). It only 
says, "The Reset() function resets the hardware of a serial device." It doesn't 
define what state the device should return to after resetting the hardware.

Under the generic description of EFI_SERIAL_IO_PROTOCOL, we have

"The default attributes for all UART-style serial device interfaces are:
115,200 baud, a 1 byte receive FIFO, a 1,000,000 microsecond timeout per 
character, no parity, 8 data bits, and 1 stop bit."

Now, the PCDs that are currently used in the code may differ from the above 
standard-mandated values, but that's the platform's responsibility. The point 
is that the UEFI spec seems to require that the device be returned to a 
predefined state, and the PCDs make that possible. I don't think that the 
argument in the commit message, "Serial Reset command should set the attributes 
which have been changed by user after calling SerialSetAttributes.", is 
consistent with the UEFI spec's intent.

However, I could easily be wrong about this, given especially that the other 
two SerialIo implementations already follow the suggested practice.

I guess I'll have to stay neutral on this patch. Hopefully it won't regress 
anything.

Thanks,
Laszlo

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Pankaj Bansal
> Sent: Friday, September 15, 2017 9:14 PM
> To: edk2-devel@lists.01.org
> Cc: Pankaj Bansal 
> Subject: [edk2] [PATCH] Fix not able to change serial attributes
> 
> Issue : When try to change serial attributes using sermode command, the 
> default values are set Cause : The SerialReset command resets the attributes' 
> values to default Fix : Serial Reset command should set the attributes which 
> have been changed by user after calling SerialSetAttributes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pankaj Bansal 
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66
> -
>  1 file changed, 18 insertions(+), 48 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index 43d33db..dc7e13a 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -220,7 +220,6 @@ SerialReset (
>)
>  {
>EFI_STATUSStatus;
> -  EFI_TPL   Tpl;
>  
>Status = SerialPortInitialize ();
>if (EFI_ERROR (Status)) {
> @@ -228,49 +227,17 @@ SerialReset (
>}
>  
>//
> -  // Set the Serial I/O mode and update the device path
> -  //
> -
> -  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> -
> -  //
> -  // Set the Serial I/O mode
> -  //
> -  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
> -

Re: [edk2] [PATCH] Fix not able to change serial attributes

2017-09-18 Thread Zeng, Star

Thanks for your good comments. :)
Since there is no clear description for the behavior of Reset() :(, I prone to 
align the behavior with MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c and 
IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c, that means I agree the 
fix.


Laszlo and Gary, if you can help do some simple regression test with the patch, 
that will be better.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Sunday, September 17, 2017 4:18 AM
To: Zeng, Star ; Pankaj Bansal ; 
edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: Re: [edk2] [PATCH] Fix not able to change serial attributes

On 09/16/17 03:21, Zeng, Star wrote:
> Pankaj,
> 
> Thanks for the contribution.
> Could you help update the title to be like MdeModulePkg SerialDxe: XX?
> 
> In fact, I agree the fix as it matches the code at 
> MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c and 
> IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c.
> But I am curious about how you reproduce the issue by sermode command as I 
> see sermode command only calls SerialIo->SetAttributes() but not 
> SerialIo->Reset().
> 
> 
> Ray, Laszlo,
> Do you have any comment?

Thanks for the CC.

The UEFI spec is very superficial on EFI_SERIAL_IO_PROTOCOL.Reset(). It only 
says, "The Reset() function resets the hardware of a serial device." It doesn't 
define what state the device should return to after resetting the hardware.

Under the generic description of EFI_SERIAL_IO_PROTOCOL, we have

"The default attributes for all UART-style serial device interfaces are:
115,200 baud, a 1 byte receive FIFO, a 1,000,000 microsecond timeout per 
character, no parity, 8 data bits, and 1 stop bit."

Now, the PCDs that are currently used in the code may differ from the above 
standard-mandated values, but that's the platform's responsibility. The point 
is that the UEFI spec seems to require that the device be returned to a 
predefined state, and the PCDs make that possible. I don't think that the 
argument in the commit message, "Serial Reset command should set the attributes 
which have been changed by user after calling SerialSetAttributes.", is 
consistent with the UEFI spec's intent.

However, I could easily be wrong about this, given especially that the other 
two SerialIo implementations already follow the suggested practice.

I guess I'll have to stay neutral on this patch. Hopefully it won't regress 
anything.

Thanks,
Laszlo


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Pankaj Bansal
> Sent: Friday, September 15, 2017 9:14 PM
> To: edk2-devel@lists.01.org
> Cc: Pankaj Bansal 
> Subject: [edk2] [PATCH] Fix not able to change serial attributes
> 
> Issue : When try to change serial attributes using sermode command, the 
> default values are set Cause : The SerialReset command resets the attributes' 
> values to default Fix : Serial Reset command should set the attributes which 
> have been changed by user after calling SerialSetAttributes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Pankaj Bansal 
> ---
>  MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 
> -
>  1 file changed, 18 insertions(+), 48 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
> b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> index 43d33db..dc7e13a 100644
> --- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> +++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
> @@ -220,7 +220,6 @@ SerialReset (
>)
>  {
>EFI_STATUSStatus;
> -  EFI_TPL   Tpl;
>  
>Status = SerialPortInitialize ();
>if (EFI_ERROR (Status)) {
> @@ -228,49 +227,17 @@ SerialReset (
>}
>  
>//
> -  // Set the Serial I/O mode and update the device path
> -  //
> -
> -  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
> -
> -  //
> -  // Set the Serial I/O mode
> -  //
> -  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
> -  This->Mode->Timeout   = 1000 * 1000;
> -  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
> -  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
> -  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
> -  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
> -
> -  //
> -  // Check if the device path has actually changed
> -  //
> -  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
> -  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&
> -  mSerialDevicePath.Uart.Parity   == (UINT8) This->Mode->Parity &&
> -  mSerialDevicePath.Uart.StopBits == (UINT8) This->Mode->StopBits
> - ) {
> -gBS->RestoreTPL (Tpl);
> -return EFI_SUCCESS;
> -  }
> -
> -  //
> -  // Update the device

Re: [edk2] [PATCH v2] ArmPkg/PlatformBootManagerLib: process pending capsules

2017-09-18 Thread Leif Lindholm

On Fri, Sep 15, 2017 at 04:03:32PM -0700, Ard Biesheuvel wrote:
> Process any capsule HOBs that were left for us by CapsulePei. This
> involves calling ProcessCapsules() twice, as explained in the comment
> in DxeCapsuleLibFmp [sic].
> 
> 1) The first call must be before EndOfDxe. The system capsules is processed.
>If device capsule FMP protocols are exposted at this time and device FMP
>capsule has zero EmbeddedDriverCount, the device capsules are processed.
>Each individual capsule result is recorded in capsule record variable.
>System may reset in this function, if reset is required by capsule and
>all capsules are processed.
>If not all capsules are processed, reset will be defered to second call.
> 
> 2) The second call must be after EndOfDxe and after ConnectAll, so that all
>device capsule FMP protocols are exposed.
>The system capsules are skipped. If the device capsules are NOT processed
>in first call, they are processed here.
>Each individual capsule result is recorded in capsule record variable.
>System may reset in this function, if reset is required by capsule
>processed in first call and second call.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

/
Leif

> ---
> v2: add CapsuleLib resolution to ArmPkg.dsc
> 
>  ArmPkg/ArmPkg.dsc|  1 +
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c   | 16 
> 
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  2 ++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index cf86f89bd702..fc24a788df57 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -43,6 +43,7 @@
>BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
>BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>
> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
>HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c 
> b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 6c0b352ae366..a3b2d7925f72 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -18,7 +18,9 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -447,6 +449,14 @@ PlatformBootManagerBeforeConsole (
>VOID
>)
>  {
> +  EFI_STATUSStatus;
> +
> +  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
> +DEBUG ((DEBUG_INFO, "ProcessCapsules Before EndOfDxe ..\n"));
> +Status = ProcessCapsules ();
> +DEBUG ((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
> +  }
> +
>//
>// Signal EndOfDxe PI Event
>//
> @@ -528,6 +538,12 @@ PlatformBootManagerAfterConsole (
>//
>EfiBootManagerConnectAll ();
>  
> +  if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) {
> +DEBUG((DEBUG_INFO, "ProcessCapsules After EndOfDxe ..\n"));
> +Status = ProcessCapsules ();
> +DEBUG((DEBUG_INFO, "ProcessCapsules returned %r\n", Status));
> +  }
> +
>//
>// Enumerate all possible boot options.
>//
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index e5ffd5db4276..58c4d6d2c7d6 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -43,9 +43,11 @@
>BaseLib
>BaseMemoryLib
>BootLogoLib
> +  CapsuleLib
>DebugLib
>DevicePathLib
>DxeServicesLib
> +  HobLib
>MemoryAllocationLib
>PcdLib
>PrintLib
> -- 
> 2.11.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change serial attributes

2017-09-18 Thread Zeng, Star

Pankaj,

Thanks for the update.

I raised a question in the V1 patch review, could you help kindly provide the 
feedback?
"how you reproduce the issue by sermode command as I see sermode command only 
calls SerialIo->SetAttributes() but not SerialIo->Reset()"


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pankaj 
Bansal
Sent: Monday, September 18, 2017 3:43 PM
To: edk2-devel@lists.01.org
Cc: Pankaj Bansal 
Subject: [edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change 
serial attributes

Issue : When try to change serial attributes using sermode command, the default 
values are set Cause : The SerialReset command resets the attributes' values to 
default Fix : Serial Reset command should set the attributes which have been 
changed by user after calling SerialSetAttributes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---
Changes in v2:
   - Modified Patch description

 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 -
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index 43d33db..dc7e13a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -220,7 +220,6 @@ SerialReset (
   )
 {
   EFI_STATUSStatus;
-  EFI_TPL   Tpl;
 
   Status = SerialPortInitialize ();
   if (EFI_ERROR (Status)) {
@@ -228,49 +227,17 @@ SerialReset (
   }
 
   //
-  // Set the Serial I/O mode and update the device path
-  //
-
-  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
-
-  //
-  // Set the Serial I/O mode
-  //
-  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
-  This->Mode->Timeout   = 1000 * 1000;
-  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
-  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
-  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
-  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
-
-  //
-  // Check if the device path has actually changed
-  //
-  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
-  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&
-  mSerialDevicePath.Uart.Parity   == (UINT8) This->Mode->Parity &&
-  mSerialDevicePath.Uart.StopBits == (UINT8) This->Mode->StopBits
- ) {
-gBS->RestoreTPL (Tpl);
-return EFI_SUCCESS;
-  }
-
-  //
-  // Update the device path
+  // Go set the current attributes
   //
-  mSerialDevicePath.Uart.BaudRate = This->Mode->BaudRate;
-  mSerialDevicePath.Uart.DataBits = (UINT8) This->Mode->DataBits;
-  mSerialDevicePath.Uart.Parity   = (UINT8) This->Mode->Parity;
-  mSerialDevicePath.Uart.StopBits = (UINT8) This->Mode->StopBits;
-
-  Status = gBS->ReinstallProtocolInterface (
-  mSerialHandle,
-  ,
-  ,
-  
-  );
-
-  gBS->RestoreTPL (Tpl);
+  Status = This->SetAttributes (
+   This,
+   This->Mode->BaudRate,
+   This->Mode->ReceiveFifoDepth,
+   This->Mode->Timeout,
+   (EFI_PARITY_TYPE) This->Mode->Parity,
+   (UINT8) This->Mode->DataBits,
+   (EFI_STOP_BITS_TYPE) This->Mode->StopBits
+   );
 
   return Status;
 }
@@ -513,11 +480,6 @@ SerialDxeInitialize (  {
   EFI_STATUSStatus;
 
-  Status = SerialPortInitialize ();
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
   mSerialIoMode.BaudRate = PcdGet64 (PcdUartDefaultBaudRate);
   mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
   mSerialIoMode.Parity   = (UINT32) PcdGet8 (PcdUartDefaultParity);
@@ -529,6 +491,14 @@ SerialDxeInitialize (
   mSerialDevicePath.Uart.StopBits = PcdGet8 (PcdUartDefaultStopBits);
 
   //
+  // Issue a reset to initialize the COM port  //  Status = 
+ mSerialIoTemplate.Reset ();  if (EFI_ERROR (Status)) 
+ {
+return Status;
+  }
+
+  //
   // Make a new handle with Serial IO protocol and its device path on it.
   //
   Status = gBS->InstallMultipleProtocolInterfaces (
--
2.7.4

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

[edk2] [Patch][edk2-platforms/minnowboard-max-udk2017] Vlv2TbltDevicePkg: Change firmware update debuglib to Null.

2017-09-18 Thread Guo, Mang

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Guo Mang 
---
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 07c007f..f9b9faa 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -1251,17 +1251,13 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
   SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf {
 
   
FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
-!if $(TARGET) != RELEASE
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!endif
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
   }
 
   SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
 
   
FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256/FmpAuthenticationLibRsa2048Sha256.inf
-!if $(TARGET) != RELEASE
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!endif
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
   }
 
   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf {
-- 
2.10.1.windows.1

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

[edk2] [PATCH v4 4/6] MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore

2017-09-18 Thread Dandan Bi

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

For oneof/numeric/checkbox, their storage may be bit field.
When generating  string to get default value
for these questions, we need to parse the Ifr data to get
the bit Varstore info,and then generating the correct
 string.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   | 346 +++--
 .../Universal/HiiDatabaseDxe/HiiDatabase.h |   6 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf|   3 +-
 3 files changed, 319 insertions(+), 36 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index c9ff1cf..f7018a4 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -1223,19 +1223,19 @@ InsertBlockData (
   // Insert block data in its Offset and Width order.
   //
   for (Link = BlockLink->ForwardLink; Link != BlockLink; Link = 
Link->ForwardLink) {
 BlockArray = BASE_CR (Link, IFR_BLOCK_DATA, Entry);
 if (BlockArray->Offset == BlockSingleData->Offset) {
-  if (BlockArray->Width > BlockSingleData->Width) {
+  if ((BlockArray->Width > BlockSingleData->Width) || 
(BlockSingleData->IsBitVar && BlockArray->Width == BlockSingleData->Width)) {
 //
 // Insert this block data in the front of block array
 //
 InsertTailList (Link, >Entry);
 return;
   }
 
-  if (BlockArray->Width == BlockSingleData->Width) {
+  if ((!BlockSingleData->IsBitVar) && BlockArray->Width == 
BlockSingleData->Width) {
 //
 // The same block array has been added.
 //
 if (BlockSingleData != BlockArray) {
   FreePool (BlockSingleData);
@@ -1978,10 +1978,11 @@ Done:
   @param  HiiHandle  The hii handle for this form package.
   @param  VarStorageData The varstore data structure.
   @param  IfrOpHdr   Ifr opcode header for this opcode.
   @param  VarWidth   The buffer width for this opcode.
   @param  ReturnData The data block added for this opcode.
+  @param  IsBitVar   Whether the the opcode refers to bit storage.
 
   @retval  EFI_SUCCESS   This opcode is required.
   @retval  EFI_NOT_FOUND This opcode is not required.
   @retval  OthersContain some error.
  
@@ -1991,20 +1992,26 @@ IsThisOpcodeRequired (
   IN IFR_BLOCK_DATA   *RequestBlockArray,
   IN EFI_HII_HANDLE   HiiHandle,
   IN OUT IFR_VARSTORAGE_DATA  *VarStorageData,
   IN EFI_IFR_OP_HEADER*IfrOpHdr,
   IN UINT16   VarWidth,
-  OUTIFR_BLOCK_DATA   **ReturnData
+  OUTIFR_BLOCK_DATA   **ReturnData,
+  IN BOOLEAN  IsBitVar
   )
 {
   IFR_BLOCK_DATA   *BlockData;
   UINT16   VarOffset;
   EFI_STRING_IDNameId;
   EFI_IFR_QUESTION_HEADER  *IfrQuestionHdr;
+  UINT16   BitOffset;
+  UINT16   BitWidth;
+  UINT16   TotalBits;
 
   NameId= 0;
   VarOffset = 0;
+  BitOffset = 0;
+  BitWidth = 0;
   IfrQuestionHdr = (EFI_IFR_QUESTION_HEADER  *)((CHAR8 *) IfrOpHdr + sizeof 
(EFI_IFR_OP_HEADER));
 
   if (VarStorageData->Type == EFI_HII_VARSTORE_NAME_VALUE) {
 NameId = IfrQuestionHdr->VarStoreInfo.VarName;
 
@@ -2016,11 +2023,27 @@ IsThisOpcodeRequired (
   // This question is not in the requested string. Skip it.
   //
   return EFI_NOT_FOUND;
 }
   } else {
-VarOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+//
+// Get the byte offset/with and bit offset/width
+//
+if (IsBitVar) {
+  BitOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+  BitWidth = VarWidth;
+  VarOffset = BitOffset / 8;
+  //
+  // Use current bit width and the bit width before current bit (with same 
byte offset) to calculate the byte width.
+  //
+  TotalBits = BitOffset % 8 + BitWidth;
+  VarWidth = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+} else {
+  VarOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+  BitWidth = VarWidth;
+  BitOffset = VarOffset * 8;
+}
 
 //
 // Check whether this question is in requested block array.
 //
 if (!BlockArrayCheck (RequestBlockArray, VarOffset, VarWidth, FALSE, 
HiiHandle)) {
@@ -2051,10 +2074,13 @@ IsThisOpcodeRequired (
 
   BlockData->Width  = VarWidth;
   BlockData->QuestionId = IfrQuestionHdr->QuestionId;
   BlockData->OpCode = IfrOpHdr->OpCode;
   BlockData->Scope  = IfrOpHdr->Scope;
+  BlockData->IsBitVar   = IsBitVar;
+  BlockData->BitOffset  = BitOffset;
+  BlockData->BitWidth   = BitWidth;

[edk2] [PATCH v4 3/6] MdeModulePkg/UefiHiiLib: Validate question with bit fields

2017-09-18 Thread Dandan Bi

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

In UefiHiiLib, there are codes to validate the current setting of
questions, now update the logic to handle question with bit storage.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 246 ---
 MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h |   4 +-
 MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf   |   5 +-
 3 files changed, 178 insertions(+), 77 deletions(-)

diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c 
b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
index 583b9e5..e1341ef 100644
--- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
+++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
@@ -1167,10 +1167,16 @@ ValidateQuestionFromVfr (
   EFI_IFR_STRING   *IfrString;
   CHAR8*VarStoreName;
   UINTNIndex;
   CHAR16   *QuestionName;
   CHAR16   *StringPtr;
+  UINT16   BitOffset;
+  UINT16   BitWidth;
+  UINT16   TotalBits;
+  UINTNStartBit;
+  UINTNEndBit;
+  BOOLEAN  QuestionReferBitField;
 
   //
   // Initialize the local variables.
   //
   Index = 0;
@@ -1180,10 +1186,13 @@ ValidateQuestionFromVfr (
   IfrVarStore   = NULL;
   IfrNameValueStore = NULL;
   IfrEfiVarStore= NULL;
   ZeroMem (, sizeof (IFR_VARSTORAGE_DATA));
   ZeroMem (, sizeof (VarBlockData));
+  BitOffset = 0;
+  BitWidth = 0;
+  QuestionReferBitField = FALSE;
 
   //
   // Check IFR value is in block data, then Validate Value
   //
   PackageOffset = sizeof (EFI_HII_PACKAGE_LIST_HEADER);
@@ -1343,12 +1352,23 @@ ValidateQuestionFromVfr (
 }
   } else {
 //
 // Get Offset by Question header and Width by DataType Flags
 //
-Offset = IfrOneOf->Question.VarStoreInfo.VarOffset;
-Width  = (UINT16) (1 << (IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE));
+if (QuestionReferBitField) {
+  //
+  // Get the byte offset/width for bit field.
+  //
+  BitOffset = IfrOneOf->Question.VarStoreInfo.VarOffset;
+  BitWidth = IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE_BIT;
+  Offset = BitOffset / 8;
+  TotalBits = BitOffset % 8 + BitWidth;
+  Width = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+} else {
+  Offset = IfrOneOf->Question.VarStoreInfo.VarOffset;
+  Width  = (UINT16) (1 << (IfrOneOf->Flags & 
EFI_IFR_NUMERIC_SIZE));
+}
 //
 // Check whether this question is in current block array.
 //
 if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
   //
@@ -1368,11 +1388,20 @@ ValidateQuestionFromVfr (
 
 //
 // Get the current value for oneof opcode
 //
 VarValue = 0;
-CopyMem (, VarBuffer +  Offset, Width);
+if (QuestionReferBitField) {
+  //
+  // Get the value in bit fields.
+  //
+  StartBit = BitOffset % 8;
+  EndBit = StartBit + BitWidth - 1;
+  VarValue = BitFieldRead32 (*(UINT32*)(VarBuffer + Offset), 
StartBit, EndBit);
+} else {
+  CopyMem (, VarBuffer +  Offset, Width);
+}
   }
   //
   // Set Block Data, to be checked in the following Oneof option 
opcode.
   //
   VarBlockData.OpCode = IfrOpHdr->OpCode;
@@ -1414,12 +1443,23 @@ ValidateQuestionFromVfr (
 }
   } else {
 //
 // Get Offset by Question header and Width by DataType Flags
 //
-Offset = IfrNumeric->Question.VarStoreInfo.VarOffset;
-Width  = (UINT16) (1 << (IfrNumeric->Flags & 
EFI_IFR_NUMERIC_SIZE));
+if (QuestionReferBitField) {
+  //
+  // Get the byte offset/width for bit field.
+  //
+  BitOffset = IfrNumeric->Question.VarStoreInfo.VarOffset;
+  BitWidth = IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE_BIT;
+  Offset = BitOffset / 8;
+  TotalBits = BitOffset % 8 + BitWidth;
+  Width  = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+} else {
+  Offset = IfrNumeric->Question.VarStoreInfo.VarOffset;
+  Width  = (UINT16) (1 << (IfrNumeric->Flags & 
EFI_IFR_NUMERIC_SIZE));
+}
 //
 // Check whether this question is in current block array.
 //
 if (!BlockArrayCheck (CurrentBlockArray, Offset,

[edk2] [PATCH v4 5/6] MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore

2017-09-18 Thread Dandan Bi

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

For oneof/numeric/CheckBox(storage can be Bit VarStore)
If the question value can be updated and shown correctly
in UI page, we need do enhancements in following cases:
1. Parse the Ifr data to get the bit VarStore info correctly.
2. Set/get value to/from bit VarStore correctly.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  | 138 -
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 110 ++--
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h |   3 +
 .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf  |   3 +-
 4 files changed, 212 insertions(+), 42 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 6b3e5e0..821e282 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -57,10 +57,11 @@ CreateStatement (
 
   Statement->Signature = FORM_BROWSER_STATEMENT_SIGNATURE;
 
   Statement->Operand = ((EFI_IFR_OP_HEADER *) OpCodeData)->OpCode;
   Statement->OpCode  = (EFI_IFR_OP_HEADER *) OpCodeData;
+  Statement->QuestionReferToBitField = FALSE;
 
   StatementHdr = (EFI_IFR_STATEMENT_HEADER *) (OpCodeData + sizeof 
(EFI_IFR_OP_HEADER));
   CopyMem (>Prompt, >Prompt, sizeof (EFI_STRING_ID));
   CopyMem (>Help, >Help, sizeof (EFI_STRING_ID));
 
@@ -1312,10 +1313,12 @@ ParseOpCodes (
   BOOLEAN InUnknownScope;
   UINT8   UnknownDepth;
   FORMSET_DEFAULTSTORE*PreDefaultStore;
   LIST_ENTRY  *DefaultLink;
   BOOLEAN HaveInserted;
+  UINT16  TotalBits;
+  BOOLEAN QuestionReferBitField;
 
   SuppressForQuestion  = FALSE;
   SuppressForOption= FALSE;
   InScopeDisable   = FALSE;
   DepthOfDisable   = 0;
@@ -1333,10 +1336,11 @@ ParseOpCodes (
   MapExpressionList= NULL;
   TempVarstoreId   = 0;
   ConditionalExprCount = 0;
   InUnknownScope   = FALSE;
   UnknownDepth = 0;
+  QuestionReferBitField= FALSE;
 
   //
   // Get the number of Statements and Expressions
   //
   CountOpCodes (FormSet, , );
@@ -1978,47 +1982,98 @@ ParseOpCodes (
   ASSERT(CurrentStatement != NULL);
 
   CurrentStatement->Flags = ((EFI_IFR_ONE_OF *) OpCodeData)->Flags;
   Value = >HiiValue;
 
-  switch (CurrentStatement->Flags & EFI_IFR_NUMERIC_SIZE) {
-  case EFI_IFR_NUMERIC_SIZE_1:
-CurrentStatement->Minimum = ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u8.MinValue;
-CurrentStatement->Maximum = ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u8.MaxValue;
-CurrentStatement->Step= ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u8.Step;
-CurrentStatement->StorageWidth = (UINT16) sizeof (UINT8);
-Value->Type = EFI_IFR_TYPE_NUM_SIZE_8;
-break;
+  if (QuestionReferBitField) {
+//
+// Get the bit var store info (bit/byte offset, bit/byte offset)
+//
+CurrentStatement->QuestionReferToBitField = TRUE;
+CurrentStatement->BitStorageWidth = CurrentStatement->Flags & 
EFI_IFR_NUMERIC_SIZE_BIT;
+CurrentStatement->BitVarOffset = 
CurrentStatement->VarStoreInfo.VarOffset;
+CurrentStatement->VarStoreInfo.VarOffset = 
CurrentStatement->BitVarOffset / 8;
+TotalBits = CurrentStatement->BitVarOffset % 8 + 
CurrentStatement->BitStorageWidth;
+CurrentStatement->StorageWidth = (TotalBits % 8 == 0? TotalBits / 8: 
TotalBits / 8 + 1);
 
-  case EFI_IFR_NUMERIC_SIZE_2:
-CopyMem (>Minimum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u16.MinValue, sizeof (UINT16));
-CopyMem (>Maximum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u16.MaxValue, sizeof (UINT16));
-CopyMem (>Step,&((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u16.Step, sizeof (UINT16));
-CurrentStatement->StorageWidth = (UINT16) sizeof (UINT16);
-Value->Type = EFI_IFR_TYPE_NUM_SIZE_16;
-break;
+//
+// Get the Minimum/Maximum/Step value(Note: bit field type has been 
stored as UINT32 type)
+//
+CurrentStatement->Minimum = ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.MinValue;
+CurrentStatement->Maximum = ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.MaxValue;
+CurrentStatement->Step= ((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.Step;
 
-  case EFI_IFR_NUMERIC_SIZE_4:
-CopyMem (>Minimum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.MinValue, sizeof (UINT32));
-CopyMem (>Maximum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.MaxValue, sizeof (UINT32));
-CopyMem (>Step,&((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.Step, sizeof (UINT32));
-CurrentStatement->StorageWidth

[edk2] [PATCH v4 6/6] MdeModulePkg/DriverSample: Add questions with bit/union VarStore

2017-09-18 Thread Dandan Bi

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

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Universal/DriverSampleDxe/DriverSample.c   | 120 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |   2 +
 .../Universal/DriverSampleDxe/NVDataStruc.h|  34 +++-
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 172 +
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  63 
 5 files changed, 390 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index bbd9713..af31615 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -18,10 +18,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define DISPLAY_ONLY_MY_ITEM  0x0002
 
 CHAR16 VariableName[] = L"MyIfrNVData";
 CHAR16 MyEfiVar[] = L"MyEfiVar";
+CHAR16 MyEfiBitVar[] = L"MyEfiBitVar";
+CHAR16 MyEfiUnionVar[] = L"MyEfiUnionVar";
+
 EFI_HANDLE  DriverHandle[2] = {NULL, NULL};
 DRIVER_SAMPLE_PRIVATE_DATA  *mPrivateData = NULL;
 EFI_EVENT   mEvent;
 
 HII_VENDOR_DEVICE_PATH  mHiiVendorDevicePath0 = {
@@ -662,10 +665,17 @@ ExtractConfig (
 // through hii database, not support in this path.
 //
 if (HiiIsConfigHdrMatch(Request, , MyEfiVar)) {
   return EFI_UNSUPPORTED;
 }
+if (HiiIsConfigHdrMatch(Request, , MyEfiBitVar)) {
+  return EFI_UNSUPPORTED;
+}
+if (HiiIsConfigHdrMatch(Request, , 
MyEfiUnionVar)) {
+  return EFI_UNSUPPORTED;
+}
+
 //
 // Set Request to the unified request string.
 //
 ConfigRequest = Request;
 //
@@ -883,10 +893,16 @@ RouteConfig (
   // through hii database, not support in this path.
   //
   if (HiiIsConfigHdrMatch(Configuration, , MyEfiVar)) 
{
 return EFI_UNSUPPORTED;
   }
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyEfiBitVar)) {
+return EFI_UNSUPPORTED;
+  }
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyEfiUnionVar)) {
+return EFI_UNSUPPORTED;
+  }
 
   //
   // Get Buffer Storage data from EFI variable
   //
   BufferSize = sizeof (DRIVER_SAMPLE_CONFIGURATION);
@@ -1295,10 +1311,14 @@ DriverCallback (
 for (Index = 0; Index < 3; Index ++) {
   SetArrayData (Value, EFI_IFR_TYPE_NUM_SIZE_8, Index, BufferValue--);
 }
   break;
 
+  case 0x:
+Value->u8 = 12;
+break;
+
   default:
 Status = EFI_UNSUPPORTED;
   break;
   }
 }
@@ -1309,10 +1329,14 @@ DriverCallback (
   switch (QuestionId) {
   case 0x1240:
 Value->u8 = DEFAULT_CLASS_MANUFACTURING_VALUE;
   break;
 
+ case 0x:
+Value->u8 = 13;
+break;
+
   default:
 Status = EFI_UNSUPPORTED;  
   break;
   }
 }
@@ -1703,10 +1727,12 @@ DriverSampleInit (
   DRIVER_SAMPLE_CONFIGURATION *Configuration;
   BOOLEAN ActionFlag;
   EFI_STRING  ConfigRequestHdr;
   EFI_STRING  NameRequestHdr;
   MY_EFI_VARSTORE_DATA*VarStoreConfig;
+  MY_EFI_BITS_VARSTORE_DATA   *BitsVarStoreConfig;
+  MY_EFI_UNION_DATA   *UnionConfig;
   EFI_INPUT_KEY   HotKey;
   EDKII_FORM_BROWSER_EXTENSION_PROTOCOL *FormBrowserEx;
 
   //
   // Initialize the local variables.
@@ -1991,10 +2017,104 @@ DriverSampleInit (
   return EFI_INVALID_PARAMETER;
 }
   }
   FreePool (ConfigRequestHdr);
 
+  //
+  // Initialize Bits efi varstore configuration data
+  //
+  BitsVarStoreConfig = >BitsVarStoreConfig;
+  ZeroMem (BitsVarStoreConfig, sizeof (MY_EFI_BITS_VARSTORE_DATA));
+
+  ConfigRequestHdr = HiiConstructConfigHdr (, 
MyEfiBitVar, DriverHandle[0]);
+  ASSERT (ConfigRequestHdr != NULL);
+
+  BufferSize = sizeof (MY_EFI_BITS_VARSTORE_DATA);
+  Status = gRT->GetVariable (MyEfiBitVar, , NULL, 
, BitsVarStoreConfig);
+  if (EFI_ERROR (Status)) {
+//
+// Store zero data to EFI variable Storage.
+//
+Status = gRT->SetVariable(
+MyEfiBitVar,
+,
+EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS,
+sizeof (MY_EFI_BITS_VARSTORE_DATA),
+BitsVarStoreConfig
+);
+if (EFI_ERROR (Status)) {
+  DriverSampleUnload (ImageHandle);
+  return Status;
+}
+//
+// EFI variable for NV config doesn't exit, we should build this variable
+// based on default values stored in IFR
+//
+ActionFlag = HiiSetToDefaults (ConfigRequestHdr, 
EFI_HII_DEFAULT_CLASS_STANDARD);
+if (!ActionFlag) {
+  DriverSampleUnload (ImageHandle);
+  return EFI_INVALID_PARAMETER;

[edk2] [PATCH v4 2/6] MdeModulePkg: Add GUID/flags to implement BitField support

2017-09-18 Thread Dandan Bi

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

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Include/Guid/MdeModuleHii.h | 20 +++-
 MdeModulePkg/MdeModulePkg.dec|  4 
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Include/Guid/MdeModuleHii.h 
b/MdeModulePkg/Include/Guid/MdeModuleHii.h
index 81821da..4ef4db8 100644
--- a/MdeModulePkg/Include/Guid/MdeModuleHii.h
+++ b/MdeModulePkg/Include/Guid/MdeModuleHii.h
@@ -1,9 +1,9 @@
 /** @file
   EDKII extented HII IFR guid opcodes.
 
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.

 
@@ -209,12 +209,30 @@ typedef struct _EFI_IFR_GUID_VAREQNAME {
   /// The the Unicode String will be used as a EFI Variable Name.
   ///
   UINT16  NameId;
 } EFI_IFR_GUID_VAREQNAME;
 
+///
+/// EDKII implementation extension GUID, used to indaicate there are bit 
fields in the varstore.
+///
+#define EFI_IFR_BITVARSTORE_GUID \
+  {0x82DDD68B, 0x9163, 0x4187, {0x9B, 0x27, 0x20, 0xA8, 0xFD, 0x60,0xA7, 0x1D}}
+
+///
+/// EDKII implementation extension flags, used to indaicate the disply style 
and bit width for bit filed storage.
+/// Two high bits for display style and the low six bits for bit width.
+///
+#define EFI_IFR_DISPLAY_BIT0xC0
+#define EFI_IFR_DISPLAY_INT_DEC_BIT0x00
+#define EFI_IFR_DISPLAY_UINT_DEC_BIT   0x40
+#define EFI_IFR_DISPLAY_UINT_HEX_BIT   0x80
+
+#define EFI_IFR_NUMERIC_SIZE_BIT   0x3F
+
 #pragma pack()
 
 extern EFI_GUID gEfiIfrTianoGuid;
 extern EFI_GUID gEfiIfrFrameworkGuid;
+extern EFI_GUID gEfiIfrBitvarstoreGuid;
 
 #endif
 
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 403a66a..6fd11fa 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -182,10 +182,14 @@
 
   ## Guid for EDKII implementation GUIDed opcodes
   #  Include/Guid/MdeModuleHii.h
   gEfiIfrTianoGuid  = { 0xf0b1735, 0x87a0, 0x4193, {0xb2, 0x66, 0x53, 
0x8c, 0x38, 0xaf, 0x48, 0xce }}
 
+  ## Guid for EDKII implementation extension, used to indaicate there are bit 
fields in the varstore.
+  #  Include/Guid/MdeModuleHii.h
+  gEfiIfrBitvarstoreGuid  = {0x82DDD68B, 0x9163, 0x4187, {0x9B, 0x27, 0x20, 
0xA8, 0xFD, 0x60,0xA7, 0x1D}}
+
   ## Guid for Framework vfr GUIDed opcodes.
   #  Include/Guid/MdeModuleHii.h
   gEfiIfrFrameworkGuid  = { 0x31ca5d1a, 0xd511, 0x4931, { 0xb7, 0x82, 0xae, 
0x6b, 0x2b, 0x17, 0x8c, 0xd7 }}
 
   ## Guid to specify the System Non Volatile FV
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH v4 0/6] Support bitfield in storage of vfr

2017-09-18 Thread Dandan Bi

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

This patch series is to:
(1) Update VfrCompiler to parse bit field in data structure.
(2) Update HiiDatabase,SetupBrowser and UefiHiiLib to handle questions with Bit 
VarStore.
(3) Add sample questions with bit/union VarStore in DriverSample.

Cc: Eric Dong 
Cc: Liming Gao 

Dandan Bi (6):
  BaseTool/VfrCompiler: Support Bit fields in EFI/Buffer VarStore
  MdeModulePkg: Add GUID/flags to implement BitField support
  MdeModulePkg/UefiHiiLib: Add codes to validate question with bit
fields
  MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore
  MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore
  MdeModulePkg/DriverSample: Add sample questions with bit/union
VarStore

 BaseTools/Source/C/Include/Common/MdeModuleHii.h   |   13 +-
 BaseTools/Source/C/VfrCompile/VfrError.cpp |3 +-
 BaseTools/Source/C/VfrCompile/VfrError.h   |3 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp   |  105 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h |   52 +-
 BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 1238 +---
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp|  241 +++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h  |   23 +-
 MdeModulePkg/Include/Guid/MdeModuleHii.h   |   20 +-
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c   |  246 ++--
 MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h   |4 +-
 MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf |5 +-
 MdeModulePkg/MdeModulePkg.dec  |4 +
 .../Universal/DriverSampleDxe/DriverSample.c   |  120 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |2 +
 .../Universal/DriverSampleDxe/NVDataStruc.h|   34 +-
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr |  172 +++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |   63 +
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   |  346 +-
 .../Universal/HiiDatabaseDxe/HiiDatabase.h |6 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf|3 +-
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  |  138 ++-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c |  110 +-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h |3 +
 .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf  |3 +-
 25 files changed, 2312 insertions(+), 647 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [PATCH v2] MdeModulePkg/SerialDxe: Fix not able to change serial attributes

2017-09-18 Thread Pankaj Bansal

Issue : When try to change serial attributes using sermode
command, the default values are set
Cause : The SerialReset command resets the attributes' values
to default
Fix : Serial Reset command should set the attributes which have
been changed by user after calling SerialSetAttributes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pankaj Bansal 
---
Changes in v2:
   - Modified Patch description

 MdeModulePkg/Universal/SerialDxe/SerialIo.c | 66 -
 1 file changed, 18 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/SerialDxe/SerialIo.c 
b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
index 43d33db..dc7e13a 100644
--- a/MdeModulePkg/Universal/SerialDxe/SerialIo.c
+++ b/MdeModulePkg/Universal/SerialDxe/SerialIo.c
@@ -220,7 +220,6 @@ SerialReset (
   )
 {
   EFI_STATUSStatus;
-  EFI_TPL   Tpl;
 
   Status = SerialPortInitialize ();
   if (EFI_ERROR (Status)) {
@@ -228,49 +227,17 @@ SerialReset (
   }
 
   //
-  // Set the Serial I/O mode and update the device path
-  //
-
-  Tpl = gBS->RaiseTPL (TPL_NOTIFY);
-
-  //
-  // Set the Serial I/O mode
-  //
-  This->Mode->ReceiveFifoDepth  = PcdGet16 (PcdUartDefaultReceiveFifoDepth);
-  This->Mode->Timeout   = 1000 * 1000;
-  This->Mode->BaudRate  = PcdGet64 (PcdUartDefaultBaudRate);
-  This->Mode->DataBits  = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
-  This->Mode->Parity= (UINT32) PcdGet8 (PcdUartDefaultParity);
-  This->Mode->StopBits  = (UINT32) PcdGet8 (PcdUartDefaultStopBits);
-
-  //
-  // Check if the device path has actually changed
-  //
-  if (mSerialDevicePath.Uart.BaudRate == This->Mode->BaudRate &&
-  mSerialDevicePath.Uart.DataBits == (UINT8) This->Mode->DataBits &&
-  mSerialDevicePath.Uart.Parity   == (UINT8) This->Mode->Parity &&
-  mSerialDevicePath.Uart.StopBits == (UINT8) This->Mode->StopBits
- ) {
-gBS->RestoreTPL (Tpl);
-return EFI_SUCCESS;
-  }
-
-  //
-  // Update the device path
+  // Go set the current attributes
   //
-  mSerialDevicePath.Uart.BaudRate = This->Mode->BaudRate;
-  mSerialDevicePath.Uart.DataBits = (UINT8) This->Mode->DataBits;
-  mSerialDevicePath.Uart.Parity   = (UINT8) This->Mode->Parity;
-  mSerialDevicePath.Uart.StopBits = (UINT8) This->Mode->StopBits;
-
-  Status = gBS->ReinstallProtocolInterface (
-  mSerialHandle,
-  ,
-  ,
-  
-  );
-
-  gBS->RestoreTPL (Tpl);
+  Status = This->SetAttributes (
+   This,
+   This->Mode->BaudRate,
+   This->Mode->ReceiveFifoDepth,
+   This->Mode->Timeout,
+   (EFI_PARITY_TYPE) This->Mode->Parity,
+   (UINT8) This->Mode->DataBits,
+   (EFI_STOP_BITS_TYPE) This->Mode->StopBits
+   );
 
   return Status;
 }
@@ -513,11 +480,6 @@ SerialDxeInitialize (
 {
   EFI_STATUSStatus;
 
-  Status = SerialPortInitialize ();
-  if (EFI_ERROR (Status)) {
-return Status;
-  }
-
   mSerialIoMode.BaudRate = PcdGet64 (PcdUartDefaultBaudRate);
   mSerialIoMode.DataBits = (UINT32) PcdGet8 (PcdUartDefaultDataBits);
   mSerialIoMode.Parity   = (UINT32) PcdGet8 (PcdUartDefaultParity);
@@ -529,6 +491,14 @@ SerialDxeInitialize (
   mSerialDevicePath.Uart.StopBits = PcdGet8 (PcdUartDefaultStopBits);
 
   //
+  // Issue a reset to initialize the COM port
+  //
+  Status = mSerialIoTemplate.Reset ();
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  //
   // Make a new handle with Serial IO protocol and its device path on it.
   //
   Status = gBS->InstallMultipleProtocolInterfaces (
-- 
2.7.4

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

[edk2] (no subject)

2017-09-18 Thread Pankaj Bansal

Hi Laszlo, Zeng

Thanks for patch review.

@Zeng: The sermode command calls SerialSetAttributes, which sets H/W attributes 
of Serial device.
After that the SerialIo protocol is reinstalled, which causes 
MdeModulePkg/Universal/Console/TerminalDxe
and MdeModulePkg/Universal/Console/ConPlatformDxe drivers' bindings to stop and 
then start.
This in turn calls SerialReset, which undoes changes of SerialSetAttributes.

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

[edk2] [PATCH] ShellPkg/dmpstore: Show name of known variable vendor GUID

2017-09-18 Thread Ruiyu Ni

From: Huajing Li 

Change "dmpstore" to show name of known variable vendor GUID.
The name is got from ShellProtocol.GetGuidName().

Cc: Jaben Carsey 
Reviewed-by: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Huajing Li 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c  | 17 +
 .../UefiShellDebug1CommandsLib.uni  |  1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index aeffc89b19..062ab5dc3a 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -424,6 +424,7 @@ CascadeProcessVariables (
   CHAR16*AttrString;
   CHAR16*HexString;
   EFI_STATUSSetStatus;
+  CHAR16*GuidName;
 
   if (ShellGetExecutionBreakFlag()) {
 return (SHELL_ABORTED);
@@ -521,10 +522,18 @@ CascadeProcessVariables (
 Status = EFI_OUT_OF_RESOURCES;
   }
 } else {
-  ShellPrintHiiEx (
--1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), 
gShellDebug1HiiHandle,
-AttrString, , FoundVarName, DataSize
-);
+  Status = gEfiShellProtocol->GetGuidName(, );
+  if (EFI_ERROR (Status)) {
+ShellPrintHiiEx (
+  -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), 
gShellDebug1HiiHandle,
+  AttrString, , FoundVarName, DataSize
+  );
+  } else {
+ShellPrintHiiEx (
+  -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE2), 
gShellDebug1HiiHandle,
+  AttrString, GuidName, FoundVarName, DataSize
+  );
+  }
   DumpHex (2, 0, DataSize, DataBuffer);
 }
 SHELL_FREE_NON_NULL (AttrString);
diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
index f733a67f0b..b6a133a454 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
@@ -385,6 +385,7 @@
 #string STR_DMPSTORE_LOAD_GEN_FAIL #language en-US "%H%s%N: Failed to set 
variable %H%s%N: %r.\r\n"
 #string STR_DMPSTORE_LOAD_BAD_FILE #language en-US "%H%s%N: Incorrect file 
format.\r\n"
 #string STR_DMPSTORE_HEADER_LINE   #language en-US "Variable %H%s%N 
'%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
+#string STR_DMPSTORE_HEADER_LINE2  #language en-US "Variable %H%s%N 
'%H%s%N:%H%s%N' DataSize = 0x%02x\r\n"
 #string STR_DMPSTORE_DELETE_LINE   #language en-US "Delete variable 
'%H%g%N:%H%s%N': %r\r\n"
 #string STR_DMPSTORE_NO_VAR_FOUND  #language en-US "%H%s%N: No matching 
variables found.\r\n"
 #string STR_DMPSTORE_NO_VAR_FOUND_SFO  #language en-US 
"VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
-- 
2.12.2.windows.2

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

45 matches

Mail list logo