Re: [edk2] [MdeModulePkg/PeiCore] How is Data being NULL on entry ensured?

2016-08-12 Thread Marvin H?user
Hey Andrew,

Thank you very much! I suppose I was misguided by the ENTRY_POINT property 
being PeiCore in the build file.

Regards,
Marvin.

> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Saturday, August 13, 2016 4:47 AM
> To: Marvin H?user 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [MdeModulePkg/PeiCore] How is Data being NULL on
> entry ensured?
> 
> 
> > On Aug 12, 2016, at 7:01 PM, Marvin H?user
>  wrote:
> >
> > Dear list subscribers,
> >
> > I have just been looking around the PeiCore code and wondered, how it is
> ensured, that the third ("Data") argument of the entry point is NULL on the
> first run.
> > EFI_PEI_CORE_ENTRY_POINT only has two arguments and hence most SEC
> implementations, including MdeModulePkg/SecCore, are going to pass only
> the first two arguments to the entry point.
> > I'm aware that the code works and I have never seen an occasion of it
> failing or seeming to fail because of this design, though I wondered, how is 
> it
> assured, that the third argument, which is not part of the first call, is 
> being
> NULL on entry and not some leftover on the temporary stack/in the
> argument 3 register?
> >
> 
> Marvin,
> 
> The code you are looking for is in the entry point library.
> 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/PeiCoreE
> ntryPoint/PeiCoreEntryPoint.c#L59
> 
> **/
> VOID
> EFIAPI
> _ModuleEntryPoint(
>   IN CONST  EFI_SEC_PEI_HAND_OFF*SecCoreData,
>   IN CONST  EFI_PEI_PPI_DESCRIPTOR  *PpiList
> )
> {
>   ProcessModuleEntryPointList (SecCoreData, PpiList, NULL);
> 
>   //
>   // Should never return
>   //
>   ASSERT(FALSE);
>   CpuDeadLoop ();
> }
> 
> 
> ProcessModuleEntryPointList() call is auto-generated and it will cal the entry
> point listed in the PEI Core INF file. So that is why it is hard to grep for.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thank you for your time!
> >
> > Regards,
> > Marvin.
> > ___
> > 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] [MdeModulePkg/PeiCore] How is Data being NULL on entry ensured?

2016-08-12 Thread Andrew Fish

> On Aug 12, 2016, at 7:01 PM, Marvin H?user  wrote:
> 
> Dear list subscribers,
> 
> I have just been looking around the PeiCore code and wondered, how it is 
> ensured, that the third ("Data") argument of the entry point is NULL on the 
> first run.
> EFI_PEI_CORE_ENTRY_POINT only has two arguments and hence most SEC 
> implementations, including MdeModulePkg/SecCore, are going to pass only the 
> first two arguments to the entry point.
> I'm aware that the code works and I have never seen an occasion of it failing 
> or seeming to fail because of this design, though I wondered, how is it 
> assured, that the third argument, which is not part of the first call, is 
> being NULL on entry and not some leftover on the temporary stack/in the 
> argument 3 register?
> 

Marvin,

The code you are looking for is in the entry point library. 

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.c#L59

**/
VOID
EFIAPI 
_ModuleEntryPoint(
  IN CONST  EFI_SEC_PEI_HAND_OFF*SecCoreData,
  IN CONST  EFI_PEI_PPI_DESCRIPTOR  *PpiList
)
{
  ProcessModuleEntryPointList (SecCoreData, PpiList, NULL);
  
  //
  // Should never return
  //
  ASSERT(FALSE);
  CpuDeadLoop ();  
}


ProcessModuleEntryPointList() call is auto-generated and it will cal the entry 
point listed in the PEI Core INF file. So that is why it is hard to grep for. 

Thanks,

Andrew Fish

> Thank you for your time!
> 
> Regards,
> Marvin.
> ___
> 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] DHCP Automatic Configure at Driver Connect

2016-08-12 Thread Wu, Jiaxin
> Jiaxin,
> 
> > > The issue is not just a performance issue around DHCP timing.  Even
> > > with a static IP address set the fact that the network interface
> > > comes "up" at each boot is problematic because our requirement is
> > > only to enable the network interface when directed to by an application.
> >
> > In my memory, we only talked about the performance issue before. Here,
> > I understand your requirement: 1) The network interface should be in
> > *un-
> > configured* status at each boot time until the third part
> > configuration; 2) the policy setting should be ignored, right? I don't

This is what I want to confirm with you that my understanding is right? You 
complained the network interface comes "up" at each boot, what does  "comes up" 
exactly mean? So, I gave the above two guesses 1) and 2).


> > think it's reasonable. I know the old Ip4Config did as you said here,
> > but currently, the IPv4 default policy concept is new enrolled by
> > Ip4Config2 protocol, the device policy should be in one state no matter
> static or DHCP.
> 
> Sorry I'm having trouble understanding what you meant - can you rephrase
> or expand this?  Are you saying that it is not reasonable to want behavior
> consistent with the original Ip4Config behavior?  This seems like something

Of course not. I mean if my guessing 2) is correct, it's not reasonable because 
UEFI 2.6 spec only defined Static/DHCP policy. The policy should be in one of 
them. I just recalled the old Ip4Config behavior: no policy assigned default 
(if I remember correctly).  So, don't misunderstand my truly means. It's 
unrelated to the *behavior consistency*.


> that should be doable - in my experiment suppressing the configuration in
> Ip4Config2OnDhcp4SbInstalled does exactly this so it seems like some
> additional policy (automatic versus on-demand) could be defined to handle
> this.
> 
> > >
> > > Modifying the IP configuration dynamically to suppress the interface
> > > coming up at every boot is also problematic because it means we
> > > would need to store the IP address configuration in another NVRAM
> location.
> > > In other words, the combining of the IP address settings storage
> > > *and* the policy of whether to configure at boot or wait until
> > > explicitly requested is problematic - we really would like to
> > > control these settings independently.  Is that possible within the
> > > scope of the spec?  Could we just have a PCD that suppresses the
> > > automatic configure at
> > boot under any circumstance?
> >
> > Actually, if you want to recover the Ipv4 setting to the
> > *un-configured* status (Note here: policy should be always set,
> > *un-configured* means no IP address configuration), Ip4Config2 provide
> > the ability to change the default policy. As git commit version SHA-1:
> > 7648748e99eeeadec38fda7568adb260c4acc861 described, "This update let
> > the other platform drivers have chance to change the default config
> > data by consume Ip4Config2Protocol. "  So, you don't need to set
> > another NVRAM location to do that, that means additional DXE_DRIVER
> > can add in your platform ahead to change the default config data. The
> > only operation in this DXE_DRIVER is to register a notify for
> > Ip4Config2 protocol to change the default policy. That's also the
> > reason why we don't want to enroll PCD to change it, you know two
> interface to do the same thing is not a good idea.
> 
> Let's say I have configured a static IP of 192.168.0.1.  But since I don't 
> want
> the network interface to automatically configure at boot I think you are
> saying that I should implement a platform policy driver that uses Ip4Config2
> to set an *unconfigured* state.  But there is no definition for an
> "unconfigured" state in UEFI 2.6 so how would one do this?  Furthermore,
> wouldn't this effectively delete the previous static IP data?  Maybe I just
> don't understand what you're describing - perhaps some pseudocode for
> your proposal that accomplishes the use case I'm describing would help.
> 

