Re: [edk2-devel] [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.

2020-05-27 Thread Gaurav Jain
Hi Leif

Tein Hock has reviewed this patch.
Please help to merge in edk2.

Regards
Gaurav Jain

> -Original Message-
> From: Loh, Tien Hock 
> Sent: Thursday, April 30, 2020 6:47 AM
> To: Leif Lindholm 
> Cc: Pankaj Bansal ; Ard Biesheuvel
> ; Gaurav Jain ;
> Meenakshi Aggarwal ;
> devel@edk2.groups.io; Haojian Zhuang ; Varun
> Sethi 
> Subject: [EXT] RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer Limit 65535 in R/W.
> 
> Caution: EXT Email
> 
> Hi Leif,
> 
> Yes, that's a Reviewed-by.
> 
> Thanks.
> 
> > -Original Message-
> > From: Leif Lindholm 
> > Sent: Wednesday, April 29, 2020 7:16 PM
> > To: Loh, Tien Hock 
> > Cc: Pankaj Bansal ; Ard Biesheuvel
> > ; Gaurav Jain ;
> Meenakshi
> > Aggarwal ; devel@edk2.groups.io;
> Haojian
> > Zhuang ; Varun Sethi 
> > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer
> > Limit 65535 in R/W.
> >
> > Hi Tien Hock,
> >
> > Can I take that as a Reviewed-by:?
> >
> > Regards,
> >
> > Leif
> >
> > On Wed, Apr 29, 2020 at 05:17:18 +, Loh, Tien Hock wrote:
> > > Hi Ard,
> > >
> > > I have checked the patch and it looks good.
> > >
> > > However, I can no longer test the patch as the new DwMmc driver no
> > > longer
> > uses the protocol.
> > > Sorry for the delay, I initially thought I can test it until I
> > > investigated further
> > today.
> > >
> > > Thanks
> > >
> > >
> > > > -Original Message-
> > > > From: Pankaj Bansal 
> > > > Sent: Monday, April 27, 2020 2:19 PM
> > > > To: Ard Biesheuvel ; Leif Lindholm
> > > > ; Gaurav Jain ; Meenakshi
> > > > Aggarwal 
> > > > Cc: devel@edk2.groups.io; Haojian Zhuang
> > > > ; Loh, Tien Hock
> > > > ; Varun Sethi 
> > > > Subject: RE: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > Transfer
> > > > Limit 65535 in R/W.
> > > >
> > > > + Meenakshi
> > > >
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel 
> > > > > Sent: Monday, April 6, 2020 7:42 PM
> > > > > To: Leif Lindholm ; Gaurav Jain
> > > > > 
> > > > > Cc: devel@edk2.groups.io; Pankaj Bansal ;
> > > > > Haojian Zhuang ; Loh, Tien Hock
> > > > > 
> > > > > Subject: Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > > > > Transfer Limit 65535 in R/W.
> > > > >
> > > > > On 4/6/20 4:08 PM, Leif Lindholm wrote:
> > > > > > Hi Gaurav,
> > > > > >
> > > > > > Haojian, Tien Hock - can you help review/test this change?
> > > > > >
> > > > > > Best Regards,
> > > > > >
> > > > > > Leif
> > > > > >
> > > > > > On Fri, Apr 03, 2020 at 14:54:07 +0530, Gaurav Jain wrote:
> > > > > >> Moved BlockCount calculation below BufferSize Validation checks.
> > > > > >> First Ensure Buffersize is Not Zero and multiple of Media 
> > > > > >> BlockSize.
> > > > > >> then calculate BlockCount and perform Block checks.
> > > > > >>
> > > > > >> Corrected BlockCount calculation, as BufferSize is multiple
> > > > > >> of BlockSize, So adding (BlockSize-1) bytes to BufferSize and
> > > > > >> then divide by BlockSize will have no impact on BlockCount.
> > > > > >>
> > > > > >> Reading Large Images from MMC causes errors.
> > > > > >> As per SD Host Controller Spec version 4.20, Restriction of
> > > > > >> 16-bit Block Count transfer is 65535.
> > > > > >> Max block transfer limit in single cmd is 65535 blocks.
> > > > > >> Added Max Block check that can be processed is 0x.
> > > > > >> then Update BlockCount on the basis of MaxBlock.
> > > > > >>
> > > > > >> Signed-off-by: Gaurav Jain 
> > > > >
> > > > >
> > > > > Hello Gaurav,
> > > > >
> > > > > Could you please elaborate on the underlying need for this change?
> > > > > If you are considering using this driver for future NXP
> > > > > platforms, I should point out that this legacy driver is only
> > > > > kept around for existing users, and new users should use the
> > > > > driver stack in MdeModulePkg, which is based on the UEFI spec.
> > > > >
> > > > > --
> > > > > Ard.
> > > > >
> > > > >
> > > > >
> > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60335): https://edk2.groups.io/g/devel/message/60335
Mute This Topic: https://groups.io/mt/74497516/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 1/1] SignedCapsulePkg: Add handling of NULL returned from FMP Descriptor

2020-05-06 Thread Gaurav Jain
Firmware management protocol that does not support GetImageInfo
return Unsupported.
hence FMP Image Information Buffer is NULL.
Freeing NULL buffer results in Exception.

Added NULL check for Image Info buffer
and skip processing FMP protocol handle,
which does not support GetImageInfo.

Signed-off-by: Gaurav Jain 
---
 .../Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git 
a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c 
b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
index bdb70bdb32cc..b4438ac6f55a 100644
--- a/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
+++ b/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.c
@@ -579,7 +579,7 @@ GetFmpImageDescriptors (
 // PackageVersionName
   );
   if (Status != EFI_BUFFER_TOO_SMALL) {
-DEBUG ((DEBUG_ERROR, "SystemFirmwareUpdateDxe: Unexpected Failure.  Status 
= %r\n", Status));
+DEBUG ((DEBUG_INFO, "SystemFirmwareUpdateDxe: Status = %r\n", Status));
 return NULL;
   }
 
@@ -678,6 +678,9 @@ FindMatchingFmpHandles (
 
 );
 
+if (OriginalFmpImageInfoBuf == NULL) {
+  continue;
+}
 //
 // Loop through the set of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
 //
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58686): https://edk2.groups.io/g/devel/message/58686
Mute This Topic: https://groups.io/mt/74024929/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [edk2-test PATCH v2] SctPkg: Updated Start Address Alignment code

2020-05-02 Thread Gaurav Jain
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2671

AllocatePages Functionality test.
Existing Code Increase Start address by 64k,
even if Start is already aligned to 64k.

Suggested Change will not modify Start,
if Start is already aligned to 64k

For Eg.
Available Memory(Start is aligned to 64k):
Start  End  PageNum(Free Pages)
8000  EBD6EFFF  6BD6F(Number of Free pages are not 64k Aligned)

edk2-test code:
Start is increased & aligned by 64k(Equal to 16 pages of size 4k each).
Request for Pagenum is minus by 16 Pages.
Start = (Start + 0x1) & 0x
Start = (0x8000 + 0x1) & 0x = 0x8001
PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1) = 0x6BD6F - 0x10 = 0x6BD5F

edk2 memory allocation code to Align the requested pages to 64k boundary:

NumberOfPages += EFI_SIZE_TO_PAGES(Alignment) - 1 = 0x6BD5F + 0xF = 0x6BD6E
NumberOfPages &= ~(EFI_SIZE_TO_PAGES(Alignment) - 1) = 0x6BD6E & 0xFFF0
 = 0x6BD60

We can observe that requested pages is incraesed by 1 page,
which results in below error.
ConvertPages: range 8001 - EBD6 covers multiple entries.

Signed-off-by: Gaurav Jain 
---

Notes:
v2
Updated Commit Message with an Example

 .../BlackBoxTest/MemoryAllocationServicesBBTestFunction.c  | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
