date:20170924

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

2017-09-24 Thread Meenakshi Aggarwal

Hi All,


Please review the patch and share your comments.


Thanks,
Meenakshi

> -Original Message-
> From: Sakar Arora [mailto:sakar.ar...@arm.com]
> Sent: Wednesday, September 20, 2017 1:50 PM
> To: Ard Biesheuvel 
> Cc: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org; leif.lindh...@linaro.org
> Subject: RE: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> Thanks for the information. Seems my understanding was not correct in this
> context. I have no other doubts on this change.
> 
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, September 20, 2017 12:02 PM
> To: Sakar Arora 
> Cc: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org; leif.lindh...@linaro.org
> Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> 
> On 19 September 2017 at 22:32, Sakar Arora  wrote:
> > The DXE core dispatcher relies on the available memory to allocate space
> for loading the rest of the modules from the UEFI image. If we declare the
> UEFI image memory space (from which DXE dispatcher reads the efi
> modules) open to allocation, it might lead to data corruption, depending on
> the location of UEFI image and cumulative size of uncompressed EFI
> modules.
> >
> > Also, since UEFI allows unloading and loading of drivers at runtime, IMO,
> there is a need to preserve the UEFI image even after all the modules have
> been decompressed and loaded in the boot sequence.
> >
> 
> None of this is relevant. The uncompressed firmware volume containing DXE
> core and everything else is preserved as before. The only thing that gets
> discarded is the outer FV, which only contains the PrePi SEC module, and the
> compressed FV, neither of which is ever touched again after DXE core has
> started executing. So we should not register the FV in the first place, and 
> not
> reserve the memory so we don't lose it.
> 
> If you still think this may break anything, could you please elaborate?
> 
> 
> 
> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Tuesday, September 19, 2017 6:18 PM
> > To: Sakar Arora 
> > Cc: Meenakshi Aggarwal ;
> > edk2-devel@lists.01.org; leif.lindh...@linaro.org
> > Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> >
> > On 19 September 2017 at 01:07, Sakar Arora 
> wrote:
> >> This change will create the possibility for memory space holding the UEFI
> image to be over-written by the DXE core code, since this space will then be
> available for allocation.
> >
> > Yes. But why does this matter after the entire payload has been
> decompressed into memory already?
> >
> >
> >> Any such change, if required, should be done just before booting the OS.
> >>
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> >> Of Meenakshi Aggarwal
> >> Sent: Tuesday, September 19, 2017 6:02 PM
> >> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org;
> >> ard.biesheu...@linaro.org
> >> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux
> >>
> >> While creating Hob list, ArmPlatformPkg is hiding UEFI memory.
> >> whereas this memory can be used by OS.
> >>
> >> This patch, allows OS to use UEFI code area.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Udit Kumar 
> >> Signed-off-by: Meenakshi Aggarwal 
> >> ---
> >>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69
> >> -
> >>  1 file changed, 69 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> >> index 2feb11f..d03214b 100644
> >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> >> @@ -70,11 +70,7 @@ MemoryPeim (
> >>  {
> >>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> >>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> >> -  UINT64   ResourceLength;
> >>EFI_PEI_HOB_POINTERS NextHob;
> >> -  EFI_PHYSICAL_ADDRESS FdTop;
> >> -  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> >> -  EFI_PHYSICAL_ADDRESS ResourceTop;
> >>BOOLEAN  Found;
> >>
> >>// Get Virtual Memory Map from the Platform Library @@ -121,71
> +117,6 @@ MemoryPeim (
> >>  );
> >>}
> >>
> >> -  //
> >> -  // Reserved the memory space occupied by the firmware volume
> >> -  //
> >> -
> >> -  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> >> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> >> (PcdSystemMemorySize);
> >> -  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> >> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> >> -
> >> -

Re: [edk2] SNP transmit print

2017-09-24 Thread Meenakshi Aggarwal

Hi,

Created bug:

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

Thanks,
Meenakshi

> -Original Message-
> From: Fu, Siyuan [mailto:siyuan...@intel.com]
> Sent: Monday, September 25, 2017 6:29 AM
> To: Udit Kumar ; Meenakshi Aggarwal
> ; edk2-devel@lists.01.org
> Cc: Tian, Feng ; Zeng, Star 
> Subject: RE: SNP transmit print
> 
> Hi, Meenakshi
> 
> You are correct that it's a bug in SNP driver to print error information for
> successfully transmitted packet. Please create a Bugzilla ticket for this 
> issue
> and we will follow it. Thanks.
> 
> 
> BestRegards
> Fu Siyuan
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Udit Kumar
> Sent: Friday, September 22, 2017 7:04 PM
> To: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org
> Cc: Tian, Feng ; Zeng, Star 
> Subject: Re: [edk2] SNP transmit print
> 
> Thanks Meenakshi for digging a bit more
> 
> If we have use like below
> Library/DxeNetLib/DxeNetLib.c
> Status = Snp->Transmit (Snp, 0, Length, Packet, NULL, NULL, NULL);
> if ((Status != EFI_SUCCESS) && (Status != EFI_NOT_READY)) {
>   Status = EFI_DEVICE_ERROR;
>   break;
> }
> Then, we can simple put print when Status is EFI_DEVICE_ERROR;
> 
> But with below We are retrying once ☹ ,  What I think if first time if you got
> error EFI_NOT_READY, then after MnpRecycleTxBuf we likely not to get this
> error.
> I am not 100% sure here.
> 
> What I think, in MdeModulePkg/Universal/Network/SnpDxe/Transmit.c
> Have print only for EFI_NOT_READY.
> And in MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c, print error as
> well for second case.
> Hope this helps
> 
> Thanks
> Udit
> 
> > -Original Message-
> > From: Meenakshi Aggarwal
> > Sent: Friday, September 22, 2017 3:56 PM
> > To: Udit Kumar ; edk2-devel@lists.01.org
> > Cc: feng.t...@intel.com; star.z...@intel.com
> > Subject: RE: SNP transmit print
> >
> > Hi Udit,
> >
> > Yes, I think we should print this message is Status is
> > EFI_DEVICE_ERROR because in case of EFI_NOT_READY, we are retrying:
> >
> >   //
> >   // Transmit the packet through SNP.
> >   //
> >   Status = Snp->Transmit (
> >   Snp,
> >   HeaderSize,
> >   Length,
> >   Packet,
> >   TxData->SourceAddress,
> >   TxData->DestinationAddress,
> >   
> >   );
> >   if (Status == EFI_NOT_READY) {
> > Status = MnpRecycleTxBuf (MnpDeviceData);
> > if (EFI_ERROR (Status)) {
> >   Token->Status = EFI_DEVICE_ERROR;
> >   goto SIGNAL_TOKEN;
> > }
> >
> > DEBUG ((EFI_D_ERROR, "Snp->Transmit  retry\n"));
> > Status = Snp->Transmit (
> > Snp,
> > HeaderSize,
> > Length,
> > Packet,
> > TxData->SourceAddress,
> > TxData->DestinationAddress,
> > 
> > );
> >   }
> >
> > (MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c)
> >
> >
> > Please suggest if we can remove this print in case of BUSY status.
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Udit Kumar
> > > Sent: Thursday, September 21, 2017 5:21 PM
> > > To: Meenakshi Aggarwal ; edk2-
> > > de...@lists.01.org
> > > Subject: RE: SNP transmit print
> > >
> > > I think these error prints should be check against Status
> > >
> > > Regards
> > > Udit
> > >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
> > > > Behalf Of Meenakshi Aggarwal
> > > > Sent: Thursday, September 21, 2017 3:36 PM
> > > > To: edk2-devel@lists.01.org
> > > > Subject: [edk2] SNP transmit print
> > > >
> > > > Hi,
> > > >
> > > >
> > > > While performing tftp using PCI interface, below message is coming
> > > > continuously:
> > > >
> > > > Snp->undi.transmit()  8000h:4h
> > > >
> > > > Snp->undi.transmit()  8000h:4h
> > > > [==> ]   34812 Kb
> > > > Snp->undi.transmit()  8000h:4h
> > > >
> > > > Snp->undi.transmit()  8000h:4h
> > > >
> > > > Snp->undi.transmit()  8000h:4h
> > > >
> > > > Snp->undi.transmit()  8000h:4h
> > > >
> > > >
> > > > It is coming from file
> > > "MdeModulePkg/Universal/Network/SnpDxe/Transmit.c"
> > > >
> > > >   DEBUG (
> > > > (EFI_D_ERROR,
> > > > "\nSnp->undi.transmit()  %xh:%xh\n",
> > > > Snp->Cdb.StatFlags,
> > > > Snp->Cdb.StatCode)
> > > > );
> > > >
> > > >
> > > > I want to know if it is really an error message because tftp and
> > > > ping are working perfectly, but this error message is coming.
> > > >
> > > >
> > > > Thanks,
> > > > Meenakshi
> > > >
> > > >
> > > >
> > > >

Re: [edk2] [RFC 0/6] Create central repository for boilerplate configuration

2017-09-24 Thread Gao, Liming

Laszlo and Leif:
  PCD value are the position relation. In DSC file, the latter one will 
override the first one. But, PCD type can't be overridden. For example, if PCD 
is listed in [PcdsFixedAtBuild] section in the config.inc, PCD can't be listed 
in [PcdsPatchableInModule] section in Platform.dsc. I think Platform may not 
only override PCD value, but also override PCD type. To meet with platform 
requirement, we had better not place PCD setting in common DSC file. 

Thanks
Liming
>-Original Message-
>From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>Sent: Sunday, September 24, 2017 12:58 AM
>To: Laszlo Ersek 
>Cc: edk2-devel@lists.01.org; Kinney, Michael D
>; Justen, Jordan L ;
>Andrew Fish ; Ard Biesheuvel
>; Gao, Liming ; Zhu,
>Yonghong 
>Subject: Re: [edk2] [RFC 0/6] Create central repository for boilerplate
>configuration
>
>On Fri, Sep 22, 2017 at 01:20:57PM +0200, Laszlo Ersek wrote:
>> On 09/20/17 23:09, Leif Lindholm wrote:
>> > On Wed, Sep 20, 2017 at 08:14:59PM +0200, Laszlo Ersek wrote:
>>
>> >> (2) Replacing a build define called FOOBAR with CONFIG_FOOBAR will
>break
>> >> all downstream build scripts. Is the CONFIG_ prefix a requirement?
>> >
>> > It was explicitly intended to break compatibility, to ensure we didn't
>> > end up with things accidentally working until something unrelated
>> > changed in the future.
>>
>> Interesting idea. I guess we could try to reach out to all of the
>> "repeat builders" of OVMF.
>
>The proposal to drive the CONFIG options as Pcds would also be a
>solution to this issue.
>
>> >> (3) I think PCDs should not be included in ConfigPkg DSC include files,
>> >> even if several platforms set the same value. The set of libraries and
>> >> driver modules commonly used for a given feature is mostly constant
>> >> across platforms (and it is easy to extend, incrementally); but I don't
>> >> think the same holds for PCDs. Especially if a user wants to change a
>> >> PCD for one platform but not the other. Even if repeated settings for a
>> >> PCD worked (all on the same level of "specificity"), I'd find the result
>> >> confusing.
>> >
>> > Also a subject for discussion.
>> > My intent was that if most of the open source platforms had an
>> > override on the default of a particular Pcd, we could override it in
>> > the config fragments without changing the .dec (and affecting
>> > non-public ports).
>>
>> Right, that's great...
>>
>> > Individual platforms can still override (again).
>>
>> ... but this "again" part is what confuses me (assuming it would
>> technically work). We'd have a PCD default in the .dec, then a setting
>> in the central .dsc.inc that ultimately qualifies as a platform-level
>> setting, and finally a setting in the actual platform .dsc, which *also*
>> qualifies as a platform-level setting. IOW, one in the .dec, and two in
>> the (final) .dsc.
>
>Yes. But I cannot think of another way of handling it, that does not
>also mean stuffing a lot of boiler plate back into the platform-level
>files.
>
>> I have no clue if this works, but even if it does, the priority could
>> depend on the order of inclusion, which I find confusing.
>
>Oh, definitely. But also under complete control of the platform.
>
>Potentially, if this becomes some great success story, we will want to
>extend the build command with a separate [includes] section to give
>greater control over enforcing order.
>
>> Liming, Yonghong, can you guys please comment on this?
>
>Yes, please :)
>
>Regards,
>
>Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr deref