First, *based on the current UEFI Spec (only static / dhcp policy)*, I want to 
highlight my understanding of *un-configured* status you mentioned: policy 
should be always set, *un-configured* means *no IP address configuration*.

If we want to implement such a *un-configured* state, one *DXE_DRIVER* can be 
used with Ip4Config2 protocol. Detailed see below pseudocode:

First,  register a notify for Ip4Config2 protocol in your Dxe Driver EntryPoint:
EfiCreateProtocolNotifyEvent (
,
TPL_CALLBACK,
Ip4Config2InstalledCallback,
NULL,

);

Then, In your Ip4Config2InstalledCallback() function, you can change the 
default policy for the current boot:
 Ip4Config2InstalledCallback () {
 gBS->LocateProtocol (
  , 
  Registration, 
  (VOID **) 
  );

 //
 // Change the policy to Ip4Config2PolicyDhcp to clean the 

[edk2] [MdeModulePkg/PeiCore] How is Data being NULL on entry ensured?

2016-08-12 Thread Marvin H?user
Dear list subscribers,

I have just been looking around the PeiCore code and wondered, how it is 
ensured, that the third ("Data") argument of the entry point is NULL on the 
first run.
EFI_PEI_CORE_ENTRY_POINT only has two arguments and hence most SEC 
implementations, including MdeModulePkg/SecCore, are going to pass only the 
first two arguments to the entry point.
I'm aware that the code works and I have never seen an occasion of it failing 
or seeming to fail because of this design, though I wondered, how is it 
assured, that the third argument, which is not part of the first call, is being 
NULL on entry and not some leftover on the temporary stack/in the argument 3 
register?

Thank you for your time!

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


[edk2] [PATCH v1 1/1] MdeModulePkg/PeiCore: Fix ConverSinglePpiPointer () typo.

2016-08-12 Thread Marvin Häuser
Rename ConverSinglePpiPointer () to ConvertSinglePpiPointer ().

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marvin Haeuser 
---
 MdeModulePkg/Core/Pei/Ppi/Ppi.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Ppi/Ppi.c b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
index 706e835a7091..db6eded6d6ed 100644
--- a/MdeModulePkg/Core/Pei/Ppi/Ppi.c
+++ b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
@@ -48,7 +48,7 @@ InitializePpiServices (
 
 **/
 VOID
-ConverSinglePpiPointer (
+ConvertSinglePpiPointer (
   IN PEI_PPI_LIST_POINTERS *PpiPointer,
   IN UINTN TempBottom,
   IN UINTN TempTop,
@@ -124,7 +124,7 @@ ConvertPpiPointers (
   //
   // Convert PPI pointer in old Heap
   //
-  ConverSinglePpiPointer (
+  ConvertSinglePpiPointer (
 >PpiData.PpiListPtrs[Index],
 (UINTN)SecCoreData->PeiTemporaryRamBase,
 (UINTN)SecCoreData->PeiTemporaryRamBase + 
SecCoreData->PeiTemporaryRamSize,
@@ -135,7 +135,7 @@ ConvertPpiPointers (
   //
   // Convert PPI pointer in old Stack
   //
-  ConverSinglePpiPointer (
+  ConvertSinglePpiPointer (
 >PpiData.PpiListPtrs[Index],
 (UINTN)SecCoreData->StackBase,
 (UINTN)SecCoreData->StackBase + SecCoreData->StackSize,
@@ -151,7 +151,7 @@ ConvertPpiPointers (
   continue;
 }
 
-ConverSinglePpiPointer (
+ConvertSinglePpiPointer (
   >PpiData.PpiListPtrs[Index],
   (UINTN)PrivateData->HoleData[IndexHole].Base,
   (UINTN)PrivateData->HoleData[IndexHole].Base + 
PrivateData->HoleData[IndexHole].Size,
-- 
2.9.0.windows.1

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


Re: [edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core by grabbing memory from PeiTemporaryRamBase?

2016-08-12 Thread Marvin H?user
Hello Andrew,

Unfortunately I cannot test anything right now and I don't have a whole lot of 
knowledge in this area, though I might have a hint for you.

While PpiList is equal to the original TempRam base, the TempRam based passed 
to PEI is equal to the original TempRam base + the size of the PpiList, hence 
PpiList is smaller than the base address passed to PEI. The PpiList is then 
installed via the PeiServicesInstallPpi () function:

call: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L386
implementation: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Ppi/Ppi.c#L183

The list is then added to PpiData.PpiListPtrs.

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Ppi/Ppi.c#L229

I am not sure at which point of time you are experiencing the crash, but after 
permanent memory is available, ConvertPpiPointers () is called, which then 
calls ConverSinglePpiPointer () for old heap, old stack and old hole (I'm 
afraid I do not know what TempRam Hole is and if it is related).

ConvertPpiPointers () call: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L237
Old Heap ConverSinglePpiPointer () call: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Ppi/Ppi.c#L127

The call for the old heap conversion passes the TempRam base, which has been 
incremented as we know, and thus the comparison to TempBottom will fail, as 
TempBottom is PeiTemporaryRamBase, which is larger than PpiList, which is one 
of the items in PpiListPtrs, which is the object of the conversion.

comparison to TempBottom: 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Ppi/Ppi.c#L60

As the pointer to the PpiList passed by SecCore is probably not converted as 
detailed above, I assume something post-mem attempts to access this former 
PpiList by the old temporary RAM address and that somehow causes trouble; I 
assume the SEC PpiList being part of the PEI memory is an assumption made by 
the person who wrote this code. I'm not sure about why it crashes, as I do not 
know the entire PEI control flow, though I hope this can help you in some way.

Please excuse me if I have made a mistake in understanding the referenced code 
and wasted your time.

Regards,
Marvin.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Saturday, August 13, 2016 1:25 AM
> To: edk2-devel 
> Subject: [edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the PEI
> Core by grabbing memory from PeiTemporaryRamBase?
> 
> I grabbed some memory between SEC and the PEI Core by adjusting
> SecCoreData-> PeiTemporaryRamBase and SecCoreData->
> PeiTemporaryRamSize.
> 
> When looking at the code I don't really understand the logic of the algorithm?
> So maybe I'm doing something wrong.
> 
> This adjustment does not seem right to me?
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Dispatcher/Dispatcher.c#L768
>   //
>   // Heap Offset
>   //
>   BaseOfNewHeap = TopOfNewStack;
>   if (BaseOfNewHeap >= (UINTN)SecCoreData->PeiTemporaryRamBase) {
> Private->HeapOffsetPositive = TRUE;
> Private->HeapOffset = (UINTN)(BaseOfNewHeap -
> (UINTN)SecCoreData->PeiTemporaryRamBase);
>   } else {
> Private->HeapOffsetPositive = FALSE;
> Private->HeapOffset = (UINTN)((UINTN)SecCoreData-
> >PeiTemporaryRamBase - BaseOfNewHeap);
>   }
> 
> 
> The above code seems to be making a very strange adjustment. I noticed the
> adjustment in my failing case was off by 0xC0 which is the amount of
> memory I carved out prior to entering the PEI Core.
> 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/
> Dispatcher/Dispatcher.c#L796
> 
>   //
>   // Temporary Ram Support PPI is provided by platform, it will copy
>   // temporary memory to permenent memory and do stack switching.
>   // After invoking Temporary Ram Support PPI, the following code's
>   // stack is in permanent memory.
>   //
>   TemporaryRamSupportPpi->TemporaryRamMigration (
> PeiServices,
> TemporaryRamBase,
> (EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack -
> TemporaryStackSize),
> TemporaryRamSize
> );
> 
> 
> And this is also a case in which the stack got bigger. But it seems to me the
> shift if really defined by TemporaryRamBase, TopOfNewStack, and
> TemporaryStackSize in this case.
> 
> The failure I hit was OldCoreData->Fv pointer was shifted so when the PPI
> was called the system crashed. Is this a bug in the
> gEfiTemporaryRamSupportPpiGuid path?
> 
> If I changed the HeadOffset algorithm my crash went away? Private-
> >HeapOffset = ((UINTN)TopOfNewStack - TemporaryStackSize) -
> 

[edk2] [MdeModulePkg][PeiCore] I seemed to have crashed the PEI Core by grabbing memory from PeiTemporaryRamBase?

2016-08-12 Thread Andrew Fish
I grabbed some memory between SEC and the PEI Core by adjusting SecCoreData-> 
PeiTemporaryRamBase and SecCoreData-> PeiTemporaryRamSize.

When looking at the code I don't really understand the logic of the algorithm? 
So maybe I'm doing something wrong. 

This adjustment does not seem right to me?
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L768
  //
  // Heap Offset
  //
  BaseOfNewHeap = TopOfNewStack;
  if (BaseOfNewHeap >= (UINTN)SecCoreData->PeiTemporaryRamBase) {
Private->HeapOffsetPositive = TRUE;
Private->HeapOffset = (UINTN)(BaseOfNewHeap - 
(UINTN)SecCoreData->PeiTemporaryRamBase);
  } else {
Private->HeapOffsetPositive = FALSE;
Private->HeapOffset = (UINTN)((UINTN)SecCoreData->PeiTemporaryRamBase - 
BaseOfNewHeap);
  }


The above code seems to be making a very strange adjustment. I noticed the 
adjustment in my failing case was off by 0xC0 which is the amount of memory I 
carved out prior to entering the PEI Core. 

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L796

  //
  // Temporary Ram Support PPI is provided by platform, it will copy 
  // temporary memory to permenent memory and do stack switching.
  // After invoking Temporary Ram Support PPI, the following code's 
  // stack is in permanent memory.
  //
  TemporaryRamSupportPpi->TemporaryRamMigration (
PeiServices,
TemporaryRamBase,
(EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack - 
TemporaryStackSize),
TemporaryRamSize
);


And this is also a case in which the stack got bigger. But it seems to me the 
shift if really defined by TemporaryRamBase, TopOfNewStack, and 
TemporaryStackSize in this case. 

The failure I hit was OldCoreData->Fv pointer was shifted so when the PPI was 
called the system crashed. Is this a bug in the gEfiTemporaryRamSupportPpiGuid 
path?

If I changed the HeadOffset algorithm my crash went away? Private->HeapOffset = 
((UINTN)TopOfNewStack - TemporaryStackSize) - TemporaryRamBase;

Thanks,

Andrew Fish

PS My failure case was the EmulatorPkg. I've not had a chance to verify this 
failure in the open source yet, but I'm guessing reversing this #if will make 
it happen.


https://github.com/tianocore/edk2/blob/master/EmulatorPkg/Sec/Sec.c#L107

#if 0
  // Tell the PEI Core to not use our buffer in temp RAM
  SecPpiList = (EFI_PEI_PPI_DESCRIPTOR *)SecCoreData->PeiTemporaryRamBase;
  SecCoreData->PeiTemporaryRamBase = (VOID 
*)((UINTN)SecCoreData->PeiTemporaryRamBase + SecReseveredMemorySize);
  SecCoreData->PeiTemporaryRamSize -= SecReseveredMemorySize;
#else
  {
//
// When I subtrack from SecCoreData->PeiTemporaryRamBase PEI Core crashes? 
Either there is a bug
// or I don't understand temp RAM correctly?
//
EFI_PEI_PPI_DESCRIPTORPpiArray[10];

SecPpiList = [0];
ASSERT (sizeof (PpiArray) >= SecReseveredMemorySize);
  }
#endif




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


Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency

2016-08-12 Thread Kinney, Michael D
Paolo,

That is a good idea to find factors of ACPI_TIMER_FREQUENCY 3579545 Hz
and find a value that is close to 100 uS for calibration.  The pair you
found 343 * 10436 = 3579548 Hz.  If we look at frequencies lower and 
higher than ACPI_TIMER_FREQUENCY, the value 3579543 has many more factors
and is only 2 Hz from the target.

Number Factors
===
35795421, 2, 67, 134, 26713, 53426, 1789771, 3579542
 
35795431, 3, 9, 11, 19, 33, 57, 99, 121, 171, 173, 209, 363, 
   519, 627, 1089, 1557, 1881, 1903, 2299, 3287, 5709, 
   6897, 9861, 17127, 20691, 20933, 29583, 36157, 62799, 
   108471, 188397, 325413, 397727, 1193181, 3579543

35795441, 2, 4, 8, 447443, 894886, 1789772, 3579544

35795451, 5, 715909, 3579545

35795461, 2, 3, 6, 41, 82, 123, 246, 14551, 29102, 43653, 
   87306, 596591, 1193182, 1789773, 3579546

35795471, 3579547

35795481, 2, 4, 7, 14, 28, 49, 98, 196, 343, 686, 1372, 2609, 
   5218, 10436, 18263, 36526, 73052, 127841, 255682, 511364, 
   894887, 1789774, 3579548


So we could choose from the following pairs for calibration:

209 * 17127209 ticks is  58.3 uS calibration time
363 * 9861 363 ticks is 101.4 uS calibration time
519 * 6897 519 ticks is 145.0 uS calibration time
627 * 5709 627 ticks is 175.2 uS calibration time

I would recommend the pair 363 * 9861 which is closest to the
Current 100 uS calibration time.

Another accuracy improvement on the current algorithm is to
Wait for the ACPI timer value to change before capturing the
Initial TSC value, so only whole ticks are counted.  The current 
algorithm could start the beginning, middle, or end of an ACPI 
counter tick.

Here is an updated measurement algorithm.

  //
  // Wait for ACPI timer to start next count
  //
  Ticks = IoRead32 (TimerAddr);
  while (Ticks == IoRead32 (TimerAddr)) {
CpuPause();
  }

  //
  // Immediately capture start TSC value
  //
  StartTSC = AsmReadTsc (); 

  //
  // Compute the number of ticks to wait to measure TSC frequency.
  // Use 363 * 9861 = 3579543 Hz which is within 2 Hz of 
  // ACPI_TIMER_FREQUENCY.  363 counts is a calibration time of 
  // 101.4 uS.
  // Subtract 1 because the calibration loop waits one extra count.
  //
  Ticks = IoRead32 (TimerAddr) + 363 - 1;

  //
  // Wait until the ACPI timer has counted the number of calibration ticks.
  // Timer wrap-arounds are handled correctly by this function.
  // When the current ACPI timer value is greater than 'Ticks', 
  // the while loop will exit.
  //
  while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
CpuPause();
  }

  //
  // Immediately capture end TSC value
  //
  EndTSC = AsmReadTsc ();   // TSC 
value 363 ticks later

  TscFrequency = MultU64x32 (
   (EndTSC - StartTSC), // Total 
number of TSC counts
   9861
   );

We would have to run some experiments to see if this further improves the 
accuracy and consistency of the measurements.

Mike

> -Original Message-
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Sent: Friday, August 12, 2016 1:31 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> 
> Subject: Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC
> Frequency
> 
> 
> 
> On 11/08/2016 04:37, Star Zeng wrote:
> > Minimize the code overhead between the two TSC reads by adding
> > new internal API to calculate TSC Frequency instead of reusing
> > MicroSecondDelay ().
> >
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Paul A Lohr 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng 
> > ---
> >  PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 
> > +-
> >  .../Library/AcpiTimerLib/BaseAcpiTimerLib.c| 33 -
> >  .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 
> >  3 files changed, 99 insertions(+), 21 deletions(-)
> >
> > diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > index 806a4f7ce24c..e6fea231123d 100644
> > --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >ACPI Timer implements one instance of Timer Library.
> >
> > -  Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
> >This program and the accompanying materials
> >are licensed and made available under the terms and conditions of the BSD
> License
> 

Re: [edk2] [RFC] MdeModulePkg: ARM/AARCH64 requirements added to .dsc

2016-08-12 Thread Andrew Fish

> On Aug 12, 2016, at 10:11 AM, Leif Lindholm  wrote:
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm 
> ---
> 
> This is not a proposed change, but rather a demonstration:
> When trying to build HelloWorld standalone for ARM/AARCH64,
> with a command line like
> build -a AARCH64 -t GCC5 -p MdeModulePkg/MdeModulePkg.dsc \
>   -m MdeModulePkg/Application/HelloWorld/HelloWorld.inf
> this fails with
> 
> /work/git/edk2/MdeModulePkg/MdeModulePkg.dsc(...): error 4000: Instance of 
> library class [ArmMmuLib] is not found
>   in [/work/git/edk2/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf] [AARCH64]
>   consumed by module 
> [/work/git/edk2/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf]
> 
> Adding the below stanzas resolve the issue, but looking
> into why other architectures have no issues with this,
> I spot a lot of low-level MMU tweakery directly under
> DxeIplPeim.
> 
> Would this not be better handled through some standardised
> mechanism?
> 

Leif,

The DXE IPL containing the paging code predated the edk2. On x86 the common 
boot flow is IA32 PEI and X64 DXE. To turn on X64 (long mode) paging must be 
enabled. So the paging code is here since this is the location than needs to 
transition to long mode. I'm guessing this code did not get turned into a 
library "back in the day" as it was the only place it was being used. 

Thanks,

Andrew Fish

> /
>Leif
> 
> MdeModulePkg/MdeModulePkg.dsc | 8 
> 1 file changed, 8 insertions(+)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> index 05120c7..e9c2866 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -24,6 +24,14 @@ [Defines]
>   BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
>   SKUID_IDENTIFIER   = DEFAULT
> 
> +[LibraryClasses.ARM, LibraryClasses.AARCH64]
> +  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +[LibraryClasses.ARM]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +[LibraryClasses.AARCH64]
> +  ArmLib|ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> +
> [LibraryClasses]
>   #
>   # Entry point
> -- 
> 2.1.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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


[edk2] [RFC] MdeModulePkg: ARM/AARCH64 requirements added to .dsc

2016-08-12 Thread Leif Lindholm
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm 
---

This is not a proposed change, but rather a demonstration:
When trying to build HelloWorld standalone for ARM/AARCH64,
with a command line like
build -a AARCH64 -t GCC5 -p MdeModulePkg/MdeModulePkg.dsc \
   -m MdeModulePkg/Application/HelloWorld/HelloWorld.inf
this fails with

/work/git/edk2/MdeModulePkg/MdeModulePkg.dsc(...): error 4000: Instance of 
library class [ArmMmuLib] is not found
in [/work/git/edk2/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf] [AARCH64]
consumed by module 
[/work/git/edk2/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf]

Adding the below stanzas resolve the issue, but looking
into why other architectures have no issues with this,
I spot a lot of low-level MMU tweakery directly under
DxeIplPeim.

Would this not be better handled through some standardised
mechanism?

/
Leif

 MdeModulePkg/MdeModulePkg.dsc | 8 
 1 file changed, 8 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 05120c7..e9c2866 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -24,6 +24,14 @@ [Defines]
   BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
   SKUID_IDENTIFIER   = DEFAULT
 
+[LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+[LibraryClasses.ARM]
+  ArmLib|ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.inf
+  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+[LibraryClasses.AARCH64]
+  ArmLib|ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
+
 [LibraryClasses]
   #
   # Entry point
-- 
2.1.4

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


Re: [edk2] Double or floating point on edk2 ?

2016-08-12 Thread Michael Zimmermann
sry for bringing up this ancient thread but I got a question about this:
shouldn't soft-float be 100% safe because it's a software implementations
and thus never throw exceptions?

Thanks
Michael

On Tue, Dec 8, 2015 at 6:28 PM, Shubha Ramani 
wrote:

> Thanks for your well-thought out and detailed answer !I guess the safe bet
> is "No". Stay away from floating point on EDK2.
> Shubha Shubha D. ramanishubharam...@gmail.com
> shubharam...@yahoo.com
>
>
> On Saturday, December 5, 2015 4:24 PM, Scott Duplichan <
> sc...@notabs.org> wrote:
>
>
>  Daryl McDaniel [mailto:edk2-li...@mc2research.org] wrote:
>
> ]Sent: Saturday, December 05, 2015 11:00 AM
> ]To: 'Shubha Ramani' ; edk2-de...@ml01.01.org
> ]Subject: Re: [edk2] Double or floating point on edk2 ?
> ]
> ]Hello,
> ]
> ]Doing floating point operations within the UEFI environment
> ]is not recommended.  There are many reasons, but the main one
> ]is that floating point exceptions are unlikely to be handled.
>
> I think the need to enable and trap floating point exceptions
> is rare. For most applications, the normal masked response
> works well. Here is Intel's official position:
>
>   "Exception-handling software is often difficult to write,
> and the masked responses have been tailored to deliver the
> most reasonable result for each condition. For the majority
> of applications, masking all exceptions yields satisfactory
> results with the least programming effort."
>
> ]The default FPU mode (rounding, precision, ...) may also not
> ]be what you would want.
>
> I think the default FPU mode meets the requirements of the C
> programming language. But if not, it could be changed.
>
> ]There is also a chance that one or more of the UEFI functions
> ]may destroy a floating point or vector register that you are using.
>
> Is this really true? I think the compiler will preserve the registers
> when needed for C code. ASM code could certainly destroy floating
> point registers. A problem would occur if SMI or the timer interrupt
> code were to use floating point registers. Could it be possible
> for timer callback code to use floating point and destroy registers
> of the interrupted application?
>
> ]That said, there is nothing that would prevent you from using
> ]floating point types and operations in your Shell application.
> ]If you do, it is your responsibility to ensure that the environment
> ]is properly set up for your needs.  The UEFI specification details
> ]what registers are preserved, or not.
> ]
> ]Using StdLib does not alleviate this responsibility.  The only thing
> ]StdLib does is ensure that the correct rounding, floor, and NAN modes
> ]are set in the FPU in order to conform to the ISO/IEC 9899 Addendum-1
> ](C95) specification.  The sole purpose of StdLib's floating point
> ]support is to facilitate porting existing C applications to the UEFI
> ]environment.
> ]
> ]Unfortunately, using floating point is entirely at your own risk.
>
> The good thing is the problem with unresolved external symbol __dtoui3
> is gone with the current EDK2. This problem used to occur when building
> AppPkg for IA32 using VS2013 or newer. It no longer occurs because of
> "BaseTools: Add /arch:IA32 option in VS2012 and VS2013". Though that
> change was made to prevent the compiler from generating the cmov
> instruction, it also has the side effect of turning off SSE2 math.
>
> Thanks,
> Scott
>
>
> ]Sincerely,
> ]Daryl McDaniel
>
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shubha Ramani
> > Sent: Friday, December 04, 2015 4:39 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] Double or floating point on edk2 ?
> >
> > I know this question has been answered before. Do I need to convert my
> application to a UEFI StdLib applicationto do double or
> > floating point math ? Can't do it in a regular Shell App ?
> > Please illuminate.
> > Thanks,
> > Shubha Shubha D. ramanishubharam...@gmail.com
> > shubharam...@yahoo.com
> > ___
> > 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
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] IP4 Config Troubles with DHCP