index d18fe1f..a42cd9a 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MemoryAllocationServices/BlackBoxTest/MemoryAllocationServicesBBTestFunction.c
@@ -706,7 +706,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start;
@@ -836,7 +836,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start;
@@ -959,7 +959,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory = Start + (SctLShiftU64 (PageNum/3, EFI_PAGE_SHIFT) & 
0x);
@@ -1082,7 +1082,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start + (SctLShiftU64 (PageNum * 2 / 3, EFI_PAGE_SHIFT) & 
0x);
@@ -1212,7 +1212,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start;
@@ -1335,7 +1335,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start;
@@ -1474,7 +1474,7 @@ BBTestAllocatePagesInterfaceTest (
 if (PageNum <= 0x10) {
   break;
 }
-Start   = (Start + 0x1) & 0x;
+Start   = (Start + 0x) & 0x;
 PageNum = PageNum - EFI_SIZE_TO_PAGES(0x1);
 
 Memory  = Start;
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58531): https://edk2.groups.io/g/devel/message/58531
Mute This Topic: https://groups.io/mt/73935158/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.

2020-04-21 Thread Gaurav Jain
Hi Tien Hock

Can you help to review the patch?

Regards
Gaurav Jain

> -Original Message-
> From: Loh, Tien Hock 
> Sent: Tuesday, April 7, 2020 1:23 PM
> To: Gaurav Jain ; Ard Biesheuvel
> ; Leif Lindholm 
> Cc: devel@edk2.groups.io; Pankaj Bansal ;
> Haojian Zhuang 
> Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer Limit 65535 in R/W.
> 
> Caution: EXT Email
> 
> Hi Leif, Gaurav,
> 
> The changes look good to me, but I haven't tested it on Intel's SoCFPGA
> platform.
> I will need some time to test it as I'm working on some other tasks, maybe in
> a week or so.
> 
> Thanks
> 
> > -Original Message-
> > From: Gaurav Jain 
> > Sent: Tuesday, April 7, 2020 3:02 PM
> > To: Ard Biesheuvel ; Leif Lindholm
> > 
> > Cc: devel@edk2.groups.io; Pankaj Bansal ;
> > Haojian Zhuang ; Loh, Tien Hock
> > 
> > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added
> MaxBlock
> > Transfer Limit 65535 in R/W.
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel 
> > > Sent: Monday, April 6, 2020 7:42 PM
> > > To: Leif Lindholm ; Gaurav Jain
> > > 
> > > Cc: devel@edk2.groups.io; Pankaj Bansal ;
> > > Haojian Zhuang ; Loh, Tien Hock
> > > 
> > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> > > Transfer Limit 65535 in R/W.
> > >
> > > Caution: EXT Email
> > >
> > > On 4/6/20 4:08 PM, Leif Lindholm wrote:
> > > > Hi Gaurav,
> > > >
> > > > Haojian, Tien Hock - can you help review/test this change?
> > > >
> > > > Best Regards,
> > > >
> > > > Leif
> > > >
> > > > On Fri, Apr 03, 2020 at 14:54:07 +0530, Gaurav Jain wrote:
> > > >> Moved BlockCount calculation below BufferSize Validation checks.
> > > >> First Ensure Buffersize is Not Zero and multiple of Media BlockSize.
> > > >> then calculate BlockCount and perform Block checks.
> > > >>
> > > >> Corrected BlockCount calculation, as BufferSize is multiple of
> > > >> BlockSize, So adding (BlockSize-1) bytes to BufferSize and then
> > > >> divide by BlockSize will have no impact on BlockCount.
> > > >>
> > > >> Reading Large Images from MMC causes errors.
> > > >> As per SD Host Controller Spec version 4.20, Restriction of
> > > >> 16-bit Block Count transfer is 65535.
> > > >> Max block transfer limit in single cmd is 65535 blocks.
> > > >> Added Max Block check that can be processed is 0x.
> > > >> then Update BlockCount on the basis of MaxBlock.
> > > >>
> > > >> Signed-off-by: Gaurav Jain 
> > >
> > >
> > > Hello Gaurav,
> > >
> > > Could you please elaborate on the underlying need for this change?
> > > If you are considering using this driver for future NXP platforms, I
> > > should point out that this legacy driver is only kept around for
> > > existing users, and new users should use the driver stack in
> > > MdeModulePkg, which is based on the UEFI spec.
> > >
> > > --
> > > Ard.
> >
> > Hello Ard
> >
> > This change is for existing Platforms as well, that are using
> > EmbeddedPkg driver.
> > I can see Max Block Transfer Limit in MdeModulePkg also.
> > This Limit is not defined in EmbeddedPkg, which is causing errors on
> > NXP existing platform While reading Large images from MMC.
> > Block transfer limit is defined in SD spec.
> >
> > Regards
> > Gaurav Jain
> > >
> > >
> > >
> > > >> ---
> > > >>   EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 38
> > > ---
> > > >>   1 file changed, 25 insertions(+), 13 deletions(-)
> > > >>
> > > >> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > >> b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > >> index 17c20c0159ba..b508c466d9c5 100644
> > > >> --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > >> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> > > >> @@ -242,6 +242,8 @@ MmcIoBlocks (
> > > >> UINTN   BytesRemainingToBeTransfered;
> > > >> UINTN   BlockCount;
> > > >> UINTN   ConsumeSize;
> > > >>

Re: [edk2-devel] [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.

2020-04-07 Thread Gaurav Jain


> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, April 6, 2020 7:42 PM
> To: Leif Lindholm ; Gaurav Jain 
> Cc: devel@edk2.groups.io; Pankaj Bansal ;
> Haojian Zhuang ; Loh, Tien Hock
> 
> Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock
> Transfer Limit 65535 in R/W.
> 
> Caution: EXT Email
> 
> On 4/6/20 4:08 PM, Leif Lindholm wrote:
> > Hi Gaurav,
> >
> > Haojian, Tien Hock - can you help review/test this change?
> >
> > Best Regards,
> >
> > Leif
> >
> > On Fri, Apr 03, 2020 at 14:54:07 +0530, Gaurav Jain wrote:
> >> Moved BlockCount calculation below BufferSize Validation checks.
> >> First Ensure Buffersize is Not Zero and multiple of Media BlockSize.
> >> then calculate BlockCount and perform Block checks.
> >>
> >> Corrected BlockCount calculation, as BufferSize is multiple of
> >> BlockSize, So adding (BlockSize-1) bytes to BufferSize and then
> >> divide by BlockSize will have no impact on BlockCount.
> >>
> >> Reading Large Images from MMC causes errors.
> >> As per SD Host Controller Spec version 4.20, Restriction of 16-bit
> >> Block Count transfer is 65535.
> >> Max block transfer limit in single cmd is 65535 blocks.
> >> Added Max Block check that can be processed is 0x.
> >> then Update BlockCount on the basis of MaxBlock.
> >>
> >> Signed-off-by: Gaurav Jain 
> 
> 
> Hello Gaurav,
> 
> Could you please elaborate on the underlying need for this change? If you
> are considering using this driver for future NXP platforms, I should point out
> that this legacy driver is only kept around for existing users, and new users
> should use the driver stack in MdeModulePkg, which is based on the UEFI
> spec.
> 
> --
> Ard.

Hello Ard

This change is for existing Platforms as well, that are using EmbeddedPkg 
driver.
I can see Max Block Transfer Limit in MdeModulePkg also.
This Limit is not defined in EmbeddedPkg, which is causing errors on NXP 
existing platform While reading Large images from MMC.
Block transfer limit is defined in SD spec.

Regards
Gaurav Jain
> 
> 
> 
> >> ---
> >>   EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 38
> ---
> >>   1 file changed, 25 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> >> b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> >> index 17c20c0159ba..b508c466d9c5 100644
> >> --- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> >> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
> >> @@ -242,6 +242,8 @@ MmcIoBlocks (
> >> UINTN   BytesRemainingToBeTransfered;
> >> UINTN   BlockCount;
> >> UINTN   ConsumeSize;
> >> +  UINT32  MaxBlock;
> >> +  UINTN   RemainingBlock;
> >>
> >> BlockCount = 1;
> >> MmcHostInstance = MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS
> (This); @@
> >> -262,19 +264,6 @@ MmcIoBlocks (
> >>   return EFI_NO_MEDIA;
> >> }
> >>
> >> -  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost-
> >IsMultiBlock(MmcHost)) {
> >> -BlockCount = (BufferSize + This->Media->BlockSize - 1) / This->Media-
> >BlockSize;
> >> -  }
> >> -
> >> -  // All blocks must be within the device
> >> -  if ((Lba + (BufferSize / This->Media->BlockSize)) > (This->Media-
> >LastBlock + 1)) {
> >> -return EFI_INVALID_PARAMETER;
> >> -  }
> >> -
> >> -  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly
> == TRUE)) {
> >> -return EFI_WRITE_PROTECTED;
> >> -  }
> >> -
> >> // Reading 0 Byte is valid
> >> if (BufferSize == 0) {
> >>   return EFI_SUCCESS;
> >> @@ -285,14 +274,36 @@ MmcIoBlocks (
> >>   return EFI_BAD_BUFFER_SIZE;
> >> }
> >>
> >> +  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost-
> >IsMultiBlock(MmcHost)) {
> >> +BlockCount = BufferSize / This->Media->BlockSize;  }
> >> +
> >> +  // All blocks must be within the device  if ((Lba + (BufferSize /
> >> + This->Media->BlockSize)) > (This->Media->LastBlock + 1)) {
> >> +return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly
> == TRUE)) {
> >>

[edk2-devel] [PATCH 1/1] EmbeddedPkg/MmcDxe: Added MaxBlock Transfer Limit 65535 in R/W.

2020-04-03 Thread Gaurav Jain
Moved BlockCount calculation below BufferSize Validation checks.
First Ensure Buffersize is Not Zero and multiple of Media BlockSize.
then calculate BlockCount and perform Block checks.

Corrected BlockCount calculation, as BufferSize is multiple of BlockSize,
So adding (BlockSize-1) bytes to BufferSize and
then divide by BlockSize will have no impact on BlockCount.

Reading Large Images from MMC causes errors.
As per SD Host Controller Spec version 4.20,
Restriction of 16-bit Block Count transfer is 65535.
Max block transfer limit in single cmd is 65535 blocks.
Added Max Block check that can be processed is 0x.
then Update BlockCount on the basis of MaxBlock.

Signed-off-by: Gaurav Jain 
---
 EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c | 38 ---
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
index 17c20c0159ba..b508c466d9c5 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c
@@ -242,6 +242,8 @@ MmcIoBlocks (
   UINTN   BytesRemainingToBeTransfered;
   UINTN   BlockCount;
   UINTN   ConsumeSize;
+  UINT32  MaxBlock;
+  UINTN   RemainingBlock;
 
   BlockCount = 1;
   MmcHostInstance = MMC_HOST_INSTANCE_FROM_BLOCK_IO_THIS (This);
@@ -262,19 +264,6 @@ MmcIoBlocks (
 return EFI_NO_MEDIA;
   }
 
-  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost->IsMultiBlock(MmcHost)) {
-BlockCount = (BufferSize + This->Media->BlockSize - 1) / 
This->Media->BlockSize;
-  }
-
-  // All blocks must be within the device
-  if ((Lba + (BufferSize / This->Media->BlockSize)) > (This->Media->LastBlock 
+ 1)) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly == TRUE)) {
-return EFI_WRITE_PROTECTED;
-  }
-
   // Reading 0 Byte is valid
   if (BufferSize == 0) {
 return EFI_SUCCESS;
@@ -285,14 +274,36 @@ MmcIoBlocks (
 return EFI_BAD_BUFFER_SIZE;
   }
 
+  if (MMC_HOST_HAS_ISMULTIBLOCK(MmcHost) && MmcHost->IsMultiBlock(MmcHost)) {
+BlockCount = BufferSize / This->Media->BlockSize;
+  }
+
+  // All blocks must be within the device
+  if ((Lba + (BufferSize / This->Media->BlockSize)) > (This->Media->LastBlock 
+ 1)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if ((Transfer == MMC_IOBLOCKS_WRITE) && (This->Media->ReadOnly == TRUE)) {
+return EFI_WRITE_PROTECTED;
+  }
+
   // Check the alignment
   if ((This->Media->IoAlign > 2) && (((UINTN)Buffer & (This->Media->IoAlign - 
1)) != 0)) {
 return EFI_INVALID_PARAMETER;
   }
 
+  // Max block number in single cmd is 65535 blocks.
+  MaxBlock = 0x;
+  RemainingBlock = BlockCount;
   BytesRemainingToBeTransfered = BufferSize;
   while (BytesRemainingToBeTransfered > 0) {
 
+if (RemainingBlock <= MaxBlock) {
+  BlockCount = RemainingBlock;
+} else {
+  BlockCount = MaxBlock;
+}
+
 // Check if the Card is in Ready status
 CmdArg = MmcHostInstance->CardInfo.RCA << 16;
 Response[0] = 0;
@@ -338,6 +349,7 @@ MmcIoBlocks (
   DEBUG ((EFI_D_ERROR, "%a(): Failed to transfer block and Status:%r\n", 
__func__, Status));
 }
 
+RemainingBlock -= BlockCount;
 BytesRemainingToBeTransfered -= ConsumeSize;
 if (BytesRemainingToBeTransfered > 0) {
   Lba+= BlockCount;
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56937): https://edk2.groups.io/g/devel/message/56937
Mute This Topic: https://groups.io/mt/72744881/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v3 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-03-17 Thread Gaurav Jain
ASSERT in SetTime_Conf Consistency Test.
SCT Test expect return as Invalid Parameter.
So removed ASSERT().

Signed-off-by: Gaurav Jain 
---

Notes:
v3
Removed Time Validity Checks in function SetWakeupTime.

v2
reverted changes related to valid range of years.

 EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c 
b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
index 08fb9b0100b6..20f1fa640ecc 100644
--- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
+++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
@@ -85,10 +85,6 @@ IsDayValid (
   IN  EFI_TIME  *Time
   )
 {
-  ASSERT (Time->Day >= 1);
-  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
-  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
-
   if (Time->Day < 1 ||
   Time->Day > mDayOfMonth[Time->Month - 1] ||
   (Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
@@ -113,6 +109,7 @@ IsTimeValid(
   Time->Hour   > 23 ||
   Time->Minute > 59 ||
   Time->Second > 59 ||
+  Time->Nanosecond > 9  ||
   !IsValidTimeZone (Time->TimeZone) ||
   !IsValidDaylight (Time->Daylight)) {
 return FALSE;
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55928): https://edk2.groups.io/g/devel/message/55928
Mute This Topic: https://groups.io/mt/72021031/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-26 Thread Gaurav Jain
Gentle Reminder!!
Please review..

Regards
Gaurav Jain

> -Original Message-
> From: Gaurav Jain 
> Sent: Tuesday, February 18, 2020 5:54 PM
> To: devel@edk2.groups.io
> Cc: Leif Lindholm ; Ard Biesheuvel
> ; Pankaj Bansal ; Gaurav
> Jain 
> Subject: [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services
> test.
> 
> ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> SCT Test expect return as Invalid Parameter.
> So removed ASSERT().
> 
> Added Time Validity Checks in SetWakeupTime.
> 
> Signed-off-by: Gaurav Jain 
> ---
> Changes in v2:
>   - reverted changes related to valid range of years.
> ---
>  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> index 08fb9b0100b6..70a0d78125b9 100644
> --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> @@ -85,10 +85,6 @@ IsDayValid (
>IN  EFI_TIME  *Time
>)
>  {
> -  ASSERT (Time->Day >= 1);
> -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
> -
>if (Time->Day < 1 ||
>Time->Day > mDayOfMonth[Time->Month - 1] ||
>(Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) { @@ -
> 113,6 +109,7 @@ IsTimeValid(
>Time->Hour   > 23 ||
>Time->Minute > 59 ||
>Time->Second > 59 ||
> +  Time->Nanosecond > 9  ||
>!IsValidTimeZone (Time->TimeZone) ||
>!IsValidDaylight (Time->Daylight)) {
>  return FALSE;
> @@ -254,6 +251,9 @@ SetWakeupTime (
>OUT EFI_TIME*Time
>)
>  {
> +  if (Time == NULL || !IsTimeValid (Time)) {
> +return EFI_INVALID_PARAMETER;
> +  }
>return LibSetWakeupTime (Enabled, Time);  }
> 
> --
> 2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54878): https://edk2.groups.io/g/devel/message/54878
Mute This Topic: https://groups.io/mt/71367009/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v3 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-24 Thread Gaurav Jain
ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
Conformance Test.
SCT Test expect return as Invalid Parameter or Unsupported.
Added Checks for Function Parameters.
return Invalid or Unsupported if Check fails.

Added Checks in PciIoPollIo(), PciIoIoRead()
PciIoIoWrite()

Signed-off-by: Gaurav Jain 
---

Notes:
v3
- Corrected Width check in PciIoIoRead, PciIoIoWrite.
- Removed BarIndex check as it is already in GetBarResource().
- Moved DEV_SUPPORTED_ATTRIBUTES Macro from NonDiscoverablePciDeviceIo.c
  to NonDiscoverablePciDeviceIo.h

v2
- Reverted ASSERT(FALSE) code.
- Added Checks for Width, BarIndex, Buffer,
  Address range in PciIoIoRead, PciIoIoWrite.
- Added Checks for Width, BarIndex, Result,
  Address range in PciIoPollIo, PciIoPollMem,
  PciIoCopyMem.
- Added Checks for Attributes, BarIndex,
  Address range in PciIoSetBarAttributes.

 .../NonDiscoverablePciDeviceIo.c  | 155 +-
 .../NonDiscoverablePciDeviceIo.h  |   3 +
 2 files changed, 155 insertions(+), 3 deletions(-)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 2d55c9699322..c3e83003a01c 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -93,6 +93,31 @@ PciIoPollMem (
   OUT UINT64  *Result
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  UINTN   Count;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Result == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+  Count = 1;
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -126,6 +151,31 @@ PciIoPollIo (
   OUT UINT64  *Result
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  UINTN   Count;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Result == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+  Count = 1;
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -396,6 +446,29 @@ PciIoIoRead (
   IN OUT VOID *Buffer
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width >= EfiPciIoWidthMaximum) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Buffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -425,6 +498,29 @@ PciIoIoWrite (
   IN OUT VOID *Buffer
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width >= EfiPciIoWidthMaximum) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Buffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -556,6 +652,35 @@ PciIoCopyMem (
   IN UINTNCount
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *DestDesc;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *SrcDesc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, DestBarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (DestOffse

Re: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-24 Thread Gaurav Jain



> -Original Message-
> From: Wu, Hao A 
> Sent: Monday, February 24, 2020 1:56 PM
> To: Gaurav Jain ; devel@edk2.groups.io; Gao, Liming
> ; af...@apple.com; ler...@redhat.com;
> l...@nuviainc.com; Kinney, Michael D 
> Cc: Wang, Jian J ; Ni, Ray ; Ard
> Biesheuvel ; Pankaj Bansal
> 
> Subject: RE: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1]
> MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
> 
> Caution: EXT Email
> 
> > -Original Message-
> > From: Gaurav Jain [mailto:gaurav.j...@nxp.com]
> > Sent: Monday, February 24, 2020 3:04 PM
> > To: Wu, Hao A; devel@edk2.groups.io; Gao, Liming; af...@apple.com;
> > ler...@redhat.com; l...@nuviainc.com; Kinney, Michael D
> > Cc: Wang, Jian J; Ni, Ray; Ard Biesheuvel; Pankaj Bansal
> > Subject: RE: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1]
> > MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
> >
> >
> > > I think the above check for 'Attributes' can be dropped.
> > > I found that the implementation of the PciIoGetBarAttributes()
> > > function
> > does not
> > > expose any configurable attributes. So the logic can fall through to
> > > the
> > ASSERT
> > > (for DEBUG images) and then returns EFI_UNSUPPORTED.
> >
> > I agree that PciIoGetBarAttributes() function sets *Supports as 0.
> > But In SCT Test for SetBarAttributes, there is a test case for
> > Unsupported Attribute which expects EFI_UNSUPPORTED. If I drop this
> > check, ASSERT will come, which is not expected.
> > Can we keep check for 'Attributes'?
> 
> 
> Oh, I forgot that.
> 
> I have one question, is there any special reason for you to pick the supported
> bits specified by:
> EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
> 
> Is it relating with the SCT test case?

In PciIoAttributes() function, I can see the code
#define DEV_SUPPORTED_ATTRIBUTES \
(EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
So I used the same bits in PciIoSetBarAttributes() to have a check for valid 
attributes.

In SCT Test code
First get the Bar attributes and set one of Unsupported attribute bit.
Call PciIoSetBarAttributes() with Unsupported attribute and in return, test 
expects EFI_UNSUPPORTED.

Regards
Gaurav Jain
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > > -Original Message-
> > > From: Wu, Hao A 
> > > Sent: Friday, February 21, 2020 6:53 AM
> > > To: devel@edk2.groups.io; Gaurav Jain ; Gao,
> > Liming
> > > ; af...@apple.com; ler...@redhat.com;
> > > l...@nuviainc.com; Kinney, Michael D 
> > > Cc: Wang, Jian J ; Ni, Ray
> > > ; Ard Biesheuvel ;
> > > Pankaj Bansal 
> > > Subject: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1]
> > > MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
> > >
> > > Caution: EXT Email
> > >
> > > A couple of inline comments below. Please help to handle them in the
> > > next version of patch.
> > > With them addressed,
> > > Reviewed-by: Hao A Wu 
> > >
> > >
> > > Hello Liming and Stewards,
> > >
> > > I would like to confirm with you for whether the patch should catch
> > > the upcoming stable tag.
> > >
> > > My personal take is that the patch is more like a code refinement
> > > rather
> > than a
> > > bug fix.
> > >
> > > Could you help to make a final call for this one? Thanks in advance.
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > Of
> > > > Gaurav Jain
> > > > Sent: Thursday, February 20, 2020 11:40 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Ard Biesheuvel; Pankaj
> > > > Bansal; Gaurav Jain
> > > > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed
> > > > Asserts in SCT PCIIO Protocol Test.
> > > >
> > > > ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
> > > > Conformance Test.
> > > > SCT Test expect return as Invalid Parameter or Unsupported.
> > > > Added Checks for Function Parameters.
> > > > return Invalid or Unsupported if Check fails.
> > > >
> > > > Added Checks in PciIoPollIo(), PciIoIoRead()
> > > > PciIoIoWrite()
> > > >
> > > > Signed-off-by: Gau

Re: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-23 Thread Gaurav Jain


> I think the above check for 'Attributes' can be dropped.
> I found that the implementation of the PciIoGetBarAttributes() function does 
> not
> expose any configurable attributes. So the logic can fall through to the 
> ASSERT
> (for DEBUG images) and then returns EFI_UNSUPPORTED.

I agree that PciIoGetBarAttributes() function sets *Supports as 0.
But In SCT Test for SetBarAttributes, there is a test case for Unsupported 
Attribute which expects EFI_UNSUPPORTED. If I drop this check, ASSERT will 
come, which is not expected.
Can we keep check for 'Attributes'?

> -Original Message-
> From: Wu, Hao A 
> Sent: Friday, February 21, 2020 6:53 AM
> To: devel@edk2.groups.io; Gaurav Jain ; Gao, Liming
> ; af...@apple.com; ler...@redhat.com;
> l...@nuviainc.com; Kinney, Michael D 
> Cc: Wang, Jian J ; Ni, Ray ; Ard
> Biesheuvel ; Pankaj Bansal
> 
> Subject: [EXT] RE: [edk2-stable202002][edk2-devel] [PATCH v2 1/1]
> MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.
> 
> Caution: EXT Email
> 
> A couple of inline comments below. Please help to handle them in the next
> version of patch.
> With them addressed,
> Reviewed-by: Hao A Wu 
> 
> 
> Hello Liming and Stewards,
> 
> I would like to confirm with you for whether the patch should catch the
> upcoming stable tag.
> 
> My personal take is that the patch is more like a code refinement rather than 
> a
> bug fix.
> 
> Could you help to make a final call for this one? Thanks in advance.
> 
> Best Regards,
> Hao Wu
> 
> 
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Gaurav Jain
> > Sent: Thursday, February 20, 2020 11:40 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Ard Biesheuvel; Pankaj Bansal;
> > Gaurav Jain
> > Subject: [edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts
> > in SCT PCIIO Protocol Test.
> >
> > ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
> > Conformance Test.
> > SCT Test expect return as Invalid Parameter or Unsupported.
> > Added Checks for Function Parameters.
> > return Invalid or Unsupported if Check fails.
> >
> > Added Checks in PciIoPollIo(), PciIoIoRead()
> > PciIoIoWrite()
> >
> > Signed-off-by: Gaurav Jain 
> > ---
> >
> > Notes:
> > v2
> > - Reverted ASSERT(FALSE) code.
> > - Added Checks for Width, BarIndex, Buffer,
> >   Address range in PciIoIoRead, PciIoIoWrite.
> > - Added Checks for Width, BarIndex, Result,
> >   Address range in PciIoPollIo, PciIoPollMem,
> >   PciIoCopyMem.
> > - Added Checks for Attributes, BarIndex,
> >   Address range in PciIoSetBarAttributes.
> >
> >  .../NonDiscoverablePciDeviceIo.c  | 180 ++
> >  1 file changed, 180 insertions(+)
> >
> > diff --git
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > index 2d55c9699322..4dd804356021 100644
> > ---
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > +++
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > @@ -93,6 +93,35 @@ PciIoPollMem (
> >OUT UINT64  *Result
> >)
> >  {
> > +  NON_DISCOVERABLE_PCI_DEVICE *Dev;
> > +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
> > +  UINTN   Count;
> > +  EFI_STATUS  Status;
> > +
> > +  if ((UINT32)Width > EfiPciIoWidthUint64) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (BarIndex >= PCI_MAX_BAR) {
> > +return EFI_UNSUPPORTED;
> > +  }
> > +
> > +  if (Result == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
> > +  Count = 1;
> > +
> > +  Status = GetBarResource (Dev, BarIndex, );  if (EFI_ERROR
> > + (Status)) {
> > +return Status;
> > +  }
> > +
> > +  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
> > +return EFI_UNSUPPORTED;
> > +  }
> > +
> >ASSERT (FALSE);
> >return EFI_UNSUPPORTED;
> >  }
> > @@ -126,6 +155,35 @@ PciIoPollIo (
> >OUT UINT64  *Result
> >)
> >  {
> > +  NON_DISCOVERABLE_PCI_DEVICE

Re: [edk2-devel] [EXT] RE: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-20 Thread Gaurav Jain



> -Original Message-
> From: Wu, Hao A 
> Sent: Wednesday, February 19, 2020 12:04 PM
> To: Gaurav Jain ; devel@edk2.groups.io; Ard Biesheuvel
> 
> Cc: Wang, Jian J ; Ni, Ray ; Pankaj
> Bansal 
> Subject: [EXT] RE: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO
> Protocol Test.
> 
> Caution: EXT Email
> 
> > -----Original Message-
> > From: Gaurav Jain [mailto:gaurav.j...@nxp.com]
> > Sent: Thursday, January 30, 2020 4:18 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Pankaj Bansal; Gaurav Jain
> > Subject: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO
> > Protocol Test.
> >
> > ASSERT in CopyMem_Conf, PollMem_Conf, SetBarAttributes_Conf
> > Conformance Test.
> > SCT Test expect return as Invalid Parameter.
> > So removed ASSERT().
> 
> 
> Include Ard (as the driver contributor) to see if he has additional comments.
> 
> A couple of general level comments:
> 1. I think the ASSERT can still be added after the checks you add. The ASSERT
>may serve as a notification in the DEBUG image that the service is not
>implemented.
> 
> 2. I found that for functions:
>PciIoPollIo()
>PciIoIoRead()
>PciIoIoWrite()
>They also do not have checks that perfectly match with the UEFI spec. Even
>though they are not exposed by the SCT tests, could you help to address 
> them
>as well?
> 
> Some other inline comments below.

Done the suggested changes and sent v2.
Please review.
> 
> 
> >
> > Signed-off-by: Gaurav Jain 
> > ---
> >  .../NonDiscoverablePciDeviceIo.c  | 20 ---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > index 2d55c9699322..76cb000602fc 100644
> > ---
> > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > +++
> > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverableP
> > ciDeviceIo.c
> > @@ -93,7 +93,15 @@ PciIoPollMem (
> >OUT UINT64  *Result
> >)
> >  {
> > -  ASSERT (FALSE);
> > +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> 
> 
> Looks to me that the 1st part of the 'if' check is redundant.
> Could you help to double confirm?
> 
> 
> > +  Width > EfiPciIoWidthUint64) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (Result == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> >return EFI_UNSUPPORTED;
> >  }
> >
> > @@ -556,7 +564,10 @@ PciIoCopyMem (
> >IN UINTNCount
> >)
> >  {
> > -  ASSERT (FALSE);
> > +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> 
> 
> Looks to me that the 1st part of the 'if' check is redundant.
> Could you help to double confirm?
> 
> Best Regards,
> Hao Wu
> 
> 
> > +  Width > EfiPciIoWidthUint64) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> >return EFI_UNSUPPORTED;
> >  }
> >
> > @@ -1414,7 +1425,10 @@ PciIoSetBarAttributes (
> >IN OUT UINT64   *Length
> >)
> >  {
> > -  ASSERT (FALSE);
> > +  if (Offset == NULL || Length == NULL) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> > +
> >return EFI_UNSUPPORTED;
> >  }
> >
> > --
> > 2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54677): https://edk2.groups.io/g/devel/message/54677
Mute This Topic: https://groups.io/mt/71426402/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-20 Thread Gaurav Jain
ASSERT in PollMem_Conf, CopyMem_Conf, SetBarAttributes_Conf
Conformance Test.
SCT Test expect return as Invalid Parameter or Unsupported.
Added Checks for Function Parameters.
return Invalid or Unsupported if Check fails.

Added Checks in PciIoPollIo(), PciIoIoRead()
PciIoIoWrite()

Signed-off-by: Gaurav Jain 
---

Notes:
v2
- Reverted ASSERT(FALSE) code.
- Added Checks for Width, BarIndex, Buffer,
  Address range in PciIoIoRead, PciIoIoWrite.
- Added Checks for Width, BarIndex, Result,
  Address range in PciIoPollIo, PciIoPollMem,
  PciIoCopyMem.
- Added Checks for Attributes, BarIndex,
  Address range in PciIoSetBarAttributes.

 .../NonDiscoverablePciDeviceIo.c  | 180 ++
 1 file changed, 180 insertions(+)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 2d55c9699322..4dd804356021 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -93,6 +93,35 @@ PciIoPollMem (
   OUT UINT64  *Result
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  UINTN   Count;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
+  if (Result == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+  Count = 1;
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -126,6 +155,35 @@ PciIoPollIo (
   OUT UINT64  *Result
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  UINTN   Count;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
+  if (Result == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+  Count = 1;
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -396,6 +454,33 @@ PciIoIoRead (
   IN OUT VOID *Buffer
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
+  if (Buffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -425,6 +510,33 @@ PciIoIoWrite (
   IN OUT VOID *Buffer
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
+  if (Buffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, BarIndex, );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  if (Offset + (Count << (Width & 0x3)) > Desc->AddrLen) {
+return EFI_UNSUPPORTED;
+  }
+
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
@@ -556,6 +668,40 @@ PciIoCopyMem (
   IN UINTNCount
   )
 {
+  NON_DISCOVERABLE_PCI_DEVICE *Dev;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *DestDesc;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *SrcDesc;
+  EFI_STATUS  Status;
+
+  if ((UINT32)Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (DestBarIndex >= PCI_MAX_BAR ||
+  SrcBarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
+  Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
+
+  Status = GetBarResource (Dev, DestBarIndex

Re: [EXT] RE: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-19 Thread Gaurav Jain
> Gaurav:
>   Does this patch catch to edk2 stable tag 202002?
Yes

Thanks
Gaurav

> -Original Message-
> From: Gao, Liming 
> Sent: Wednesday, February 19, 2020 2:36 PM
> To: devel@edk2.groups.io; Gaurav Jain 
> Cc: Leif Lindholm ; Ard Biesheuvel
> ; Pankaj Bansal 
> Subject: [EXT] RE: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in
> SCT Runtime Services test.
> 
> Caution: EXT Email
> 
> Gaurav:
>   Does this patch catch to edk2 stable tag 202002?
> 
> Thanks
> Liming
> > -Original Message-----
> > From: devel@edk2.groups.io  On Behalf Of Gaurav
> > Jain
> > Sent: Tuesday, February 18, 2020 8:24 PM
> > To: devel@edk2.groups.io
> > Cc: Leif Lindholm ; Ard Biesheuvel
> > ; Pankaj Bansal ;
> > Gaurav Jain 
> > Subject: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT
> Runtime Services test.
> >
> > ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> > SCT Test expect return as Invalid Parameter.
> > So removed ASSERT().
> >
> > Added Time Validity Checks in SetWakeupTime.
> >
> > Signed-off-by: Gaurav Jain 
> > ---
> > Changes in v2:
> >   - reverted changes related to valid range of years.
> > ---
> >  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > index 08fb9b0100b6..70a0d78125b9 100644
> > --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > @@ -85,10 +85,6 @@ IsDayValid (
> >IN  EFI_TIME  *Time
> >)
> >  {
> > -  ASSERT (Time->Day >= 1);
> > -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> > -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
> > -
> >if (Time->Day < 1 ||
> >Time->Day > mDayOfMonth[Time->Month - 1] ||
> >(Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
> > @@ -113,6 +109,7 @@ IsTimeValid(
> >Time->Hour   > 23 ||
> >Time->Minute > 59 ||
> >Time->Second > 59 ||
> > +  Time->Nanosecond > 9  ||
> >!IsValidTimeZone (Time->TimeZone) ||
> >!IsValidDaylight (Time->Daylight)) {
> >  return FALSE;
> > @@ -254,6 +251,9 @@ SetWakeupTime (
> >OUT EFI_TIME*Time
> >)
> >  {
> > +  if (Time == NULL || !IsTimeValid (Time)) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> >return LibSetWakeupTime (Enabled, Time);  }
> >
> > --
> > 2.17.1
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54664): https://edk2.groups.io/g/devel/message/54664
Mute This Topic: https://groups.io/mt/71422977/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [EXT] RE: [edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-18 Thread Gaurav Jain
Hi Gao

> Gaurav:
>   Does this patch catch to edk2 stable tag 202002?
Yes

> -Original Message-
> From: Gao, Liming 
> Sent: Monday, February 17, 2020 2:15 PM
> To: devel@edk2.groups.io; Gaurav Jain ; Gao, Liming
> 
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Pankaj Bansal 
> Subject: [EXT] RE: [edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in
> SCT PCIIO Protocol Test.
> 
> Caution: EXT Email
> 
> Gaurav:
>   Does this patch catch to edk2 stable tag 202002?
> 
> Thanks
> Liming
> > -Original Message-----
> > From: devel@edk2.groups.io  On Behalf Of Gaurav
> > Jain
> > Sent: Monday, February 17, 2020 3:18 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J ; Wu, Hao A
> > ; Ni, Ray ; Pankaj Bansal
> > 
> > Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT
> PCIIO Protocol Test.
> >
> > Gentle Reminder!!
> > Please review..
> >
> > > -Original Message-
> > > From: Gaurav Jain 
> > > Sent: Thursday, January 30, 2020 1:48 PM
> > > To: devel@edk2.groups.io
> > > Cc: Jian J Wang ; Hao A Wu
> > > ; Ray Ni ; Pankaj Bansal
> > > ; Gaurav Jain 
> > > Subject: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO
> > > Protocol Test.
> > >
> > > ASSERT in CopyMem_Conf, PollMem_Conf, SetBarAttributes_Conf
> > > Conformance Test.
> > > SCT Test expect return as Invalid Parameter.
> > > So removed ASSERT().
> > >
> > > Signed-off-by: Gaurav Jain 
> > > ---
> > >  .../NonDiscoverablePciDeviceIo.c  | 20 ---
> > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePc
> > > iD
> > > eviceIo.c
> > >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePc
> > > iD
> > > eviceIo.c
> > > index 2d55c9699322..76cb000602fc 100644
> > > ---
> > >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePc
> > > iD
> > > eviceIo.c
> > > +++
> > >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePc
> > > +++ iDeviceIo.c
> > > @@ -93,7 +93,15 @@ PciIoPollMem (
> > >OUT UINT64  *Result
> > >)
> > >  {
> > > -  ASSERT (FALSE);
> > > +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> > > +  Width > EfiPciIoWidthUint64) {
> > > +return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  if (Result == NULL) {
> > > +return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > >return EFI_UNSUPPORTED;
> > >  }
> > >
> > > @@ -556,7 +564,10 @@ PciIoCopyMem (
> > >IN UINTNCount
> > >)
> > >  {
> > > -  ASSERT (FALSE);
> > > +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> > > +  Width > EfiPciIoWidthUint64) {
> > > +return EFI_INVALID_PARAMETER;
> > > +  }
> > >return EFI_UNSUPPORTED;
> > >  }
> > >
> > > @@ -1414,7 +1425,10 @@ PciIoSetBarAttributes (
> > >IN OUT UINT64   *Length
> > >)
> > >  {
> > > -  ASSERT (FALSE);
> > > +  if (Offset == NULL || Length == NULL) {
> > > +return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > >return EFI_UNSUPPORTED;
> > >  }
> > >
> > > --
> > > 2.17.1
> >
> >
> > 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54604): https://edk2.groups.io/g/devel/message/54604
Mute This Topic: https://groups.io/mt/71389950/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH v2 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-17 Thread Gaurav Jain
ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
SCT Test expect return as Invalid Parameter.
So removed ASSERT().

Added Time Validity Checks in SetWakeupTime.

Signed-off-by: Gaurav Jain 
---
Changes in v2:
  - reverted changes related to valid range of years.
---
 EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c 
b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
index 08fb9b0100b6..70a0d78125b9 100644
--- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
+++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
@@ -85,10 +85,6 @@ IsDayValid (
   IN  EFI_TIME  *Time
   )
 {
-  ASSERT (Time->Day >= 1);
-  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
-  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
-
   if (Time->Day < 1 ||
   Time->Day > mDayOfMonth[Time->Month - 1] ||
   (Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
@@ -113,6 +109,7 @@ IsTimeValid(
   Time->Hour   > 23 ||
   Time->Minute > 59 ||
   Time->Second > 59 ||
+  Time->Nanosecond > 9  ||
   !IsValidTimeZone (Time->TimeZone) ||
   !IsValidDaylight (Time->Daylight)) {
 return FALSE;
@@ -254,6 +251,9 @@ SetWakeupTime (
   OUT EFI_TIME*Time
   )
 {
+  if (Time == NULL || !IsTimeValid (Time)) {
+return EFI_INVALID_PARAMETER;
+  }
   return LibSetWakeupTime (Enabled, Time);
 }
 
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54571): https://edk2.groups.io/g/devel/message/54571
Mute This Topic: https://groups.io/mt/71367009/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-16 Thread Gaurav Jain
ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
SCT Test expect return as Invalid Parameter.
So removed ASSERT().

Added Time Validity Checks in SetWakeupTime.

Signed-off-by: Gaurav Jain 
---
 EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c 
b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
index 08fb9b0100b6..70a0d78125b9 100644
--- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
+++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
@@ -85,10 +85,6 @@ IsDayValid (
   IN  EFI_TIME  *Time
   )
 {
-  ASSERT (Time->Day >= 1);
-  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
-  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
-
   if (Time->Day < 1 ||
   Time->Day > mDayOfMonth[Time->Month - 1] ||
   (Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
@@ -113,6 +109,7 @@ IsTimeValid(
   Time->Hour   > 23 ||
   Time->Minute > 59 ||
   Time->Second > 59 ||
+  Time->Nanosecond > 9  ||
   !IsValidTimeZone (Time->TimeZone) ||
   !IsValidDaylight (Time->Daylight)) {
 return FALSE;
@@ -254,6 +251,9 @@ SetWakeupTime (
   OUT EFI_TIME*Time
   )
 {
+  if (Time == NULL || !IsTimeValid (Time)) {
+return EFI_INVALID_PARAMETER;
+  }
   return LibSetWakeupTime (Enabled, Time);
 }
 
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54535): https://edk2.groups.io/g/devel/message/54535
Mute This Topic: https://groups.io/mt/71345440/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-02-16 Thread Gaurav Jain
Gentle Reminder!!
Please review..

> -Original Message-
> From: Gaurav Jain 
> Sent: Thursday, January 30, 2020 1:48 PM
> To: devel@edk2.groups.io
> Cc: Jian J Wang ; Hao A Wu ;
> Ray Ni ; Pankaj Bansal ; Gaurav
> Jain 
> Subject: [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol
> Test.
> 
> ASSERT in CopyMem_Conf, PollMem_Conf, SetBarAttributes_Conf
> Conformance Test.
> SCT Test expect return as Invalid Parameter.
> So removed ASSERT().
> 
> Signed-off-by: Gaurav Jain 
> ---
>  .../NonDiscoverablePciDeviceIo.c  | 20 ---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> index 2d55c9699322..76cb000602fc 100644
> ---
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> +++
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePc
> +++ iDeviceIo.c
> @@ -93,7 +93,15 @@ PciIoPollMem (
>OUT UINT64  *Result
>)
>  {
> -  ASSERT (FALSE);
> +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> +  Width > EfiPciIoWidthUint64) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Result == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>return EFI_UNSUPPORTED;
>  }
> 
> @@ -556,7 +564,10 @@ PciIoCopyMem (
>IN UINTNCount
>)
>  {
> -  ASSERT (FALSE);
> +  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
> +  Width > EfiPciIoWidthUint64) {
> +return EFI_INVALID_PARAMETER;
> +  }
>return EFI_UNSUPPORTED;
>  }
> 
> @@ -1414,7 +1425,10 @@ PciIoSetBarAttributes (
>IN OUT UINT64   *Length
>)
>  {
> -  ASSERT (FALSE);
> +  if (Offset == NULL || Length == NULL) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>return EFI_UNSUPPORTED;
>  }
> 
> --
> 2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54534): https://edk2.groups.io/g/devel/message/54534
Mute This Topic: https://groups.io/mt/70267136/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-16 Thread Gaurav Jain
Hi Ard

I have also checked UEFI Shell Specification Version 
2.2(https://uefi.org/sites/default/files/resources/UEFI_Shell_2_2.pdf)
For date command the range of valid years is from 1998–2099 (on page 121)

So Range of valid years is conflicting in UEFI Spec and UEFI Shell Spec.
I think UEFI spec needs to be corrected. What are your thoughts?


> -Original Message-
> From: Ard Biesheuvel 
> Sent: Tuesday, February 11, 2020 7:50 PM
> To: Gaurav Jain 
> Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj Bansal
> 
> Subject: Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT
> Runtime Services test.
> 
> Caution: EXT Email
> 
> On Tue, 11 Feb 2020 at 11:37, Gaurav Jain  wrote:
> >
> > Hi Ard
> >
> > I am waiting for your response.
> >
> 
> You said
> 
> > Either UEFI spec need to be modified as per the test or SCT Test needs fix 
> > as
> per UEFI Specification.
> >
> 
> so you answered your own question, no?
> 
> > -Original Message-
> > From: Gaurav Jain
> > Sent: Friday, January 31, 2020 1:58 PM
> > To: Ard Biesheuvel 
> > Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj
> > Bansal 
> > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT
> Runtime Services test.
> >
> >
> >
> > > -Original Message-
> > > From: Ard Biesheuvel 
> > > Sent: Thursday, January 30, 2020 2:52 PM
> > > To: Gaurav Jain 
> > > Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj
> > > Bansal 
> > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT
> > > Runtime Services test.
> > >
> > > Caution: EXT Email
> > >
> > > On Thu, 30 Jan 2020 at 06:08, Gaurav Jain  wrote:
> > > >
> > > > ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> > > > SCT Test expect return as Invalid Parameter.
> > > > So removed ASSERT().
> > > >
> > >
> > > This is not all this patch does.
> > >
> > > > Signed-off-by: Gaurav Jain 
> > > > ---
> > > >  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 12
> > > > ++--
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > > > b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > > > index 08fb9b0100b6..9bfb7756f0cb 100644
> > > > --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > > > +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > > > @@ -85,10 +85,6 @@ IsDayValid (
> > > >IN  EFI_TIME  *Time
> > > >)
> > > >  {
> > > > -  ASSERT (Time->Day >= 1);
> > > > -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> > > > -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <=
> > > > 28);
> > > > -
> > > >if (Time->Day < 1 ||
> > > >Time->Day > mDayOfMonth[Time->Month - 1] ||
> > > >(Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28))
> > > > { @@ -105,14 +101,15 @@ IsTimeValid(
> > > >)
> > > >  {
> > > >// Check the input parameters are within the range specified by UEFI
> > > > -  if (Time->Year   < 1900   ||
> > > > -  Time->Year   >    ||
> > > > +  if (Time->Year   < 1998   ||
> > > > +  Time->Year   > 2099   ||
> > >
> > > That original range is based on the UEFI spec. On what basis are you
> > > making this change?
> > >
> > > If your RTC hardware cannot represent the original values, this is
> > > not the place to fix that.
> >
> > As per the UEFI SCT Test, SetWakeupTime_Conf expect
> EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100.
> > Below is the link to check the Test code
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Ftianocore%2Fedk2-test%2Fblob%2Fmaster%2Fuefi-
> sct%2FSctPkg%2FT
> >
> estCase%2FUEFI%2FEFI%2FRuntimeServices%2FTimeServices%2FBlackBoxTes
> t%2
> >
> FTimeServicesBBTestConformance.cdata=02%7C01%7Cgaurav.jain%40
> nxp.
> >
> com%7C3c6e62107ab149b8709808d7aefd73e8%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301
> >
> 635%7C0%7C0%7C637170275901140244sdata=83GGnHy%2BZvzz5yZo
> 8et56FQqH
>

[edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed SCT Issues in NonDiscoverablePciDevice.

2020-02-13 Thread Gaurav Jain
GetBarAttributes, MemRead, MemWrite consistency test failed
with Invalid BarIndex.
Added check for BarIndex and return Invalid Parameter.

PCI Controller Attribute operation with Unsupported Attributes
is failing.
Added check to return Unsupported when wrong attributed are set.

Signed-off-by: Gaurav Jain 
---
 .../NonDiscoverablePciDeviceIo.c  | 21 +++
 1 file changed, 21 insertions(+)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 76cb000602fc..804e7e6cc834 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -243,6 +243,10 @@ PciIoMemRead (
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
   EFI_STATUS  Status;
 
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
   if (Buffer == NULL) {
 return EFI_INVALID_PARAMETER;
   }
@@ -330,6 +334,10 @@ PciIoMemWrite (
   EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc;
   EFI_STATUS  Status;
 
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
   if (Buffer == NULL) {
 return EFI_INVALID_PARAMETER;
   }
@@ -1302,13 +1310,22 @@ PciIoAttributes (
 break;
 
   case EfiPciIoAttributeOperationEnable:
+if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
+  return EFI_UNSUPPORTED;
+}
 Attributes |= Dev->Attributes;
   case EfiPciIoAttributeOperationSet:
+if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
+  return EFI_UNSUPPORTED;
+}
 Enable = ((~Dev->Attributes & Attributes) & EFI_PCI_DEVICE_ENABLE) != 0;
 Dev->Attributes = Attributes;
 break;
 
   case EfiPciIoAttributeOperationDisable:
+if ((Attributes & (~DEV_SUPPORTED_ATTRIBUTES)) != 0) {
+  return EFI_UNSUPPORTED;
+}
 Dev->Attributes &= ~Attributes;
 break;
 
@@ -1369,6 +1386,10 @@ PciIoGetBarAttributes (
 return EFI_INVALID_PARAMETER;
   }
 
+  if (BarIndex >= PCI_MAX_BAR) {
+return EFI_UNSUPPORTED;
+  }
+
   Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
 
   Status = GetBarResource (Dev, BarIndex, );
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54414): https://edk2.groups.io/g/devel/message/54414
Mute This Topic: https://groups.io/mt/71264395/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-02-11 Thread Gaurav Jain
Hi Ard

I am waiting for your response.

Regards
Gaurav

-Original Message-
From: Gaurav Jain 
Sent: Friday, January 31, 2020 1:58 PM
To: Ard Biesheuvel 
Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj Bansal 

Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime 
Services test.



> -Original Message-
> From: Ard Biesheuvel 
> Sent: Thursday, January 30, 2020 2:52 PM
> To: Gaurav Jain 
> Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj 
> Bansal 
> Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT 
> Runtime Services test.
> 
> Caution: EXT Email
> 
> On Thu, 30 Jan 2020 at 06:08, Gaurav Jain  wrote:
> >
> > ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> > SCT Test expect return as Invalid Parameter.
> > So removed ASSERT().
> >
> 
> This is not all this patch does.
> 
> > Signed-off-by: Gaurav Jain 
> > ---
> >  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 12 
> > ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > index 08fb9b0100b6..9bfb7756f0cb 100644
> > --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > @@ -85,10 +85,6 @@ IsDayValid (
> >IN  EFI_TIME  *Time
> >)
> >  {
> > -  ASSERT (Time->Day >= 1);
> > -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> > -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 
> > 28);
> > -
> >if (Time->Day < 1 ||
> >Time->Day > mDayOfMonth[Time->Month - 1] ||
> >(Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) { 
> > @@ -105,14 +101,15 @@ IsTimeValid(
> >)
> >  {
> >// Check the input parameters are within the range specified by UEFI
> > -  if (Time->Year   < 1900   ||
> > -  Time->Year   >    ||
> > +  if (Time->Year   < 1998   ||
> > +  Time->Year   > 2099   ||
> 
> That original range is based on the UEFI spec. On what basis are you 
> making this change?
> 
> If your RTC hardware cannot represent the original values, this is not 
> the place to fix that.

As per the UEFI SCT Test, SetWakeupTime_Conf expect EFI_INVALID_PARAMETER for 
Time.Year is 1997 and 2100.
Below is the link to check the Test code 
https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c
 (Line: 847)

Either UEFI spec need to be modified as per the test or SCT Test needs fix as 
per UEFI Specification.

> 
> 
> >Time->Month  < 1  ||
> >Time->Month  > 12 ||
> >!IsDayValid (Time)||
> >Time->Hour   > 23 ||
> >Time->Minute > 59 ||
> >Time->Second > 59 ||
> > +  Time->Nanosecond > 9  ||
> >!IsValidTimeZone (Time->TimeZone) ||
> >!IsValidDaylight (Time->Daylight)) {
> >  return FALSE;
> > @@ -254,6 +251,9 @@ SetWakeupTime (
> >OUT EFI_TIME*Time
> >)
> >  {
> > +  if (Time == NULL || !IsTimeValid (Time)) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> >return LibSetWakeupTime (Enabled, Time);  }
> >
> > --
> > 2.17.1
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54193): https://edk2.groups.io/g/devel/message/54193
Mute This Topic: https://groups.io/mt/71163888/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-01-31 Thread Gaurav Jain


> -Original Message-
> From: Ard Biesheuvel 
> Sent: Thursday, January 30, 2020 2:52 PM
> To: Gaurav Jain 
> Cc: devel@edk2.groups.io; Leif Lindholm ; Pankaj Bansal
> 
> Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime
> Services test.
> 
> Caution: EXT Email
> 
> On Thu, 30 Jan 2020 at 06:08, Gaurav Jain  wrote:
> >
> > ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> > SCT Test expect return as Invalid Parameter.
> > So removed ASSERT().
> >
> 
> This is not all this patch does.
> 
> > Signed-off-by: Gaurav Jain 
> > ---
> >  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 12 ++--
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > index 08fb9b0100b6..9bfb7756f0cb 100644
> > --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> > @@ -85,10 +85,6 @@ IsDayValid (
> >IN  EFI_TIME  *Time
> >)
> >  {
> > -  ASSERT (Time->Day >= 1);
> > -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> > -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
> > -
> >if (Time->Day < 1 ||
> >Time->Day > mDayOfMonth[Time->Month - 1] ||
> >(Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
> > @@ -105,14 +101,15 @@ IsTimeValid(
> >)
> >  {
> >// Check the input parameters are within the range specified by UEFI
> > -  if (Time->Year   < 1900   ||
> > -  Time->Year   >    ||
> > +  if (Time->Year   < 1998   ||
> > +  Time->Year   > 2099   ||
> 
> That original range is based on the UEFI spec. On what basis are you making 
> this
> change?
> 
> If your RTC hardware cannot represent the original values, this is not the 
> place
> to fix that.

As per the UEFI SCT Test, SetWakeupTime_Conf expect EFI_INVALID_PARAMETER for 
Time.Year is 1997 and 2100.
Below is the link to check the Test code
https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c
 (Line: 847)

Either UEFI spec need to be modified as per the test or SCT Test needs fix as 
per UEFI Specification.

> 
> 
> >Time->Month  < 1  ||
> >Time->Month  > 12 ||
> >!IsDayValid (Time)||
> >Time->Hour   > 23 ||
> >Time->Minute > 59 ||
> >Time->Second > 59 ||
> > +  Time->Nanosecond > 9  ||
> >!IsValidTimeZone (Time->TimeZone) ||
> >!IsValidDaylight (Time->Daylight)) {
> >  return FALSE;
> > @@ -254,6 +251,9 @@ SetWakeupTime (
> >OUT EFI_TIME*Time
> >)
> >  {
> > +  if (Time == NULL || !IsTimeValid (Time)) {
> > +return EFI_INVALID_PARAMETER;
> > +  }
> >return LibSetWakeupTime (Enabled, Time);  }
> >
> > --
> > 2.17.1
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53605): https://edk2.groups.io/g/devel/message/53605
Mute This Topic: https://groups.io/mt/70846440/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 1/1] MdeModulePkg/Pci: Fixed Asserts in SCT PCIIO Protocol Test.

2020-01-30 Thread Gaurav Jain
ASSERT in CopyMem_Conf, PollMem_Conf, SetBarAttributes_Conf
Conformance Test.
SCT Test expect return as Invalid Parameter.
So removed ASSERT().

Signed-off-by: Gaurav Jain 
---
 .../NonDiscoverablePciDeviceIo.c  | 20 ---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 2d55c9699322..76cb000602fc 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -93,7 +93,15 @@ PciIoPollMem (
   OUT UINT64  *Result
   )
 {
-  ASSERT (FALSE);
+  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
+  Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Result == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
   return EFI_UNSUPPORTED;
 }
 
@@ -556,7 +564,10 @@ PciIoCopyMem (
   IN UINTNCount
   )
 {
-  ASSERT (FALSE);
+  if ((UINT32)Width >= EfiPciIoWidthMaximum ||
+  Width > EfiPciIoWidthUint64) {
+return EFI_INVALID_PARAMETER;
+  }
   return EFI_UNSUPPORTED;
 }
 
@@ -1414,7 +1425,10 @@ PciIoSetBarAttributes (
   IN OUT UINT64   *Length
   )
 {
-  ASSERT (FALSE);
+  if (Offset == NULL || Length == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
   return EFI_UNSUPPORTED;
 }
 
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53566): https://edk2.groups.io/g/devel/message/53566
Mute This Topic: https://groups.io/mt/70267136/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

2020-01-29 Thread Gaurav Jain
ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
SCT Test expect return as Invalid Parameter.
So removed ASSERT().

Signed-off-by: Gaurav Jain 
---
 EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c 
b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
index 08fb9b0100b6..9bfb7756f0cb 100644
--- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
+++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
@@ -85,10 +85,6 @@ IsDayValid (
   IN  EFI_TIME  *Time
   )
 {
-  ASSERT (Time->Day >= 1);
-  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
-  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
-
   if (Time->Day < 1 ||
   Time->Day > mDayOfMonth[Time->Month - 1] ||
   (Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
@@ -105,14 +101,15 @@ IsTimeValid(
   )
 {
   // Check the input parameters are within the range specified by UEFI
-  if (Time->Year   < 1900   ||
-  Time->Year   >    ||
+  if (Time->Year   < 1998   ||
+  Time->Year   > 2099   ||
   Time->Month  < 1  ||
   Time->Month  > 12 ||
   !IsDayValid (Time)||
   Time->Hour   > 23 ||
   Time->Minute > 59 ||
   Time->Second > 59 ||
+  Time->Nanosecond > 9  ||
   !IsValidTimeZone (Time->TimeZone) ||
   !IsValidDaylight (Time->Daylight)) {
 return FALSE;
@@ -254,6 +251,9 @@ SetWakeupTime (
   OUT EFI_TIME*Time
   )
 {
+  if (Time == NULL || !IsTimeValid (Time)) {
+return EFI_INVALID_PARAMETER;
+  }
   return LibSetWakeupTime (Enabled, Time);
 }
 
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53557): https://edk2.groups.io/g/devel/message/53557
Mute This Topic: https://groups.io/mt/70266383/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-