2017-09-24 Thread Zeng, Star

Reviewed-by: Star Zeng 

-Original Message-
From: Wu, Hao A 
Sent: Monday, September 25, 2017 9:51 AM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Zeng, Star 
Subject: [PATCH] MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr 
deref

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 3ed187b279..2db441725c 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -1178,6 +1178,7 @@ Done:
   // EFI_ALREADY_STARTED is not an error for bus driver.
   // Return the corresponding protocol interface.
   //
+  ASSERT (Prot != NULL);
   *Interface = Prot->Interface;
 } else if (Status == EFI_UNSUPPORTED) {
   //
-- 
2.12.0.windows.1

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

Re: [edk2] Repeated condition check

2017-09-24 Thread Santhapur Naveen

Hi Jiaxin,

Created a bug in the Bugzilla site.
Here's the bug link https://bugzilla.tianocore.org/show_bug.cgi?id=718

Thank you
Naveen

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Monday, September 25, 2017 10:40 AM
To: Santhapur Naveen; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: Repeated condition check

Thanks Naveen, we appreciate your report:).


> -Original Message-
> From: Santhapur Naveen [mailto:nave...@amiindia.co.in]
> Sent: Monday, September 25, 2017 1:05 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan 
> Subject: RE: Repeated condition check
> 
> Hi Jiaxin,
> 
> Duly noted.
> Will mention one of you in the TO/CC list from here onwards.
> 
> Thank you
> Naveen
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Monday, September 25, 2017 10:33 AM
> To: Santhapur Naveen; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: Repeated condition check
> 
> Hi Naveen,
> 
> We may ignore your email  if you don't attach any specific receiver 
> (TO OR CC list):(.
> 
> For the issue, I agree the fix. So, can you help to report the issue on 
> Bugzilla?
> We will create the formal patch to fix it.
> 
> Thanks,
> Jiaxin
> 
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Santhapur Naveen
> > Sent: Friday, September 22, 2017 4:40 PM
> > To: edk2-devel@lists.01.org
> > Subject: Re: [edk2] Repeated condition check
> >
> > Hello guys,
> >
> > Any update?
> >
> > Thank you
> > Naveen
> >
> > -Original Message-
> > From: Santhapur Naveen
> > Sent: Wednesday, September 20, 2017 12:05 PM
> > To: edk2-devel@lists.01.org
> > Subject: Repeated condition check
> >
> > Hi all,
> >
> > There is a redundant condition check in the API
> > PxeBcIcmp6ErrorDpcHandle()
> >
> > VOID
> > EFIAPI
> > PxeBcIcmp6ErrorDpcHandle (
> >   IN VOID *Context
> >   )
> > {
> >  ...
> >
> >   if (Type != ICMP_V6_DEST_UNREACHABLE &&
> >   Type != ICMP_V6_PACKET_TOO_BIG &&
> > -Type != ICMP_V6_PACKET_TOO_BIG &&
> > +Type != ICMP_V6_TIME_EXCEEDED &&
> >   Type != ICMP_V6_PARAMETER_PROBLEM) {
> > //
> > // The type of the receveid packet should be an ICMP6 error message.
> > //
> > gBS->SignalEvent (RxData->RecycleSignal);
> > goto ON_EXIT;
> >   }
> >   ...
> > }
> >
> > Unfortunately, some error checking tools were not able to capture this.
> > Please confirm and take the necessary action.
> >
> > Thank you
> > Naveen
> > ___
> > 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] Repeated condition check

2017-09-24 Thread Wu, Jiaxin

Thanks Naveen, we appreciate your report:).


> -Original Message-
> From: Santhapur Naveen [mailto:nave...@amiindia.co.in]
> Sent: Monday, September 25, 2017 1:05 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan 
> Subject: RE: Repeated condition check
> 
> Hi Jiaxin,
> 
> Duly noted.
> Will mention one of you in the TO/CC list from here onwards.
> 
> Thank you
> Naveen
> 
> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Monday, September 25, 2017 10:33 AM
> To: Santhapur Naveen; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: Repeated condition check
> 
> Hi Naveen,
> 
> We may ignore your email  if you don't attach any specific receiver (TO OR CC
> list):(.
> 
> For the issue, I agree the fix. So, can you help to report the issue on 
> Bugzilla?
> We will create the formal patch to fix it.
> 
> Thanks,
> Jiaxin
> 
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Santhapur Naveen
> > Sent: Friday, September 22, 2017 4:40 PM
> > To: edk2-devel@lists.01.org
> > Subject: Re: [edk2] Repeated condition check
> >
> > Hello guys,
> >
> > Any update?
> >
> > Thank you
> > Naveen
> >
> > -Original Message-
> > From: Santhapur Naveen
> > Sent: Wednesday, September 20, 2017 12:05 PM
> > To: edk2-devel@lists.01.org
> > Subject: Repeated condition check
> >
> > Hi all,
> >
> > There is a redundant condition check in the API
> > PxeBcIcmp6ErrorDpcHandle()
> >
> > VOID
> > EFIAPI
> > PxeBcIcmp6ErrorDpcHandle (
> >   IN VOID *Context
> >   )
> > {
> >  ...
> >
> >   if (Type != ICMP_V6_DEST_UNREACHABLE &&
> >   Type != ICMP_V6_PACKET_TOO_BIG &&
> > -Type != ICMP_V6_PACKET_TOO_BIG &&
> > +Type != ICMP_V6_TIME_EXCEEDED &&
> >   Type != ICMP_V6_PARAMETER_PROBLEM) {
> > //
> > // The type of the receveid packet should be an ICMP6 error message.
> > //
> > gBS->SignalEvent (RxData->RecycleSignal);
> > goto ON_EXIT;
> >   }
> >   ...
> > }
> >
> > Unfortunately, some error checking tools were not able to capture this.
> > Please confirm and take the necessary action.
> >
> > Thank you
> > Naveen
> > ___
> > 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] Repeated condition check

2017-09-24 Thread Santhapur Naveen

Hi Jiaxin,

Duly noted.
Will mention one of you in the TO/CC list from here onwards.

Thank you
Naveen

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Monday, September 25, 2017 10:33 AM
To: Santhapur Naveen; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: Repeated condition check

Hi Naveen, 