2016-08-12 Thread Cohen, Eugene
> Eugene,
> 
> I can reproduce the issue now. And root cause as below:
> 
> #1. Set policy to DHCP.
> #2. If DHCP process is not complete yet, then run one App to invoke the
> UDP4 Configure with "UseDefaultAddress = TRUE" (loop to keep calling
> Udp4->Configure until Ip4Mode.IsConfigured changes to TRUE)
> #3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to TRUE --
> -- failure here!!!
> 
> In step1, the policy will be set to DHCP, and then
> Ip4Config2OnPolicyChanged() will be called. In this function, "IpSb-
> >Reconfig" flag will be set to TRUE before Ip4StartAutoConfig() called. That
> means the original "IpSb->DefaultInterface" will be abandoned/freed once
> this DHCP process finished. Detailed see Ip4Config2SetDefaultAddr()
> function.
> 
> In step2, UDP4 Configure with "UseDefaultAddress = TRUE" is called, that
> means the default interface (IpSb->DefaultInterface) will be selected as
> current instance's interface. Detailed see Ip4ConfigProtocol() function.
> 
> In step3, When DHCP process finished, as I said in step1, the original "IpSb-
> >DefaultInterface" will be abandoned/freed because "IpSb->Reconfig" flag is
> true. Meanwhile, one new interface is assigned to "IpSb->DefaultInterface".
> This "IpSb->DefaultInterface" is different to the original one assigned to the
> UDP4 Configured instance. So, even DHCP process succeed, the up caller will
> never have the chance to get it's truly status.
> 
> I will send one patch to fix this issue later.
> 
> Thanks your reporting.
> 
> Best Regards!
> Jiaxin

Jiaxin, excellent to hear.  I'm glad to hear you've isolated the issue.  So the 
service binding instance we have in this case is orphaned which explains why 
destroying it and creating a new one resolves the issue.  Of course we'd be 
happy to test the patch as soon as it's available.

Thank you!

Eugene

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


Re: [edk2] DHCP Automatic Configure at Driver Connect

2016-08-12 Thread Cohen, Eugene
Jiaxin,

> > The issue is not just a performance issue around DHCP timing.  Even
> > with a static IP address set the fact that the network interface comes
> > "up" at each boot is problematic because our requirement is only to
> > enable the network interface when directed to by an application.
> 
> In my memory, we only talked about the performance issue before. Here, I
> understand your requirement: 1) The network interface should be in *un-
> configured* status at each boot time until the third part configuration; 2) 
> the
> policy setting should be ignored, right? I don't think it's reasonable. I know
> the old Ip4Config did as you said here, but currently, the IPv4 default policy
> concept is new enrolled by Ip4Config2 protocol, the device policy should be
> in one state no matter static or DHCP.

Sorry I'm having trouble understanding what you meant - can you rephrase or 
expand this?  Are you saying that it is not reasonable to want behavior 
consistent with the original Ip4Config behavior?  This seems like something 
that should be doable - in my experiment suppressing the configuration in 
Ip4Config2OnDhcp4SbInstalled does exactly this so it seems like some additional 
policy (automatic versus on-demand) could be defined to handle this.