We may ignore your email  if you don't attach any specific receiver (TO OR CC 
list):(.

For the issue, I agree the fix. So, can you help to report the issue on 
Bugzilla? We will create the formal patch to fix it.

Thanks,
Jiaxin



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Santhapur Naveen
> Sent: Friday, September 22, 2017 4:40 PM
> To: edk2-devel@lists.01.org
> Subject: Re: [edk2] Repeated condition check
> 
> Hello guys,
> 
> Any update?
> 
> Thank you
> Naveen
> 
> -Original Message-
> From: Santhapur Naveen
> Sent: Wednesday, September 20, 2017 12:05 PM
> To: edk2-devel@lists.01.org
> Subject: Repeated condition check
> 
> Hi all,
> 
> There is a redundant condition check in the API 
> PxeBcIcmp6ErrorDpcHandle()
> 
> VOID
> EFIAPI
> PxeBcIcmp6ErrorDpcHandle (
>   IN VOID *Context
>   )
> {
>  ...
> 
>   if (Type != ICMP_V6_DEST_UNREACHABLE &&
>   Type != ICMP_V6_PACKET_TOO_BIG &&
> -Type != ICMP_V6_PACKET_TOO_BIG &&
> +Type != ICMP_V6_TIME_EXCEEDED &&
>   Type != ICMP_V6_PARAMETER_PROBLEM) {
> //
> // The type of the receveid packet should be an ICMP6 error message.
> //
> gBS->SignalEvent (RxData->RecycleSignal);
> goto ON_EXIT;
>   }
>   ...
> }
> 
> Unfortunately, some error checking tools were not able to capture this.
> Please confirm and take the necessary action.
> 
> Thank you
> Naveen
> ___
> 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] Repeated condition check

2017-09-24 Thread Wu, Jiaxin

Hi Naveen, 

We may ignore your email  if you don't attach any specific receiver (TO OR CC 
list):(.

For the issue, I agree the fix. So, can you help to report the issue on 
Bugzilla? We will create the formal patch to fix it.

Thanks,
Jiaxin



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Santhapur Naveen
> Sent: Friday, September 22, 2017 4:40 PM
> To: edk2-devel@lists.01.org
> Subject: Re: [edk2] Repeated condition check
> 
> Hello guys,
> 
> Any update?
> 
> Thank you
> Naveen
> 
> -Original Message-
> From: Santhapur Naveen
> Sent: Wednesday, September 20, 2017 12:05 PM
> To: edk2-devel@lists.01.org
> Subject: Repeated condition check
> 
> Hi all,
> 
> There is a redundant condition check in the API PxeBcIcmp6ErrorDpcHandle()
> 
> VOID
> EFIAPI
> PxeBcIcmp6ErrorDpcHandle (
>   IN VOID *Context
>   )
> {
>  ...
> 
>   if (Type != ICMP_V6_DEST_UNREACHABLE &&
>   Type != ICMP_V6_PACKET_TOO_BIG &&
> -Type != ICMP_V6_PACKET_TOO_BIG &&
> +Type != ICMP_V6_TIME_EXCEEDED &&
>   Type != ICMP_V6_PARAMETER_PROBLEM) {
> //
> // The type of the receveid packet should be an ICMP6 error message.
> //
> gBS->SignalEvent (RxData->RecycleSignal);
> goto ON_EXIT;
>   }
>   ...
> }
> 
> Unfortunately, some error checking tools were not able to capture this.
> Please confirm and take the necessary action.
> 
> Thank you
> Naveen
> ___
> 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] BaseTools/Conf: Support LLVM39 and LLVM40 in CLANG38 toolchain

2017-09-24 Thread Gao, Liming

Marvin:
  My concern is that the fix is an incompatible change. It requires to modify 
library API. In fact, this is an undefined behavior. But, current compiler (VS, 
GCC and Clang) makes it work.  So, I prefer to keep the code as-is, and disable 
this warning first. If you find any real issue, we can return back and figure 
out the solution. 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Marvin H?user
>Sent: Friday, September 22, 2017 11:53 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in
>CLANG38 toolchain
>
>Hey,
>
>I just noticed this patch as it recently has been pushed. I found this has 
>been a
>reaction to https://bugzilla.tianocore.org/show_bug.cgi?id=410
>Though as Clang correctly detected, this is Undefined Behavior per the C
>specification, so why was the warning hidden?
>In context of the issue in UefiLib, providing the first element of the VA list 
>as a
>prototyped argument, would have solved the issue without UB.
>
>Do you wish such a patch?
>
>Thanks,
>Marvin.
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Gao, Liming
>> Sent: Monday, August 28, 2017 9:19 AM
>> To: Shi, Steven ; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40
>in
>> CLANG38 toolchain
>>
>> Reviewed-by: Liming Gao 
>>
>> >-Original Message-
>> >From: Shi, Steven
>> >Sent: Wednesday, August 23, 2017 3:01 PM
>> >To: edk2-devel@lists.01.org; Gao, Liming 
>> >Cc: Zhu, Yonghong ; Shi, Steven
>> >
>> >Subject: [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in
>CLANG38
>> >toolchain
>> >
>> >From: "Shi, Steven" 
>> >
>> >Add LLVM39 and LLVM40 support in CLANG38 toolchain
>> >
>> >Contributed-under: TianoCore Contribution Agreement 1.0
>> >Signed-off-by: Steven Shi 
>> >---
>> > BaseTools/Conf/tools_def.template | 5 +++--
>> > 1 file changed, 3 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/BaseTools/Conf/tools_def.template
>> >b/BaseTools/Conf/tools_def.template
>> >index 1fa3ca3..2f83341 100755
>> >--- a/BaseTools/Conf/tools_def.template
>> >+++ b/BaseTools/Conf/tools_def.template
>> >@@ -380,7 +380,8 @@ DEFINE SOURCERY_CYGWIN_TOOLS =
>> /cygdrive/c/Program
>> >Files/CodeSourcery/Sourcery G
>> > #   Intel(r) ACPI Compiler from
>> > #   https://acpica.org/downloads
>> > #   CLANG38  -Linux-  Requires:
>> >-# Clang v3.8 or later, LLVMgold plugin and GNU 
>> >binutils 2.26
>> >targeting x86_64-linux-gnu
>> >+# Clang v3.8, LLVMgold plugin and GNU binutils 
>> >2.26
>> targeting
>> >x86_64-linux-gnu
>> >+# Clang v3.9 or later, LLVMgold plugin and GNU 
>> >binutils 2.28
>> >targeting x86_64-linux-gnu
>> > #Optional:
>> > # Required to build platforms or ACPI tables:
>> > #   Intel(r) ACPI Compiler from
>> >@@ -5512,7 +5513,7 @@ DEFINE CLANG38_X64_PREFIX   =
>> >ENV(CLANG38_BIN)
>> > DEFINE CLANG38_IA32_TARGET  = -target i686-pc-linux-gnu
>> > DEFINE CLANG38_X64_TARGET   = -target x86_64-pc-linux-gnu
>> >
>> >-DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -
>Wno-
>> >empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-
>shift-
>> >negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -
>> Wno-
>> >tautological-constant-out-of-range-compare -Wno-incompatible-library-
>> >redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -
>> msoft-
>> >float -mno-implicit-float  -ftrap-
>> >function=undefined_behavior_has_been_optimized_away_by_clang -
>> >funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-
>> >tautological-compare -Wno-unknown-warning-option
>> >+DEFINE CLANG38_ALL_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -
>> Wno-
>> >empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-
>shift-
>> >negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -
>> Wno-
>> >tautological-constant-out-of-range-compare -Wno-incompatible-library-
>> >redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -
>> msoft-
>> >float -mno-implicit-float  -ftrap-
>> >function=undefined_behavior_has_been_optimized_away_by_clang -
>> >funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-
>> >tautological-compare -Wno-unknown-warning-option -Wno-varargs
>> >
>> > ###
>> > # CLANG38 IA32 definitions
>> >--
>> >2.7.4
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] FDF Spec: Per PI 1.6 to extend FFS alignment to 16M

2017-09-24 Thread Yonghong Zhu

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 2_fdf_design_discussion/25_[fv]_sections.md   | 4 ++--
 3_edk_ii_fdf_file_format/32_fdf_definition.md | 4 +++-
 README.md | 1 +
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/2_fdf_design_discussion/25_[fv]_sections.md 
b/2_fdf_design_discussion/25_[fv]_sections.md
index 928455c..82755dd 100644
--- a/2_fdf_design_discussion/25_[fv]_sections.md
+++ b/2_fdf_design_discussion/25_[fv]_sections.md
@@ -339,12 +339,12 @@ The `Fixed` and `Checksum` attributes are boolean flags, 
both default to
 
 The Alignment attribute requires the "= value".
 
 * `Fixed` - File can not be moved, default (not specified) is relocate-able.
 
-* `Alignment` - Data (value is one of: 1, 2 4, 8, 16, 32, 64 128, 512, 1K, 2K,
-  4K, 8K, 16K, 32K, 64K) byte aligned
+* `Alignment` - Data (value is one of: 1, 2, 4, 8, 16, 32, 64, 128, 512, 1K, 
2K,
+  4K, 8K, 16K, 32K, 64K, 128K, 256K, 512K, 1M, 2M, 4M, 8M, 16M) byte aligned
 
 * `Checksum` - It is recommended that this be controlled on an entire FV basis
   not at the file level, however, we are including this attribute for
   completeness.
 