> >
> > Modifying the IP configuration dynamically to suppress the interface
> > coming up at every boot is also problematic because it means we would
> > need to store the IP address configuration in another NVRAM location.
> > In other words, the combining of the IP address settings storage *and*
> > the policy of whether to configure at boot or wait until explicitly
> > requested is problematic - we really would like to control these
> > settings independently.  Is that possible within the scope of the
> > spec?  Could we just have a PCD that suppresses the automatic configure at
> boot under any circumstance?
> 
> Actually, if you want to recover the Ipv4 setting to the *un-configured*
> status (Note here: policy should be always set, *un-configured* means no IP
> address configuration), Ip4Config2 provide the ability to change the default
> policy. As git commit version SHA-1:
> 7648748e99eeeadec38fda7568adb260c4acc861 described, "This update let
> the other platform drivers have chance to change the default config data by
> consume Ip4Config2Protocol. "  So, you don't need to set another NVRAM
> location to do that, that means additional DXE_DRIVER can add in your
> platform ahead to change the default config data. The only operation in this
> DXE_DRIVER is to register a notify for Ip4Config2 protocol to change the
> default policy. That's also the reason why we don't want to enroll PCD to
> change it, you know two interface to do the same thing is not a good idea.

Let's say I have configured a static IP of 192.168.0.1.  But since I don't want 
the network interface to automatically configure at boot I think you are saying 
that I should implement a platform policy driver that uses Ip4Config2 to set an 
*unconfigured* state.  But there is no definition for an "unconfigured" state 
in UEFI 2.6 so how would one do this?  Furthermore, wouldn't this effectively 
delete the previous static IP data?  Maybe I just don't understand what you're 
describing - perhaps some pseudocode for your proposal that accomplishes the 
use case I'm describing would help.

Again, I think this is just two different pieces of information that needs to 
be stored separately: 1. static vs dhcp vs unconfigured IP address settings, 2. 
automatic versus on-demand startup .  Trying to to modify the IP address 
settings as an indirect way of communicating whether the stack should be 
configured seems far more troublesome than just storing another policy variable.

If you feel this should be brought to UNST instead let me know and I'll switch 
forums.

Eugene


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


Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency

2016-08-12 Thread Zeng, Star

On 2016/8/12 16:31, Paolo Bonzini wrote:



On 11/08/2016 04:37, Star Zeng wrote:

Minimize the code overhead between the two TSC reads by adding
new internal API to calculate TSC Frequency instead of reusing
MicroSecondDelay ().

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Paul A Lohr 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 +-
 .../Library/AcpiTimerLib/BaseAcpiTimerLib.c| 33 -
 .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 
 3 files changed, 99 insertions(+), 21 deletions(-)

diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c 
b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
index 806a4f7ce24c..e6fea231123d 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI Timer implements one instance of Timer Library.

-  Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -335,3 +335,57 @@ GetTimeInNanoSecond (

   return NanoSeconds;
 }
+
+/**
+  Calculate TSC frequency.
+
+  The TSC counting frequency is determined by comparing how far it counts
+  during a 100us period as determined by the ACPI timer. The ACPI timer is
+  used because it counts at a known frequency.
+  The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 1
+  clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
+  difference multiplied by 1 is the TSC frequency. There will be a small
+  error because of the overhead of reading the ACPI timer. An attempt is
+  made to determine and compensate for this error.
+
+  @return The number of TSC counts per second.
+
+**/
+UINT64
+InternalCalculateTscFrequency (
+  VOID
+  )
+{
+  UINT64  StartTSC;
+  UINT64  EndTSC;
+  UINT16  TimerAddr;
+  UINT32  Ticks;
+  UINT64  TscFrequency;
+  BOOLEAN InterruptState;
+
+  InterruptState = SaveAndDisableInterrupts ();
+
+  TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
+  Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 1);// Set 
Ticks to 100us in the future


ACPI_TIMER_FREQUENCY is 3579545, thus you're waiting 357 ticks but the
actual result of the division is much closer to 358.  The error is only
0.26%, but it's so simple to reduce it that I think it's worth it.  Just
change (ACPI_TIMER_FREQUENCY / 1) to (ACPI_TIMER_FREQUENCY + 5000) /
1.


Paolo,
It is nice to have it I think, and I prefer to use (ACPI_TIMER_FREQUENCY 
+ 5000) / 1.
Since I have pushed this patch, do you mind to contribute the proposal 
with a new patch?


Thanks,
Star