diff --git a/3_edk_ii_fdf_file_format/32_fdf_definition.md 
b/3_edk_ii_fdf_file_format/32_fdf_definition.md
index 67976b5..1379db4 100644
--- a/3_edk_ii_fdf_file_format/32_fdf_definition.md
+++ b/3_edk_ii_fdf_file_format/32_fdf_definition.md
@@ -225,11 +225,13 @@ The following are common definitions used by multiple 
section types.
{"4K"} {"8K"} {"16K"} {"32K"} {"64K"}
{"128K"} {"256K"} {"512K"} {"1M"} {"2M"} {"4M"}
{"8M"} {"16M"} {"32M"} {"64M"}
{"128M"} {"256M"} {"512M"} {"1G"} {"2G"}
::= {"Auto"} {"8"} {"16"} {"32"} {"64"} {"128"}
-   {"512"} {"1K"} {"4K"} {"32K"} {"64K"}
+   {"512"} {"1K"} {"4K"} {"32K"} {"64K"} {"128K"}
+   {"256K"} {"512K"} {"1M"} {"2M"} {"4M"} {"8M"}
+   {"16M"}
 ```
 
 **
 **Note:** When using the characters "|" or "||" in an expression, the
 expression must be encapsulated in open "(" and close ")" parenthesis.
diff --git a/README.md b/README.md
index 7ff8f36..71c92fc 100644
--- a/README.md
+++ b/README.md
@@ -206,5 +206,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights 
reserved.
 || [#142](https://bugzilla.tianocore.org/show_bug.cgi?id=142) 
Update EDK II FDF Specification to allow sections in any order  
|   |
 || [#478](https://bugzilla.tianocore.org/show_bug.cgi?id=478) FDF 
spec: extend the  to support  and  
|   |
 || [#353](https://bugzilla.tianocore.org/show_bug.cgi?id=353) 
Build spec: Allow nested includes in DSC and FDF files  
|   |
 || [#520](https://bugzilla.tianocore.org/show_bug.cgi?id=520) FDF 
spec: Update Precedence of PCD Values   
|   |
 || [#585](https://bugzilla.tianocore.org/show_bug.cgi?id=585) FDF 
Spec: Update the FDF_SPECIFICATION version to 0x0001001B or 1.27
|   |
+|| Per PI 1.6 to extend FFS alignment to 16M   

   |   |
-- 
2.6.1.windows.1

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

[edk2] [Patch] Build Spec: Build report to display every module's build time

2017-09-24 Thread Yonghong Zhu

fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=717
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 13_build_reports/134_platform_summary.md |  8 +-
 13_build_reports/138_module_section.md   | 43 +++-
 README.md|  1 +
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/13_build_reports/134_platform_summary.md 
b/13_build_reports/134_platform_summary.md
index 0674141..083d714 100644
--- a/13_build_reports/134_platform_summary.md
+++ b/13_build_reports/134_platform_summary.md
@@ -41,10 +41,13 @@ following items:
 * Tool Chain : %Tool chain string%
 * Target : %Target String"
 * Output Path : %Path to platform build directory%
 * Build Environment : %Environment string reported by Python%
 * Build Duration : %Build duration time string%
+* AutoGen Duration : %AutoGen duration time string if it exists%
+* Make Duration : %Make duration time string if it exists%
+* GenFds Duration : %GenFds duration time string if it exists%
 * Report Content : %List of flags the control the report content%
 
  Example
 
 ```
@@ -53,11 +56,14 @@ Platform DSC Path:  s:\edk2\Nt32Pkg\Nt32Pkg.dsc
 Architectures:  IA32
 Tool Chain: VS2008x86
 Target: DEBUG
 Output Path:s:\edk2\Build\NT32IA32
 Build Environment:  Windows-7-6.1.7601-SP1
-Build Duration: 00:01:53
+Build Duration: 00:01:29
+AutoGen Duration:   00:00:10
+Make Duration:  00:01:02
+GenFds Duration:00:00:15
 Report Contents:PCD, LIBRARY, BUILD_FLAGS, DEPEX, FLASH, FIXED_ADDRESS
 ```
 
 **
 **Note:** Platform Summary is always present and appears at the beginning of
diff --git a/13_build_reports/138_module_section.md 
b/13_build_reports/138_module_section.md
index d4aa365..455537b 100644
--- a/13_build_reports/138_module_section.md
+++ b/13_build_reports/138_module_section.md
@@ -45,10 +45,11 @@ file GUID, module size, module build time stamp and driver 
type.
 * Module INF Path: %Path of Module INF file%
 * File GUID: %Module GUID: '`FILE_GUID`' in INF `[Defines]` section%
 * Size: %Module EFI image size%
 * Build time stamp: %The time stamp in module PE32 image% (If the time stamp is
   cleared to be zero, the build time stamp is 1970-01-01 00:00:00 UTC time.)
+* Module Build Time: %The time string for this module's build%
 * Driver Type: %The driver's file type code[^2] and name in firmware volume%
 
 The following entries are options:
 
 * If using defaults or the `HASH` flag is specified:
@@ -77,10 +78,11 @@ Module Name:SmbiosDxe
 Module INF Path:MdeModule\Universal\SmbiosDxe\SmbiosDxe.inf
 File GUID:  F9D88642-0737-49BC-81B5-6889CD57D9EA
 Size:   0x7000 (28.00K)
 SHA1 HASH:  d94c3f180f25d6b562f477bc4a16b286cb66a8b6 *SmbiosDxe.efi
 Build Time Stamp:   1969-12-31 16:00:00
+Module Build Time:  1060ms
 Driver Type:0x7 (DRIVER)
 
 ... (Module Section Details for SmbiosDxe)
 <==>
 ```
@@ -94,10 +96,11 @@ Module Name:EbcDxe
 Module INF Path:MdeModule\Universal\EbcDxe\EbcDxe.inf
 File GUID:  13AC6DD0-73D0-11D4-B06B-00AA00BD6DE7
 Size:   0x9000 (36.00K)
 SHA1 HASH:  ff4c019345614afe5c88e7fc37219c30a07f4af4 *EbcDxe.efi
 Time Stamp: 1969-12-31 16:00:00
+Module Build Time:  1731ms
 Driver Type:0x7 (DRIVER)
 
 ... (Module Section Details for EbcDxe)
 
 <==>
@@ -114,10 +117,11 @@ destructor calling sequence.
 
 * Library INF Path: Path of library instance INF file
 * Class\*: The library class name of the library instance
 * C\*: The library constructor if it exists
 * D\*: The library destructor if it exists
+* Time: The build time of this library if it exists
 
 The items marked with \* are only available when the module is an EDKII style
 module and they must be listed in the next line immediately after library
 instance's INF path.
 
@@ -141,50 +145,55 @@ format is:
  inserted in the curly braces before the closing curly brace.
  ```
  D = DestructorCname
  ```
 