Another possibility is to count 343 ticks and multiply by 10436; 343 *
10436 is almost exactly ACPI_TIMER_FREQUENCY.

Paolo


+  StartTSC = AsmReadTsc (); // Get 
base value for the TSC
+  //
+  // Wait until the ACPI timer has counted 100us.
+  // Timer wrap-arounds are handled correctly by this function.
+  // When the current ACPI timer value is greater than 'Ticks', the while loop 
will exit.
+  //
+  while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
+CpuPause();
+  }
+  EndTSC = AsmReadTsc ();   // TSC 
value 100us later
+
+  TscFrequency = MultU64x32 (
+   (EndTSC - StartTSC), // Number 
of TSC counts in 100us
+   1// Number 
of 100us in a second
+   );
+
+  SetInterruptState (InterruptState);
+
+  return TscFrequency;
+}
+
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c 
b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
index 21fdb79908b8..8819ebcfccef 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI Timer implements one instance of Timer Library.

-  Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -17,6 +17,26 @@
 #include 

 /**
+  Calculate TSC frequency.
+
+  The TSC counting frequency is determined by comparing how far it counts
+  during a 100us period as determined by the ACPI timer. The ACPI 

Re: [edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

2016-08-12 Thread Gao, Liming
Duran:
  Reusing FvForceRebase flag is a good idea to resolve this issue. I agree your 
change. Reviewed-by: Liming Gao 

Thank
Liming
> -Original Message-
> From: Leo Duran [mailto:leo.du...@amd.com]
> Sent: Thursday, August 11, 2016 11:23 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong ; Gao, Liming
> ; Leo Duran 
> Subject: [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c
> 
> Account for rebase of FV section containing VTF file on IA32/IA64.
> This supports cases where the reset vector may not be set at 0xFFF0.
> 
> For example, FV section defined as:
> [FV.FvSecPei]
>   FvBaseAddress = $(FV_BOOT_BASE)
>   FvForceRebase = TRUE
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran 
> ---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index 7c839e2..8c2827b 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -2770,11 +2770,13 @@ Returns:
>//
>// Update reset vector (SALE_ENTRY for IPF)
>// Now for IA32 and IA64 platform, the fv which has bsf file must have 
> the
> -  // EndAddress of 0x. Thus, only this type fv needs to update 
> the
> -  // reset vector. If the PEI Core is found, the VTF file will probably 
> get
> -  // corrupted by updating the entry point.
> +  // EndAddress of 0x (unless the section was rebased).
> +  // Thus, only this type fv needs to update the  reset vector.
> +  // If the PEI Core is found, the VTF file will probably get
> +  // corrupted by updating the entry point.
>//
> -  if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> FV_IMAGES_TOP_ADDRESS) {
> +  if ((mFvDataInfo.ForceRebase == 1) ||
> +(mFvDataInfo.BaseAddress + mFvDataInfo.Size) ==
> FV_IMAGES_TOP_ADDRESS) {
>  Status = UpdateResetVector (, ,
> VtfFileImage);
>  if (EFI_ERROR(Status)) {
>Error (NULL, 0, 3000, "Invalid", "Could not update the reset 
> vector.");
> --
> 1.9.1

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


Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency

2016-08-12 Thread Paolo Bonzini


On 11/08/2016 04:37, Star Zeng wrote:
> Minimize the code overhead between the two TSC reads by adding
> new internal API to calculate TSC Frequency instead of reusing
> MicroSecondDelay ().
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Paul A Lohr 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c | 56 
> +-
>  .../Library/AcpiTimerLib/BaseAcpiTimerLib.c| 33 -
>  .../Library/AcpiTimerLib/DxeAcpiTimerLib.c | 31 
>  3 files changed, 99 insertions(+), 21 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c 
> b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> index 806a4f7ce24c..e6fea231123d 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/AcpiTimerLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>ACPI Timer implements one instance of Timer Library.
>  
> -  Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -335,3 +335,57 @@ GetTimeInNanoSecond (
>  
>return NanoSeconds;
>  }
> +
> +/**
> +  Calculate TSC frequency.
> +
> +  The TSC counting frequency is determined by comparing how far it counts
> +  during a 100us period as determined by the ACPI timer. The ACPI timer is
> +  used because it counts at a known frequency.
> +  The TSC is sampled, followed by waiting for ACPI_TIMER_FREQUENCY / 1
> +  clocks of the ACPI timer, or 100us. The TSC is then sampled again. The
> +  difference multiplied by 1 is the TSC frequency. There will be a small
> +  error because of the overhead of reading the ACPI timer. An attempt is
> +  made to determine and compensate for this error.
> +
> +  @return The number of TSC counts per second.
> +
> +**/
> +UINT64
> +InternalCalculateTscFrequency (
> +  VOID
> +  )
> +{
> +  UINT64  StartTSC;
> +  UINT64  EndTSC;
> +  UINT16  TimerAddr;
> +  UINT32  Ticks;
> +  UINT64  TscFrequency;
> +  BOOLEAN InterruptState;
> +
> +  InterruptState = SaveAndDisableInterrupts ();
> +
> +  TimerAddr = InternalAcpiGetAcpiTimerIoPort ();
> +  Ticks = IoRead32 (TimerAddr) + (ACPI_TIMER_FREQUENCY / 1);// Set 
> Ticks to 100us in the future

ACPI_TIMER_FREQUENCY is 3579545, thus you're waiting 357 ticks but the
actual result of the division is much closer to 358.  The error is only
0.26%, but it's so simple to reduce it that I think it's worth it.  Just
change (ACPI_TIMER_FREQUENCY / 1) to (ACPI_TIMER_FREQUENCY + 5000) /
1.

Another possibility is to count 343 ticks and multiply by 10436; 343 *
10436 is almost exactly ACPI_TIMER_FREQUENCY.

Paolo

> +  StartTSC = AsmReadTsc (); // Get 
> base value for the TSC
> +  //
> +  // Wait until the ACPI timer has counted 100us.
> +  // Timer wrap-arounds are handled correctly by this function.
> +  // When the current ACPI timer value is greater than 'Ticks', the while 
> loop will exit.
> +  //
> +  while (((Ticks - IoRead32 (TimerAddr)) & BIT23) == 0) {
> +CpuPause();
> +  }
> +  EndTSC = AsmReadTsc ();   // TSC 
> value 100us later
> +
> +  TscFrequency = MultU64x32 (
> +   (EndTSC - StartTSC), // 
> Number of TSC counts in 100us
> +   1// 
> Number of 100us in a second
> +   );
> +
> +  SetInterruptState (InterruptState);
> +
> +  return TscFrequency;
> +}
> +
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c 
> b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> index 21fdb79908b8..8819ebcfccef 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>ACPI Timer implements one instance of Timer Library.
>  
> -  Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -17,6 +17,26 @@
>  #include 
>  
>  /**
> +  Calculate TSC frequency.
> +
> +  The TSC counting frequency is determined by comparing how far it counts
> +  during a 100us period as determined by the ACPI timer. The ACPI timer is
> + 

Re: [edk2] [PATCH 4/4] Vlv2TbltDevicePkg: Add RAW file type to Rule.Common.SEC.BINARY

2016-08-12 Thread Wei, David
Reviewed-by: David Wei 

Thanks,
David  Wei 


-Original Message-
From: Gary Lin [mailto:g...@suse.com] 
Sent: Thursday, August 11, 2016 4:38 PM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [PATCH 4/4] Vlv2TbltDevicePkg: Add RAW file type to 
Rule.Common.SEC.BINARY

For GCC, it's using the reset vector in
IntelFspWrapperPkg/FspWrapperSecCore/Vtf0/Bin/ResetVec.ia32.raw.
Add the RAW file type to Rule.Common.SEC.BINARY in PlatformPkgGcc.fdf,
so that GenFds can handle the raw file.

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index e9292ca..33f2038 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -839,7 +839,11 @@ [Rule.Common.SEC]
 [Rule.Common.SEC.BINARY]
   FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
 PE32  PE32Align = 8   |.efi
-RAW BIN   Align = 16  |.com
+!if $(MINNOW2_FSP_BUILD) == TRUE
+RAW   RAW |.raw
+!else
+RAW   BIN Align = 16  |.com
+!endif
   }
 
 [Rule.Common.PEI_CORE]
-- 
2.9.2

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


Re: [edk2] [PATCH 1/4] Vlv2TbltDevicePkg/FspSupport: Fix GCC build errors

2016-08-12 Thread Wei, David
Reviewed-by: David Wei  

Thanks,
David  Wei 


-Original Message-
From: Gary Lin [mailto:g...@suse.com] 
Sent: Thursday, August 11, 2016 4:38 PM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [PATCH 1/4] Vlv2TbltDevicePkg/FspSupport: Fix GCC build errors

Fix the errors from GCC:

Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c:
 In function 'GetMemorySizeInMemoryTypeInformation':
Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c:59:40:
 error: passing argument 1 of '(*PeiServices)->GetHobList' from incompatible 
pointer type [-Werror=incompatible-pointer-types]
Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c:52:31:
 error: variable 'Status' set but not used [-Werror=unused-but-set-variable]

Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c:
 In function 'FspHobProcessForOtherData':
Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c:401:29:
 error: passing argument 1 of 'PlatformHobCreateFromFsp' from incompatible 
pointer type [-Werror=incompatible-pointer-types]

Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c:18:30:
 fatal error: Library\DebugLib.h: No such file or directory
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c:19:35:
 fatal error: Library\SerialPortLib.h: No such file or directory

Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c:18:27: 
fatal error: Library\IoLib.h: No such file or directory
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c:19:35: 
fatal error: Library\SerialPortLib.h: No such file or directory

Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c: In 
function 'EnableInternalUart':
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c:195:20:
 error: pointer targets in passing argument 1 of 'SerialPortWrite' differ in 
signedness [-Werror=pointer-sign]

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 .../Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c  | 5 ++---
 .../FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c  | 4 ++--
 .../FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c  | 6 +++---
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c
 
b/Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c
index f0b68cd..c308698 100644
--- 
a/Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c
+++ 
b/Vlv2TbltDevicePkg/FspSupport/Library/PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c
@@ -49,14 +49,13 @@ GetMemorySizeInMemoryTypeInformation (
   IN EFI_PEI_SERVICES **PeiServices
   )
 {
-  EFI_STATUS  Status;
   EFI_PEI_HOB_POINTERSHob;
   EFI_MEMORY_TYPE_INFORMATION *MemoryData;
   UINT8   Index;
   UINTN   TempPageNum;
 
   MemoryData = NULL;
-  Status = (*PeiServices)->GetHobList (PeiServices, (VOID **) );
+  (*PeiServices)->GetHobList ((CONST EFI_PEI_SERVICES **)PeiServices, (VOID 
**) );
   while (!END_OF_HOB_LIST (Hob)) {
 if (Hob.Header->HobType == EFI_HOB_TYPE_GUID_EXTENSION &&
   CompareGuid (>Name, )) {
@@ -398,7 +397,7 @@ FspHobProcessForOtherData (
   //
   // Other hob for platform
   //
-  PlatformHobCreateFromFsp ( PeiServices,  FspHobList);
+  PlatformHobCreateFromFsp ((CONST EFI_PEI_SERVICES **) PeiServices,  
FspHobList);
 
   return EFI_SUCCESS;
 }
diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c
index 2e181ac..c54b8af 100644
--- 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c
+++ 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/PlatformInit.c
@@ -15,8 +15,8 @@
 
 
 #include 
-#include 
-#include 
+#include 
+#include 
 
 VOID EnableInternalUart ();
 
diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c
index 4255ad3..c3ba557 100644
--- a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c
+++ b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/UartInit.c
@@ -15,8 +15,8 @@
 
 
 #include 
-#include 
-#include 
+#include 
+#include 
 
 #define PCI_IDX0xCF8
 #define PCI_DAT0xCFC
@@ -192,7 +192,7 @@ EnableInternalUart(
   MmioOr8 (PciD31F0RegBase + R_PCH_LPC_UART_CTRL, (UINT8) 

Re: [edk2] [PATCH 3/4] Vlv2TbltDevicePkg/PlatformFspLib: Fix the include path

2016-08-12 Thread Wei, David
Reviewed-by: David Wei  

Thanks,
David  Wei 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gary Lin
Sent: Thursday, August 11, 2016 4:38 PM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [edk2] [PATCH 3/4] Vlv2TbltDevicePkg/PlatformFspLib: Fix the include 
path

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c 
b/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
index 1306399..747b6a9 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
@@ -14,7 +14,7 @@
 **/
 #include "PiPei.h"
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.2

___
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