+   * Display the build time.
+ ```
+ Time = TimeString
+ ```
+
  Example1:
 
 ```c
 >--<
 Library
 ---
 s:\edk2\MdePkg\Library\UefiDevicePathLib\UefiDevicePathLib.inf
-{DevicePathLib}
+{DevicePathLib: Time = 643ms}
 s:\edk2\MdePkg\Library\BaseLib\BaseLib.inf
-{BaseLib}
+{BaseLib: Time = 14702ms}

[edk2] [PATCH] MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr deref

2017-09-24 Thread Hao Wu

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 3ed187b279..2db441725c 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -1178,6 +1178,7 @@ Done:
   // EFI_ALREADY_STARTED is not an error for bus driver.
   // Return the corresponding protocol interface.
   //
+  ASSERT (Prot != NULL);
   *Interface = Prot->Interface;
 } else if (Status == EFI_UNSUPPORTED) {
   //
-- 
2.12.0.windows.1

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

[edk2] [platforms: PATCH 06/10] Marvell/Applications/SpiTool: Fix bug in error test

2017-09-24 Thread Marcin Wojtas

From: Ard Biesheuvel 

Fix a misplaced closing parenthesis.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c 
b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index b6dc54f..e6e1007 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -378,7 +378,7 @@ EFI_STATUS  Status;
   FilePath = (CHAR16 *) FileStr;
   Status = ShellIsFile (FilePath);
   // When read file into flash, file doesn't have to exist
-  if (EFI_ERROR(Status && !(Flag & READ_FILE))) {
+  if (EFI_ERROR (Status) && !(Flag & READ_FILE)) {
 Print (L"sf: Wrong FilePath parameter!\n");
 return SHELL_ABORTED;
   }
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 09/10] Marvell/Drivers: MvSpiFlash: Fix usage of erase size parameter

2017-09-24 Thread Marcin Wojtas

Although, hitherto support allowed for using configurable EraseSize,
the erase command was fixed to CMD_ERASE_64K. Also it was
assumed that EraseSize equals SectorSize, which is not true
for some flash devices. Fix both issues by adding new PCD
(gMarvellTokenSpaceGuid.PcdSpiFlashPageSize) and using
this parameter properly in MvSpiFlashUpdate routine instead
of the EraseSize. Also erase command is adjusted to the settings.
Update PortingGuide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c   | 26 +++-
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h   |  6 +
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf |  1 +
 Platform/Marvell/Marvell.dec|  1 +
 Silicon/Marvell/Documentation/PortingGuide.txt  |  3 +++
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c 
b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index 9a04493..c411296 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -191,7 +191,21 @@ MvSpiFlashErase (
 return EFI_DEVICE_ERROR;
   }
 
-  Cmd[0] = CMD_ERASE_64K;
+  switch (EraseSize) {
+  case SIZE_4K:
+Cmd[0] = CMD_ERASE_4K;
+break;
+  case SIZE_32K:
+Cmd[0] = CMD_ERASE_32K;
+break;
+  case SIZE_64K:
+Cmd[0] = CMD_ERASE_64K;
+break;
+  default:
+DEBUG ((DEBUG_ERROR, "MvSpiFlash: Invalid EraseSize parameter\n"));
+return EFI_INVALID_PARAMETER;
+  }
+
   while (Length) {
 EraseAddr = Offset;
 
@@ -353,14 +367,14 @@ MvSpiFlashUpdate (
   )
 {
   EFI_STATUS Status;
-  UINT64 EraseSize, ToUpdate, Scale = 1;
+  UINT64 SectorSize, ToUpdate, Scale = 1;
   UINT8 *TmpBuf, *End;
 
-  EraseSize = PcdGet64 (PcdSpiFlashEraseSize);
+  SectorSize = PcdGet64 (PcdSpiFlashSectorSize);
 
   End = Buf + ByteCount;
 
-  TmpBuf = (UINT8 *)AllocateZeroPool (EraseSize);
+  TmpBuf = (UINT8 *)AllocateZeroPool (SectorSize);
   if (TmpBuf == NULL) {
 DEBUG((DEBUG_ERROR, "SpiFlash: Cannot allocate memory\n"));
 return EFI_OUT_OF_RESOURCES;
@@ -370,9 +384,9 @@ MvSpiFlashUpdate (
 Scale = (End - Buf) / 100;
 
   for (; Buf < End; Buf += ToUpdate, Offset += ToUpdate) {
-ToUpdate = MIN((UINT64)(End - Buf), EraseSize);
+ToUpdate = MIN((UINT64)(End - Buf), SectorSize);
 Print (L"   \rUpdating, %d%%", 100 - (End - Buf) / Scale);
-Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, 
EraseSize);
+Status = MvSpiFlashUpdateBlock (Slave, Offset, ToUpdate, Buf, TmpBuf, 
SectorSize);
 
 if (EFI_ERROR (Status)) {
   DEBUG((DEBUG_ERROR, "SpiFlash: Error while updating\n"));
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h 
b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
index 3889643..49cce43 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h
@@ -57,6 +57,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define CMD_READ_ARRAY_FAST 0x0b
 #define CMD_PAGE_PROGRAM0x02
 #define CMD_BANK_WRITE  0xc5
+#define CMD_ERASE_4K0x20
+#define CMD_ERASE_32K   0x52
 #define CMD_ERASE_64K   0xd8
 #define CMD_4B_ADDR_ENABLE  0xb7
 
@@ -66,6 +68,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define SPI_TRANSFER_BEGIN  0x01  // Assert CS before transfer
 #define SPI_TRANSFER_END0x02  // Deassert CS after transfers
 
+#define SIZE_4K 4096
+#define SIZE_32K32768
+#define SIZE_64K65536
+
 #define SPI_FLASH_16MB_BOUN 0x100
 
 typedef enum {
diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf 
b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
index d035d47..4519b02 100644
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf
@@ -58,6 +58,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize
   gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
+  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize
 
 [Protocols]
   gMarvellSpiMasterProtocolGuid
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 869e376..fc00f1a 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -127,6 +127,7 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashAddressCycles|0|UINT32|0x353
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x354
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x355
+  gMarvellTokenSpaceGuid.PcdSpiFlashSectorSize|65536|UINT64|0x359
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x356

[edk2] [platforms: PATCH 04/10] Marvell/Applications/SpiTool: Enable configurable CS and SCLK mode

2017-09-24 Thread Marcin Wojtas

Until now transfer SCLK mode and CS were fixed, when using
shell 'sf' command. This patch enables their configuration.
Update porting guide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c   |  6 +-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf |  2 ++
 Platform/Marvell/Marvell.dec  |  2 ++
 Silicon/Marvell/Documentation/PortingGuide.txt| 11 ++-
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c 
b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index 184e3d7..b6dc54f 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -218,6 +218,7 @@ EFI_STATUS  Status;
   CONST CHAR16  *LengthStr = NULL, *FileStr = NULL;
   BOOLEAN   AddrFlag = FALSE, LengthFlag = TRUE, FileFlag = FALSE;
   UINT8 Flag = 0, CheckFlag = 0;
+  UINT8 Mode, Cs;
 
   Status = gBS->LocateProtocol (
 ,
@@ -283,8 +284,11 @@ EFI_STATUS  Status;
 }
   }
 
+  Mode = PcdGet32 (PcdSpiFlashMode);
+  Cs = PcdGet32 (PcdSpiFlashCs);
+
   // Setup new spi device
-  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, 0, 0);
+  Slave = SpiMasterProtocol->SetupDevice (SpiMasterProtocol, Cs, Mode);
 if (Slave == NULL) {
   Print(L"sf: Cannot allocate SPI device!\n");
   return SHELL_ABORTED;
diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf 
b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
index 41b7b7c..887b9a5 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf
@@ -65,7 +65,9 @@
  FileHandleLib
 
 [Pcd]
+ gMarvellTokenSpaceGuid.PcdSpiFlashCs
  gMarvellTokenSpaceGuid.PcdSpiFlashId
+ gMarvellTokenSpaceGuid.PcdSpiFlashMode
 
 [Protocols]
  gMarvellSpiFlashProtocolGuid
diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec
index 4e2dd6d..869e376 100644
--- a/Platform/Marvell/Marvell.dec
+++ b/Platform/Marvell/Marvell.dec
@@ -128,6 +128,8 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|0|UINT64|0x354
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|0|UINT32|0x355
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0|UINT32|0x356
+  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0|UINT32|0x357
+  gMarvellTokenSpaceGuid.PcdSpiFlashMode|0|UINT32|0x358
 
 #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x0 }|VOID*|0x3098
diff --git a/Silicon/Marvell/Documentation/PortingGuide.txt 
b/Silicon/Marvell/Documentation/PortingGuide.txt
index aa53329..2be658e 100644
--- a/Silicon/Marvell/Documentation/PortingGuide.txt
+++ b/Silicon/Marvell/Documentation/PortingGuide.txt
@@ -285,11 +285,6 @@ Following PCDs are available for configuration of spi 
driver:
   - gMarvellTokenSpaceGuid.PcdSpiMaxFrequency
(Max SCLK line frequency (in Hz) (max transfer frequency) )
 
-  - gMarvellTokenSpaceGuid.PcdSpiDefaultMode
-   (default SCLK mode (see SPI_MODE enum in file
-edk2-platforms/Platform/Marvell/Drivers/Spi/MvSpi.h) )
-
-
 SpiFlash configuration
 ==
 Folowing PCDs for spi flash driver configuration must be set properly:
@@ -309,6 +304,12 @@ Folowing PCDs for spi flash driver configuration must be 
set properly:
   - gMarvellTokenSpaceGuid.PcdSpiFlashPollCmd
(Spi flash polling flag)
 
+  - gMarvellTokenSpaceGuid.PcdSpiFlashMode
+   (Default SCLK mode (see SPI_MODE enum in file
+edk2-platforms/Platform/Marvell/Drivers/Spi/MvSpi.h))
+
+  - gMarvellTokenSpaceGuid.PcdSpiFlashCs
+   (Chip select used for communication with the Flash)
 
 MPP configuration
 =
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 07/10] Marvell/Applications/FirmwareUpdate: Fix 32-bit issues

2017-09-24 Thread Marcin Wojtas

From: Ard Biesheuvel 

Fix casting and related issues to make this code build for 32-bit ARM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c 
b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index edb6986..664411a 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -165,7 +165,7 @@ PrepareFirmwareImage (
   IN LIST_ENTRY *CheckPackage,
   IN OUT SHELL_FILE_HANDLE  *FileHandle,
   IN OUT UINTN  **FileBuffer,
-  IN OUT UINTN  *FileSize
+  IN OUT UINT64 *FileSize
   )
 {
   CONST CHAR16 *FileStr;
@@ -203,7 +203,7 @@ PrepareFirmwareImage (
   // Read Image header into buffer
   Buffer = AllocateZeroPool (*FileSize);
 
-  Status = FileHandleRead (*FileHandle, FileSize, Buffer);
+  Status = FileHandleRead (*FileHandle, (UINTN *)FileSize, Buffer);
   if (EFI_ERROR (Status)) {
 Print (L"%s: Cannot read Image file header\n", CMD_NAME_STRING);
 ShellCloseFile (FileHandle);
@@ -256,7 +256,7 @@ ShellCommandRunFUpdate (
 {
   IN SHELL_FILE_HANDLEFileHandle;
   SPI_DEVICE  *Slave;
-  UINTN   FileSize;
+  UINT64  FileSize;
   UINTN   *FileBuffer = NULL;
   CHAR16  *ProblemParam;
   LIST_ENTRY  *CheckPackage;
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 10/10] Marvell/Drivers: MvSpiFlash: Minor style fix

2017-09-24 Thread Marcin Wojtas

This patch correct style of two variables to the camel-case
version.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c 
b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
index c411296..c7e0221 100755
--- a/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
+++ b/Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c
@@ -104,13 +104,13 @@ MvSpiFlashWriteCommon (
   UINT8 CmdStatus = CMD_READ_STATUS;
   UINT8 State;
   UINT32 Counter = 0xF;
-  UINT8 poll_bit = STATUS_REG_POLL_WIP;
-  UINT8 check_status = 0x0;
+  UINT8 PollBit = STATUS_REG_POLL_WIP;
+  UINT8 CheckStatus = 0x0;
 
   CmdStatus = (UINT8)PcdGet32 (PcdSpiFlashPollCmd);
   if (CmdStatus == CMD_FLAG_STATUS) {
-poll_bit = STATUS_REG_POLL_PEC;
-check_status = poll_bit;
+PollBit = STATUS_REG_POLL_PEC;
+CheckStatus = PollBit;
   }
 
   // Send command
@@ -127,7 +127,7 @@ MvSpiFlashWriteCommon (
 SpiMasterProtocol->Transfer (SpiMasterProtocol, Slave, 1, NULL, ,
   0);
 Counter--;
-if ((State & poll_bit) == check_status)
+if ((State & PollBit) == CheckStatus)
   break;
   } while (Counter > 0);
   if (Counter == 0) {
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 02/10] Marvell/Drivers: MvSpiDxe: Log and return correct error

2017-09-24 Thread Marcin Wojtas

From: Piotr Król 

Make log information clear where it came from and return correct code to
be interpreted by caller.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Piotr Król 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c 
b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
index aab20fc..0c6b624 100755
--- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
+++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
@@ -240,7 +240,8 @@ MvSpiTransfer (
 }
 
 if (Iterator >= SPI_TIMEOUT) {
-  DEBUG ((DEBUG_ERROR, "Timeout\n"));
+  DEBUG ((DEBUG_ERROR, "%s: Timeout\n", __FUNCTION__));
+  return EFI_TIMEOUT;
 }
   }
 
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 08/10] Marvell/Applications/SpiTool: Fix 32-bit issues

2017-09-24 Thread Marcin Wojtas

From: Ard Biesheuvel 

Fix casting and related issues to make this code build for 32-bit ARM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c 
b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
index e6e1007..9321f6b 100644
--- a/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
+++ b/Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c
@@ -211,7 +211,8 @@ EFI_STATUS  Status;
   LIST_ENTRY*CheckPackage;
   EFI_PHYSICAL_ADDRESS  Address = 0, Offset = 0;
   SHELL_FILE_HANDLE FileHandle = NULL;
-  UINTN ByteCount, FileSize, I;
+  UINTN ByteCount, I;
+  UINT64FileSize;
   UINT8 *Buffer = NULL, *FileBuffer = NULL;
   CHAR16*ProblemParam, *FilePath;
   CONST CHAR16  *AddressStr = NULL, *OffsetStr = NULL;
@@ -418,7 +419,7 @@ EFI_STATUS  Status;
 }
   }
 
-  Buffer = (UINT8 *) Address;
+  Buffer = (UINT8 *)(UINTN)Address;
   if (FileFlag) {
 Buffer = FileBuffer;
   }
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 05/10] Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash

2017-09-24 Thread Marcin Wojtas

This patch makes use of recently added SPI configuration
PCDs and sets CS with SCLK mode on Armada 7040 DB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Armada/Armada70x0.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
b/Platform/Marvell/Armada/Armada70x0.dsc
index caf3840..467dfa3 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -95,6 +95,8 @@
   gMarvellTokenSpaceGuid.PcdSpiFlashEraseSize|65536
   gMarvellTokenSpaceGuid.PcdSpiFlashPageSize|256
   gMarvellTokenSpaceGuid.PcdSpiFlashId|0x20BA18
+  gMarvellTokenSpaceGuid.PcdSpiFlashMode|3
+  gMarvellTokenSpaceGuid.PcdSpiFlashCs|0
 
   #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-- 
1.8.3.1

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

[edk2] [platforms: PATCH 00/10] Armada 70x0/80x0 SPI improvements

2017-09-24 Thread Marcin Wojtas

Hi,

Finally I found time to respin SPI patchset. As agreed, I extracted
style fixes from dynamic flash detection patch, but its main part
is kept aside for now until FlashId table (and additionally both SPI
protocols land in the edk2 mainline). A lot of minor fixes were
implemented, details can be found in the commit logs and changelog
below.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/spi-upstream-r20170925

I'm looking forward to the comments or remarks.

Best regards,
Marcin

Changelog
v1 -> v2
Slightly improve commit log prefixes.

1/10
* Move documentation to Silicon/Marvell
* Update PortingGuide with new paths in edk2-platforms repository

2/10
* Correct error print

4/10
* Sort PCDs

7/10
* Simplify change and remove additional local variable

8/10
* Correct FileSize type and let it really compile for ARM and AARCH64

9/10
* modify macros -> s/SPI_ERASE_SIZE_/SIZE_/

10/10
* New patch

3/10, 5/10, 6/10
* Add Reviewed-by's

Ard Biesheuvel (3):
  Marvell/Applications/SpiTool: Fix bug in error test
  Marvell/Applications/FirmwareUpdate: Fix 32-bit issues
  Marvell/Applications/SpiTool: Fix 32-bit issues

Joe Zhou (1):
  Marvell/Drivers: MvSpiDxe: Fix write bug

Marcin Wojtas (4):
  Marvell/Applications/SpiTool: Enable configurable CS and SCLK mode
  Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash
  Marvell/Drivers: MvSpiFlash: Fix usage of erase size parameter
  Marvell/Drivers: MvSpiFlash: Minor style fix

Nir Erez (1):
  Silicon/Marvell: Refactor Documentation

Piotr Król (1):
  Marvell/Drivers: MvSpiDxe: Log and return correct error

 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c   |   6 +-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.c  |  13 +-
 Platform/Marvell/Applications/SpiTool/SpiFlashCmd.inf|   2 +
 Platform/Marvell/Armada/Armada70x0.dsc   |   2 +
 Platform/Marvell/Documentation/Drivers/EepromDriver.txt  |  96 -
 Platform/Marvell/Documentation/Drivers/I2cDriver.txt |  64 
 Platform/Marvell/Documentation/Drivers/SpiDriver.txt | 116 --
 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt   |  45 ---
 Platform/Marvell/Documentation/PortingGuide/I2c.txt  |  20 --
 Platform/Marvell/Documentation/PortingGuide/Mdio.txt |   7 -
 Platform/Marvell/Documentation/PortingGuide/Mpp.txt  |  48 ---
 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt |  31 --
 Platform/Marvell/Documentation/PortingGuide/Phy.txt  |  45 ---
 Platform/Marvell/Documentation/PortingGuide/Pp2.txt  |  35 --
 Platform/Marvell/Documentation/PortingGuide/Reset.txt|   7 -
 Platform/Marvell/Documentation/PortingGuide/Spi.txt  |  16 -
 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt |  23 --
 Platform/Marvell/Documentation/PortingGuide/Utmi.txt |  35 --
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.c|  36 +-
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.h|   6 +
 Platform/Marvell/Drivers/Spi/Devices/MvSpiFlash.inf  |   1 +
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c  |   6 +-
 Platform/Marvell/Marvell.dec |   3 +
 Silicon/Marvell/Documentation/Drivers/EepromDriver.txt   |  96 +
 Silicon/Marvell/Documentation/Drivers/I2cDriver.txt  |  64 
 Silicon/Marvell/Documentation/Drivers/SpiDriver.txt  | 116 ++
 Silicon/Marvell/Documentation/PortingGuide.txt   | 377 

 27 files changed, 707 insertions(+), 609 deletions(-)
 delete mode 100644 Platform/Marvell/Documentation/Drivers/EepromDriver.txt
 delete mode 100644 Platform/Marvell/Documentation/Drivers/I2cDriver.txt
 delete mode 100644 Platform/Marvell/Documentation/Drivers/SpiDriver.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/I2c.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mdio.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Mpp.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Phy.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Pp2.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Reset.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Spi.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt
 delete mode 100644 Platform/Marvell/Documentation/PortingGuide/Utmi.txt
 create mode 100644 Silicon/Marvell/Documentation/Drivers/EepromDriver.txt
 create mode 100644 Silicon/Marvell/Documentation/Drivers/I2cDriver.txt
 create mode 100644 Silicon/Marvell/Documentation/Drivers/SpiDriver.txt
 create mode 100644 Silicon/Marvell/Documentation/PortingGuide.txt

-- 
1.8.3.1

[edk2] [platforms: PATCH 01/10] Silicon/Marvell: Refactor Documentation

2017-09-24 Thread Marcin Wojtas

From: Nir Erez 

This patch introduces following improvements to the PortingGuide
* Replace split documentation with single file
* Update paths to new directory structure in edk2-platforms
* Align format to Doxygen constraints

Moreover the PortingGuide and remaining Drivers' documentation
is moved to the new location under Silicon/Marvell, where in future
all other bits of the support will be moved.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Documentation/Drivers/EepromDriver.txt  |  96 -
 Platform/Marvell/Documentation/Drivers/I2cDriver.txt |  64 
 Platform/Marvell/Documentation/Drivers/SpiDriver.txt | 116 --
 Platform/Marvell/Documentation/PortingGuide/ComPhy.txt   |  45 ---
 Platform/Marvell/Documentation/PortingGuide/I2c.txt  |  20 --
 Platform/Marvell/Documentation/PortingGuide/Mdio.txt |   7 -
 Platform/Marvell/Documentation/PortingGuide/Mpp.txt  |  48 ---
 Platform/Marvell/Documentation/PortingGuide/PciEmulation.txt |  31 --
 Platform/Marvell/Documentation/PortingGuide/Phy.txt  |  45 ---
 Platform/Marvell/Documentation/PortingGuide/Pp2.txt  |  35 --
 Platform/Marvell/Documentation/PortingGuide/Reset.txt|   7 -
 Platform/Marvell/Documentation/PortingGuide/Spi.txt  |  16 -
 Platform/Marvell/Documentation/PortingGuide/SpiFlash.txt |  23 --
 Platform/Marvell/Documentation/PortingGuide/Utmi.txt |  35 --
 Silicon/Marvell/Documentation/Drivers/EepromDriver.txt   |  96 +
 Silicon/Marvell/Documentation/Drivers/I2cDriver.txt  |  64 
 Silicon/Marvell/Documentation/Drivers/SpiDriver.txt  | 116 ++
 Silicon/Marvell/Documentation/PortingGuide.txt   | 373 

 18 files changed, 649 insertions(+), 588 deletions(-)

diff --git a/Platform/Marvell/Documentation/Drivers/EepromDriver.txt 
b/Platform/Marvell/Documentation/Drivers/EepromDriver.txt
deleted file mode 100644
index d3b3b9f..000
--- a/Platform/Marvell/Documentation/Drivers/EepromDriver.txt
+++ /dev/null
@@ -1,96 +0,0 @@
-1. Introduction

-**MvEeprom** driver creates MARVELL_EEPROM_PROTOCOL, which
-+is used for managing eeprom.
-
-2. MvEeprom driver design
--
-Every I2C device driver should implement EFI_DRIVER_BINDING_PROTOCOL and
-consume EFI_I2C_IO_PROTOCOL for transactions on I2C bus. MvEeprom driver
-additionally implements MARVELL_EEPROM_PROTOCOL.
-
-  2.1 EFI_DRIVER_BINDING_PROTOCOL
-  ---
-  Driver Binding protocol is extensively covered in UEFI documentation, as
-  it is not specific to I2C stack. The only difference is that Supported()
-  function should check if EFI_I2C_IO_PROTOCOL provides valid EFI_GUID and
-  DeviceIndex values.
-  Excerpt from MvEepromSupported():
-
-Status = gBS->OpenProtocol (
-ControllerHandle,
-,
-(VOID **) ,
-gImageHandle,
-ControllerHandle,
-EFI_OPEN_PROTOCOL_BY_DRIVER
-);
-if (EFI_ERROR(Status)) {
-  return EFI_UNSUPPORTED;
-}
-
-/* get EEPROM devices' addresses from PCD */
-EepromAddresses = PcdGetPtr (PcdEepromI2cAddresses);
-if (EepromAddresses == 0) {
-  Status = EFI_UNSUPPORTED;
-  goto out;
-}
-
-Status = EFI_UNSUPPORTED;
-for (i = 0; EepromAddresses[i] != '\0'; i++) {
-  /* I2C guid must fit and valid DeviceIndex must be provided */
-  if (CompareGuid(TmpI2cIo->DeviceGuid, ) &&
-  TmpI2cIo->DeviceIndex == EepromAddresses[i]) {
-DEBUG((DEBUG_INFO, "A8kEepromSupported: attached to EEPROM device\n"));
-Status = EFI_SUCCESS;
-break;
-  }
-}
-
-  2.2 EFI_I2C_IO_PROTOCOL
-  ---
-  This protocol is provided by generic I2C stack. Multiple drivers can use IO
-  protocol at once, as queueing is implemented.
-
-  QueueRequest is a routine that queues an I2C transaction to the I2C
-  controller for execution on the I2C bus.
-
-  2.3 MARVELL_EEPROM_PROTOCOL
-  ---
-typedef struct _MARVELL_EEPROM_PROTOCOL MARVELL_EEPROM_PROTOCOL;
-
-#define EEPROM_READ   0x1
-#define EEPROM_WRITE  0x0
-typedef
-EFI_STATUS
-(EFIAPI *EFI_EEPROM_TRANSFER) (
-  IN CONST MARVELL_EEPROM_PROTOCOL  *This,
-  IN UINT16 Address,
-  IN UINT32 Length,
-  IN UINT8  *Buffer,
-  IN UINT8  Operation
-  );
-
-struct _MARVELL_EEPROM_PROTOCOL {
-  EFI_EEPROM_TRANSFER Transfer;
-  UINT8 Identifier;
-};
-
-3. Adding new I2C slave device drivers
---
-In order to support I2C slave device other than EEPROM, new driver should
-be created. Required steps follow.
-
-  1. Create driver

[edk2] [platforms: PATCH 03/10] Marvell/Drivers: MvSpiDxe: Fix write bug

2017-09-24 Thread Marcin Wojtas

From: Joe Zhou 

This patch prevents possible NULL pointer dereference
during SPI transfers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Joe Zhou 
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
 Platform/Marvell/Drivers/Spi/MvSpiDxe.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c 
b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
index 0c6b624..29e1379 100755
--- a/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
+++ b/Platform/Marvell/Drivers/Spi/MvSpiDxe.c
@@ -226,9 +226,8 @@ MvSpiTransfer (
 // Wait for memory ready
 for (Iterator = 0; Iterator < SPI_TIMEOUT; Iterator++) {
   if (MmioRead32 (SpiRegBase + SPI_INT_CAUSE_REG)) {
-*DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
-
 if (DataInPtr != NULL) {
+  *DataInPtr = MmioRead32 (SpiRegBase + SPI_DATA_IN_REG);
   DataInPtr++;
 }
 if (DataOutPtr != NULL) {
-- 
1.8.3.1

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

Re: [edk2] [PATCH v2 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing

2017-09-24 Thread Wang, Jian J

You're right that there's no such need. I just saw that this driver is loaded
before EndOfDxe but missed the fact that it's actually started after that.
So BIT7 of PcdNullPointerDetectionPropertyMask is enough.

And thanks a lot for other feedbacks in another emails, especially for the 
catching of potential attributes overridden issue, which also exists in other 
part of code. 

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, September 22, 2017 11:29 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric ; Yao,
> Jiewen ; Kinney, Michael D
> ; Justen, Jordan L ;
> Wolman, Ayellet 
> Subject: Re: [PATCH v2 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer
> detection during VBE SHIM installing
> 
> On 09/22/17 13:50, Laszlo Ersek wrote:
> > This patch looks great to me, I would like to request a few small
> > updates:
> >
> > On 09/21/17 07:20, Jian J Wang wrote:
> >> QemuVideoDxe driver will install VBE SHIM into page 0. If NULL pointer
> >
> > (1) please replace the word "install" with "link".
> >
> > The VBE Shim is technically installed into the "real-mode" C segment,
> > only the int 0x10 vector lives in page 0.
> >
> >> detection is enabled, this driver will fail to load. NULL pointer detection
> >> bypassing code is added to prevent such problem during boot.
> >>
> >> Please note that Windows 7 will try to access VBE SHIM during boot if it's
> >> installed, and then cause boot failure. This can be fixed by setting BIT7
> >> of PcdNullPointerDetectionPropertyMask to disable NULL pointer detection
> >> after EndOfDxe. As far as we know, there's no other OSs has such issue.
> >
> > This is not a request, just a comment: I verified the default value in
> > the .dec, and I see it is 0. So there's no need to post an additional
> > patch for the OVMF DSC files, in order to set BIT7.
> 
> Actually, let me take a step back, and re-think the necessity of all
> this work for QemuVideoDxe!
> 
> The facts are:
> 
> (1) The *only* purpose of the VBE Shim is to allow Windows 7 to boot in
> pure UEFI mode (i.e. without a CSM).
> 
> (2) If I understand correctly, you guys have verified that Windows 7
> cannot boot with the page0 protection enabled, *regardless* of what we
> do in QemuVideoDxe. Can you confirm this please?
> 
> With the above in mind, let's consider the effects of the
> "PcdNullPointerDetectionPropertyMask" bits:
> 
> * BIT0 clear:
>   - The page0 protection is completely disabled.
>   - This patch does nothing, in effect.
>   - The VBE Shim works.
>   - Windows 7 boots.
> 
> * BIT0 set, BIT7 also set:
>   - The page0 protection is disabled in the DXE core at the end of DXE.
>   - This patch does nothing, in effect.
>   - The VBE Shim works, because it is a UEFI driver, and it connects its
> devices (and installs the shim) after End-of-Dxe, at which point
> page0 protection is no longer in effect.
>   - Windows 7 boots fine, again because it is loaded after End-of-Dxe.
> 
> * BIT0 set, BIT7 clear:
>   - The page0 protection is never disabled until the OS (loader)
> installs its own page tables.
>   - This patch enables the VBE Shim to work, by temporarily disabling
> page0 protection.
>   - However, Windows 7 will fail to boot nonetheless, because it cannot
> cope with page0 protection. (This is fact (2).)
> 
> Now, if you consider fact (1) as well: given that Windows 7 cannot boot
> with page0 protection enabled *anyway*, why mess with the VBE Shim at
> all?
> 
> How about the following patch instead:
> 
> > diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c
> b/OvmfPkg/QemuVideoDxe/VbeShim.c
> > index e45a08e8873f..8ba5522cde3c 100644
> > --- a/OvmfPkg/QemuVideoDxe/VbeShim.c
> > +++ b/OvmfPkg/QemuVideoDxe/VbeShim.c
> > @@ -75,6 +75,20 @@ InstallVbeShim (
> >UINTNPrinted;
> >VBE_MODE_INFO*VbeModeInfo;
> >
> > +  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == 
> > BIT0)
> {
> > +DEBUG ((
> > +  DEBUG_WARN,
> > +  "%a: page 0 protected, not installing VBE shim\n",
> > +  __FUNCTION__
> > +  ));
> > +DEBUG ((
> > +  DEBUG_WARN,
> > +  "%a: page 0 protection prevents Windows 7 from booting anyway\n",
> > +  __FUNCTION__
> > +  ));
> > +return;
> > +  }
> > +
> >Segment0 = 0x0;
> >SegmentC = 0xC;
> >SegmentF = 0xF;
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] SNP transmit print

2017-09-24 Thread Fu, Siyuan

Hi, Meenakshi

You are correct that it's a bug in SNP driver to print error information for 
successfully transmitted packet. Please create a Bugzilla ticket for this issue 
and we will follow it. Thanks.


BestRegards
Fu Siyuan


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Udit 
Kumar
Sent: Friday, September 22, 2017 7:04 PM
To: Meenakshi Aggarwal ; edk2-devel@lists.01.org
Cc: Tian, Feng ; Zeng, Star 
Subject: Re: [edk2] SNP transmit print

Thanks Meenakshi for digging a bit more 

If we have use like below 
Library/DxeNetLib/DxeNetLib.c
Status = Snp->Transmit (Snp, 0, Length, Packet, NULL, NULL, NULL);
if ((Status != EFI_SUCCESS) && (Status != EFI_NOT_READY)) {
  Status = EFI_DEVICE_ERROR;
  break;
}
Then, we can simple put print when Status is EFI_DEVICE_ERROR;

But with below We are retrying once ☹ ,  What I think if first time if you got 
error 
EFI_NOT_READY, then after MnpRecycleTxBuf we likely not to get this error. 
I am not 100% sure here. 

What I think, in MdeModulePkg/Universal/Network/SnpDxe/Transmit.c 
Have print only for EFI_NOT_READY. 
And in MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c, print error as well 
for second case. 
Hope this helps 

Thanks
Udit

> -Original Message-
> From: Meenakshi Aggarwal
> Sent: Friday, September 22, 2017 3:56 PM
> To: Udit Kumar ; edk2-devel@lists.01.org
> Cc: feng.t...@intel.com; star.z...@intel.com
> Subject: RE: SNP transmit print
> 
> Hi Udit,
> 
> Yes, I think we should print this message is Status is EFI_DEVICE_ERROR 
> because
> in case of EFI_NOT_READY, we are retrying:
> 
>   //
>   // Transmit the packet through SNP.
>   //
>   Status = Snp->Transmit (
>   Snp,
>   HeaderSize,
>   Length,
>   Packet,
>   TxData->SourceAddress,
>   TxData->DestinationAddress,
>   
>   );
>   if (Status == EFI_NOT_READY) {
> Status = MnpRecycleTxBuf (MnpDeviceData);
> if (EFI_ERROR (Status)) {
>   Token->Status = EFI_DEVICE_ERROR;
>   goto SIGNAL_TOKEN;
> }
> 
> DEBUG ((EFI_D_ERROR, "Snp->Transmit  retry\n"));
> Status = Snp->Transmit (
> Snp,
> HeaderSize,
> Length,
> Packet,
> TxData->SourceAddress,
> TxData->DestinationAddress,
> 
> );
>   }
> 
> (MdeModulePkg/Universal/Network/MnpDxe/MnpIo.c)
> 
> 
> Please suggest if we can remove this print in case of BUSY status.
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Udit Kumar
> > Sent: Thursday, September 21, 2017 5:21 PM
> > To: Meenakshi Aggarwal ; edk2-
> > de...@lists.01.org
> > Subject: RE: SNP transmit print
> >
> > I think these error prints should be check against Status
> >
> > Regards
> > Udit
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Meenakshi Aggarwal
> > > Sent: Thursday, September 21, 2017 3:36 PM
> > > To: edk2-devel@lists.01.org
> > > Subject: [edk2] SNP transmit print
> > >
> > > Hi,
> > >
> > >
> > > While performing tftp using PCI interface, below message is coming
> > > continuously:
> > >
> > > Snp->undi.transmit()  8000h:4h
> > >
> > > Snp->undi.transmit()  8000h:4h
> > > [==> ]   34812 Kb
> > > Snp->undi.transmit()  8000h:4h
> > >
> > > Snp->undi.transmit()  8000h:4h
> > >
> > > Snp->undi.transmit()  8000h:4h
> > >
> > > Snp->undi.transmit()  8000h:4h
> > >
> > >
> > > It is coming from file
> > "MdeModulePkg/Universal/Network/SnpDxe/Transmit.c"
> > >
> > >   DEBUG (
> > > (EFI_D_ERROR,
> > > "\nSnp->undi.transmit()  %xh:%xh\n",
> > > Snp->Cdb.StatFlags,
> > > Snp->Cdb.StatCode)
> > > );
> > >
> > >
> > > I want to know if it is really an error message because tftp and
> > > ping are working perfectly, but this error message is coming.
> > >
> > >
> > > Thanks,
> > > Meenakshi
> > >
> > >
> > >
> > > ___
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/2] UDF partition driver fix

2017-09-24 Thread Ni, Ruiyu

Reviewed-by: Ruiyu Ni 

-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com] 
Sent: Saturday, September 23, 2017 2:12 AM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Kinney, Michael D 
; Gao, Liming ; Laszlo Ersek 
; Ni, Ruiyu ; Zeng, Star 
; Yao, Jiewen 
Subject: [PATCH v4 0/2] UDF partition driver fix

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

Hi,

This patchset fixes a bug in Partition driver that created UDF logical 
partitions by using entire block device space and thus polluting protocol 
database with broken handles.

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 :-)

v2->v3:
 - Followed Ruiyu's suggestions to improve code and add additional
   checks for ensuring a valid UDF file system and supported by current
   EDK2 UDF file system implementation. Also run Ecc.py to make sure the
   files I touched did not break EDK2 C Coding Style, as well as
   PatchCheck.py for mal-formed patches.

v3->v4:
 - Change 2/2's title as suggested by Star.
 - Remove UDF_TAG_ID() and refactor out UDF_ENTITY_ID structure as
   suggested by Ruiyu.
 - Tested build with VS2015 toolchain.

I've had a chance to test these changes with my 32GiB USB stick and formatted 
it on Windows 10 with `format` command. The UDF revisions I tested (by 
specifying it with "/R:revision") were 1.02, 1.50, 2.00,
2.01 (default) and 2.50. They all worked except the 2.50 revision which adds a 
Type 2 (Metadata) Partition and it's not supported by current
EDK2 UDF implementation -- which handles only Type 1 (Physical) Partitions. The 
UDF 2.60 revision I tested with the usual `sudo mkudffs -b 512 --media-type=hd 
/dev/sdX` command in Linux.

Remember, the *officially* supported revision is 2.60, however all revisions 
use the same volume structures as defined by ECMA 167 specification, and they 
usually differ from each other by means of new optional features, so that's why 
all those revisions worked with this implementation.

Well, at least this what I understood when looking at the specifications. 
Please correct me if I'm wrong.

Please, test building these changes in toolchains other than GCC and make sure 
they don't break the world :-)

Thanks!
Paulo

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

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 (2):
  MdePkg: Add UDF volume structure definitions
  MdeModulePkg/UDF: Fix creation of UDF logical partition

 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 366 ++--
 MdeModulePkg/Universal/Disk/UdfDxe/File.c |  16 +-
 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 627 

 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c  |   7 -
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h  | 158 ++---
 MdePkg/Include/IndustryStandard/Udf.h |  97 ++-
 6 files changed, 698 insertions(+), 573 deletions(-)

--
2.13.3.windows.1

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

Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux

Re: [edk2] SNP transmit print

Re: [edk2] [RFC 0/6] Create central repository for boilerplate configuration

Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr deref

Re: [edk2] Repeated condition check

Re: [edk2] Repeated condition check

Re: [edk2] Repeated condition check

Re: [edk2] Repeated condition check

Re: [edk2] [PATCH] BaseTools/Conf: Support LLVM39 and LLVM40 in CLANG38 toolchain

[edk2] [Patch] FDF Spec: Per PI 1.6 to extend FFS alignment to 16M

[edk2] [Patch] Build Spec: Build report to display every module's build time

[edk2] [PATCH] MdeModulePkg/DxeCore: Add check to ensure no possible NULL ptr deref

[edk2] [platforms: PATCH 06/10] Marvell/Applications/SpiTool: Fix bug in error test

[edk2] [platforms: PATCH 09/10] Marvell/Drivers: MvSpiFlash: Fix usage of erase size parameter

[edk2] [platforms: PATCH 04/10] Marvell/Applications/SpiTool: Enable configurable CS and SCLK mode

[edk2] [platforms: PATCH 07/10] Marvell/Applications/FirmwareUpdate: Fix 32-bit issues

[edk2] [platforms: PATCH 10/10] Marvell/Drivers: MvSpiFlash: Minor style fix

[edk2] [platforms: PATCH 02/10] Marvell/Drivers: MvSpiDxe: Log and return correct error

[edk2] [platforms: PATCH 08/10] Marvell/Applications/SpiTool: Fix 32-bit issues

[edk2] [platforms: PATCH 05/10] Platform/Marvell/Armada70x0: set CS and SCLK Mode for SPI flash

[edk2] [platforms: PATCH 00/10] Armada 70x0/80x0 SPI improvements

[edk2] [platforms: PATCH 01/10] Silicon/Marvell: Refactor Documentation

[edk2] [platforms: PATCH 03/10] Marvell/Drivers: MvSpiDxe: Fix write bug

Re: [edk2] [PATCH v2 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing

Re: [edk2] SNP transmit print

Re: [edk2] [PATCH v4 0/2] UDF partition driver fix

26 matches

Site Navigation

Mail list logo

Footer information