Re: [edk2] [PATCH v2 11/16] UefiCpuPkg: Add ACPI CPU Data include file

2015-10-16 Thread Laszlo Ersek 
On 10/15/15 01:26, Michael Kinney wrote:
> Add AcpuCpuData.h that defines a data structure that is shared between
> modules and is required for ACPI S3 support.
> APState field removed between V1 and V2 patch.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kinney 
> Cc: Laszlo Ersek 
> ---
>  UefiCpuPkg/Include/AcpiCpuData.h | 71 
> 
>  1 file changed, 71 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/AcpiCpuData.h

Usually v(n)->v(n+1) changes are not noted within the commit message,
only in the "notes" section of the posting (*), but I don't *always*
agree with that -- sometimes it is really useful to capture the
evolution of a patch forever. And I think this series qualifies.

(*) There's tooling support for that -- google "git notes". Others just
manually edit the formatted patch files (I don't like that; I use "git
notes".) Anyway, in this case I actually prefer the per-patch history to
be captured.

Thanks for dropping APState.

Reviewed-by: Laszlo Ersek 

Laszlo

> 
> diff --git a/UefiCpuPkg/Include/AcpiCpuData.h 
> b/UefiCpuPkg/Include/AcpiCpuData.h
> new file mode 100644
> index 000..a367257
> --- /dev/null
> +++ b/UefiCpuPkg/Include/AcpiCpuData.h
> @@ -0,0 +1,71 @@
> +/** @file
> +Definitions for CPU S3 data.
> +
> +Copyright (c) 2013 - 2015, 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
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef _ACPI_CPU_DATA_H_
> +#define _ACPI_CPU_DATA_H_
> +
> +//
> +// Register types in register table
> +//
> +typedef enum _REGISTER_TYPE {
> +  Msr,
> +  ControlRegister,
> +  MemoryMapped,
> +  CacheControl
> +} REGISTER_TYPE;
> +
> +//
> +// Element of register table entry
> +//
> +typedef struct {
> +  REGISTER_TYPE RegisterType;
> +  UINT32Index;
> +  UINT8 ValidBitStart;
> +  UINT8 ValidBitLength;
> +  UINT64Value;
> +} CPU_REGISTER_TABLE_ENTRY;
> +
> +//
> +// Register table definition, including current table length,
> +// allocated size of this table, and pointer to the list of table entries.
> +//
> +typedef struct {
> +  UINT32   TableLength;
> +  UINT32   NumberBeforeReset;
> +  UINT32   AllocatedSize;
> +  UINT32   InitialApicId;
> +  CPU_REGISTER_TABLE_ENTRY *RegisterTableEntry;
> +} CPU_REGISTER_TABLE;
> +
> +typedef struct {
> +  EFI_PHYSICAL_ADDRESS  StartupVector;
> +  EFI_PHYSICAL_ADDRESS  GdtrProfile;
> +  EFI_PHYSICAL_ADDRESS  IdtrProfile;
> +  EFI_PHYSICAL_ADDRESS  StackAddress;
> +  UINT32StackSize;
> +  UINT32NumberOfCpus;
> +  EFI_PHYSICAL_ADDRESS  MtrrTable;
> +  //
> +  // Physical address of a CPU_REGISTER_TABLE structure
> +  //
> +  EFI_PHYSICAL_ADDRESS  PreSmmInitRegisterTable;
> +  //
> +  // Physical address of a CPU_REGISTER_TABLE structure
> +  //
> +  EFI_PHYSICAL_ADDRESS  RegisterTable;
> +  EFI_PHYSICAL_ADDRESS  ApMachineCheckHandlerBase;
> +  UINT32ApMachineCheckHandlerSize;
> +} ACPI_CPU_DATA;
> +
> +#endif
> 

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

Re: [edk2] KVM: MTRR: fix memory type handling if MTRR is completely disabled

2015-10-16 Thread Laszlo Ersek 
On 10/16/15 05:05, Xiao Guangrong wrote:
> 
> 
> On 10/16/2015 12:18 AM, Laszlo Ersek wrote:
>> CC'ing Jordan and Chen Fan.
>>
>> On 10/15/15 09:10, Xiao Guangrong wrote:
>>>
>>>
>>> On 10/15/2015 02:58 PM, Janusz wrote:
 W dniu 15.10.2015 o 08:41, Xiao Guangrong pisze:
>
>
> On 10/15/2015 02:19 PM, Janusz wrote:
>> W dniu 15.10.2015 o 06:19, Xiao Guangrong pisze:
>>>
>>>
>>>
>>> Well, the bug may be not in KVM. When this bug happened, i saw OVMF
>>> only checked 1 CPU out, there is the log from OVMF's debug input:
>>>
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCD
>>>  Flushing GCDs
>>> Detect CPU count: 1
>>>
>>> So that the startup code has been freed however the APs are still
>>> running,
>>> i think that why we saw the vCPUs executed on unexpected address.
>>>
>>> After digging into OVMF's code, i noticed that BSP CPU waits for APs
>>> for a fixed timer period, however, KVM recent changes require zap
>>> all
>>> mappings if CR0.CD is changed, that means the APs need more time to
>>> startup.
>>>
>>> After following changes to OVMF, the bug is completely gone on my
>>> side:
>>>
>>> --- a/UefiCpuPkg/CpuDxe/ApStartup.c
>>> +++ b/UefiCpuPkg/CpuDxe/ApStartup.c
>>> @@ -454,7 +454,9 @@ StartApsStackless (
>>>   //
>>>   // Wait 100 milliseconds for APs to arrive at the ApEntryPoint
>>> routine
>>>   //
>>> -  MicroSecondDelay (100 * 1000);
>>> +  MicroSecondDelay (10 * 100 * 1000);
>>>
>>>   return EFI_SUCCESS;
>>> }
>>>
>>> Janusz, could you please check this instead? You can switch to your
>>> previous kernel to do this test.
>>>
>>>
>> Ok, now first time when I started VM I was able to start system
>> successfully. When I turned it off and started it again, it
>> restarted my
>> vm at system boot couple of times. Sometimes I also get very high cpu
>> usage for no reason. Also, I get less fps in GTA 5 than in kernel
>> 4.1, I
>> get something like 30-55, but on 4.1 I get all the time 60 fps.
>> This is
>> my new log: https://bpaste.net/show/61a122ad7fe5
>>
>
> Just confirm: the Qemu internal error did not appear any more, right?
 Yes, when I reverted your first patch, switched to -vga std from -vga
 none and didn't passthrough my GPU (case when I got this internal
 error), vm started without problem. I even didn't get any VM restarts
 like with passthrough

>>>
>>> Wow, it seems we have fixed the QEMU internal error now. :)
>>>
>>> Recurrently, Paolo has reverted some MTRR patches, was your test
>>> based on these reverted patches?
>>>
>>> The GPU passthrough issue may be related to vfio (not sure), Alex, do
>>> you have any idea?
>>>
>>> Laszlo, could you please check the root case is reasonable and fix it in
>>> OVMF if it's right?
>>
>> The code that you have found is in edk2's EFI_MP_SERVICES_PROTOCOL
>> implementation -- more closely, its initial CPU counter code --, from
>> edk2 git commit 533263ee5a7f. It is not specific to OVMF -- it is
>> generic edk2 code for Intel processors. (I'm CC'ing Jordan and Chen Fan
>> because they authored the patch in question.)
> 
> Okay, good to know it, i do not have much knowledge on edk2 and OVMF... :(
> 
>>
>> If VCPUs need more time to rendezvous than written in the code, on
>> recent KVM, then I think we should introduce a new FixedPCD in
>> UefiCpuPkg (practically: a compile time constant) for the timeout. Which
>> is not hard to do.
>>
>> However, we'll need two things:
>> - an idea about the concrete rendezvous timeout to set, from OvmfPkg
>>
>> - a *detailed* explanation / elaboration on your words:
>>
>>"KVM recent changes require zap all mappings if CR0.CD is changed,
>>that means the APs need more time to startup"
>>
>>Preferably with references to Linux kernel commits and the Intel SDM,
>>so that n00bs like me can get a fleeting idea. Do you mean that with
>>caching disabled, the APs execute their rendezvous code (from memory)
>>more slowly?
> 
> Kernel commit b18d5431acc causes the vCPUs need more time to startup
> as:
> - it zaps all the mappings for the guest memory in EPT or shadow page
>   table, it requires VM-exits to rebuild the mappings for all memory
>   access.
> 
> - if there is device passthrough-ed in guest and IOMMU lacks snooping
>   control feature, the memory will become UC after CR0.CD is set to 1.
> 
> And a generic factor is, if the guest has more vCPUs then more time is
> needed. That why the bug is hardly triggered on small vCPUs guest. I
> guess we need a self-adapting way to handle the case...

Thanks, this should be enough for composing a commit message.

Re: [edk2] [PATCH v2 00/16] UefiCpuPkg: Add CPU SMM and SecCore

2015-10-16 Thread Laszlo Ersek 
On 10/15/15 01:26, Michael Kinney wrote:
> Public branch: 

I diffed this against my own CRLF-ization of your v1 patches, plus two
patches applied ("UefiCpuPkg: CpuDxe: Fix ASSERT() when only 1 CPU
detected", "UefiCpuPkg: PiSmmCpuDxeSmm: prepare PT in InitPaging before
filling in PDE").

> Changes from PATCH v1 to PATCH v2:
> 1) Fix CRLF line ending issues reported by Laszlo

Confirmed.

> 2) Break up into a larger set of smaller patches

Thanks for that!

> 3) Remove APState field from ACPI_CPU_DATA structure

Confirmed.

> 4) Use module type specific CpuExceptionHandlerLib in DSC file
>instead of Null library instance

Confirmed.

> 5) Swap PTE init order for QEMU compatibility.
>Current PTE initialization algorithm works on HW but breaks QEMU
>emulator.  Update the PTE initialization order to be compatible
>with both.

Confirmed. Paolo's patch has been squashed in. (With an empty line removed.)

> 6) Update comment block that describes 32KB SMBASE alignment requirement
>to match contents of Intel(R) 64 and IA-32 Architectures Software
>Developer's Manual

Confirmed.

> 7) Remove BUGBUG comment and call to ClearSmi() that is not required.
>SMI should be cleared by root SMI handler.

Confirmed, and this is great.

Further changes I can find:

8) as mentioned above, "UefiCpuPkg: CpuDxe: Fix ASSERT() when only 1 CPU
detected" has been squashed in (from yourself).

9) SerialPortLib has been resolved to the Null instance, in
[LibraryClasses]. I'm not sure this is necessary. It certainly doesn't
hurt. On the other hand, that line has two trailing space characters;
those would be nice to remove.

10) "Include/Guid/UefiCpuPkgTokenSpace.h" has been dropped. So has the
reference to it.

I think this addresses all of my comments from earlier. I'll respond
individually to the three patches you CC'd me on.

In addition, I can see you've pushed a v3 as well. As far as I can see
(at ba4853cc0ac915755a7e8ded308c3504b8a32785), the v2-v3 differences are
small cleanups (comments, some fields renamed, additions of .h files to
.inf files). I think whatever I'll comment on those three patches
mentioned above, you can carry forward to v3.

One extra note: many of the changed comment lines in v3 have trailing
space characters; please consider dropping them.

Thanks!
Laszlo

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kinney 
> Cc: Laszlo Ersek 
> Cc: Paolo Bonzini 
> 
> Michael Kinney (16):
>   UefiCpuPkg: Add Cpuid.h include files for CPUID related defines
>   UefiCpuPkg: Update CPU MP drivers to support single CPU configuration
>   UefiCpuPkg: Add SMM Communication PPI and Handler Modules
>   UefiCpuPkg: Add PlatformSecLib
>   UefiCpuPkg: Add SecCore module
>   UefiCpuPkg: Add SecCore module and supporting library class and PCD
>   UefiCpuPkg: Add SmmCpuFeaturesLib
>   UefiCpuPkg: Add SmmCpuPlatformHookLib
>   UefiCpuPkg: Add SMM CPU Service Protocol
>   UefiCpuPkg: Add SMRAM Save State include file
>   UefiCpuPkg: Add ACPI CPU Data include file
>   UefiCpuPkg: Add CPU Hot Plug Data include file
>   UefiCpuPkg: Update DEC/DSC files for new includes and libraries
>   UefiCpuPkg: Add PiSmmCpuDxeSmm module no IA32/X64 files
>   UefiCpuPkg: Add PiSmmCpuDxeSmm module IA32 files
>   UefiCpuPkg: Add PiSmmCpuDxeSmm module X64 files
> 
>  UefiCpuPkg/CpuDxe/CpuMp.c  |   49 +-
>  UefiCpuPkg/CpuMpPei/CpuMpPei.c |   34 +-
>  UefiCpuPkg/CpuMpPei/CpuMpPei.h |1 +
>  UefiCpuPkg/Include/AcpiCpuData.h   |   71 +
>  UefiCpuPkg/Include/CpuHotPlugData.h|   33 +
>  UefiCpuPkg/Include/Library/PlatformSecLib.h|   70 +
>  UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h |  366 +
>  UefiCpuPkg/Include/Library/SmmCpuPlatformHookLib.h |  109 ++
>  UefiCpuPkg/Include/Protocol/SmmCpuService.h|  209 +++
>  UefiCpuPkg/Include/Register/Cpuid.h|   51 +
>  UefiCpuPkg/Include/Register/LocalApic.h|   13 -
>  UefiCpuPkg/Include/Register/SmramSaveStateMap.h|  190 +++
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c |1 +
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c|1 +
>  .../PlatformSecLibNull/PlatformSecLibNull.c|   90 ++
>  .../PlatformSecLibNull/PlatformSecLibNull.inf  |   37 +
>  .../PlatformSecLibNull/PlatformSecLibNull.uni  |  Bin 0 -> 1646 bytes
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  559 
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   39 +
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.uni|  Bin 0 -> 1674 bytes
>  .../SmmCpuPlatformHookLibNull.c|  108 ++
>  .../SmmCpuPlatformHookLibNull.inf  |   40 +
>  .../SmmCpuPlatformHookLibNull.uni  |  Bin 0 -> 1606 bytes
>

Re: [edk2] [PATCH v2 14/16] UefiCpuPkg: Add PiSmmCpuDxeSmm module no IA32/X64 files

2015-10-16 Thread Laszlo Ersek 
On 10/15/15 01:26, Michael Kinney wrote:
> Add module that initializes a CPU for the SMM environment and
> installs the first level SMI handler.  This module along with the
> SMM IPL and SMM Core provide the services required for
> DXE_SMM_DRIVERS to register hardware and software SMI handlers.
> 
> CPU specific features are abstracted through the SmmCpuFeaturesLib
> 
> Platform specific features are abstracted through the
> SmmCpuPlatformHookLib
> 
> Several PCDs are added to enable/disable features and configure
> settings for the PiSmmCpuDxeSmm module
> 
> Changes between [PATCH v1] and [PATCH v2]:
> 1) Swap PTE init order for QEMU compatibility.
>Current PTE initialization algorithm works on HW but breaks QEMU
>emulator.  Update the PTE initialization order to be compatible
>with both.
> 2) Update comment block that describes 32KB SMBASE alignment requirement
>to match contents of Intel(R) 64 and IA-32 Architectures Software
>Developer's Manual
> 3) Remove BUGBUG comment and call to ClearSmi() that is not required.
>SMI should be cleared by root SMI handler.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kinney 
> Cc: Laszlo Ersek 
> Cc: Paolo Bonzini 

This is obviously too big for me to review in depth, but I'll ACK it.

First however, please append the following three lines to the very end
of the commit message:

[pbonz...@redhat.com: InitPaging: prepare PT before filling in PDE]
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Paolo Bonzini 

With that change:

Acked-by: Laszlo Ersek 

Thank you for this work!!!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not execute RSM from 64-bit mode

2015-10-16 Thread Laszlo Ersek 
On 10/16/15 18:39, Kinney, Michael D wrote:
> Paolo,
> 
> Thanks for the reference.  We believe this is a document issue.
> 
> We would prefer to drop this patch.

I'm unsure if that will require another KVM patch, but given that the
original code runs on phys hardware alright, and the patch is for
UefiCpuPkg, and the issue is in the docs, I guess I'll drop the patch.
Although, I feel bad that Paolo had to come up with this mode switching
assembly in vain :(

Thanks
Laszlo

> 
> Thanks,
>  
> Mike
> 
>> -Original Message-
>> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
>> Sent: Friday, October 16, 2015 1:38 AM
>> To: Yao, Jiewen; Fan, Jeff; Laszlo Ersek; edk2-de...@ml01.01.org
>> Cc: Kinney, Michael D
>> Subject: Re: [edk2] [PATCH v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not
>> execute RSM from 64-bit mode
>>
>>
>>
>> On 16/10/2015 09:41, Yao, Jiewen wrote:
>>> Hello According to "IA32 SDM, page 1428, 4-330 Vol. 2B, RSM?Resume
>>> from System Management Mode", I do not find word say: 64bit mode is
>>> invalid. Would you please point out where you find "RSM is invalid in
>>> 64-bit mode "?
>>
>> It's in the heading.  It says
>>
>> 64-bit mode   Invalid
>> Compat/Leg mode   Valid
>>
>> Paolo
> ___
> 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 v3 51/52] OvmfPkg: double the SMRAM (TSEG) size for the 64-bit DXE phase builds

2015-10-16 Thread Kinney, Michael D 
Laszlo,

The PCD setting for TSEG size needs to be set for both IA32 and X64 modules in 
OvmfPkg/OvmfPkgIa32X64.dsc.
The this current patch, TSEG PCD is set to DEC default value for PEIMs of 1MB 
and 2MB for DXE/SMM phase.  
This causes SMM Core to only see 1MB SMRAM area.

I found this when trying to verify large number of VCPUs with 
OvmfPkg/OvmfPkgIa32X64.dsc build.

In order to test large VCPU numbers, we may also want to increase SMRAM size to 
more than 2 MB.

Thanks,

Mike

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Wednesday, October 14, 2015 3:27 PM
>To: edk2-de...@ml01.01.org
>Subject: [edk2] [PATCH v3 51/52] OvmfPkg: double the SMRAM (TSEG) size for
>the 64-bit DXE phase builds
>
>When building OVMF with -D SMM_REQUIRE -D SECURE_BOOT_ENABLE, using
>gcc-4.8, for the DEBUG build target, and with DEBUG_VERBOSE enabled in
>PcdDebugPrintErrorLevel, the build report files report the following
>binary sizes for the SMM_CORE and SMM modules, in kilobytes:
>
>  Driver  Ia32  X64
>    --  ---
>  PiSmmCore  56.7598.22
>  CpuIo2Smm  13.4722.72
>  SmmLockBox 24.6239.69
>  PiSmmCpuDxeSmm 66.06   123.03
>  FvbServicesSmm 22.2237.06
>  SmmFaultTolerantWriteDxe   35.1960.06
>  VariableSmm   356.88   640.28
>    --  ---
>  Total 575.19  1021.06
>
>The 1 MB default for PcdQ35TsegMbytes, from OvmfPkg.dec, is insufficient
>for loading the privileged half of the variable driver (VariableSmm) on
>X64. This results in many warnings about the dependent drivers being
>impossible to dispatch, and ultimately a DXE core assertion failure.
>
>PcdQ35TsegMbytes can assume the values 1, 2 or 8; set it to 2 for the DSC
>files with 64-bit DXE phases. For consistency between the DSC files, spell
>out 1 in OvmfPkgIa32.dsc too.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Laszlo Ersek 
>---
>
>Notes:
>v3:
>- new in v3
>
> OvmfPkg/OvmfPkgIa32.dsc| 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> 3 files changed, 3 insertions(+)
>
>diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>index 9040bdc..660a645 100644
>--- a/OvmfPkg/OvmfPkgIa32.dsc
>+++ b/OvmfPkg/OvmfPkgIa32.dsc
>@@ -401,6 +401,7 @@ [PcdsFixedAtBuild]
>
> !if $(SMM_REQUIRE) == TRUE
>   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|1
> !endif
>
> !if $(SECURE_BOOT_ENABLE) == TRUE
>diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>index 4aa7ba5..16dd785 100644
>--- a/OvmfPkg/OvmfPkgIa32X64.dsc
>+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>@@ -407,6 +407,7 @@ [PcdsFixedAtBuild]
> [PcdsFixedAtBuild.X64]
> !if $(SMM_REQUIRE) == TRUE
>   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|2
> !endif

Move PcdQ35TsegMbytes setting above [PcdsFixedAtBuild.X64] 

!if $(SMM_REQUIRE) == TRUE
  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes |2
!endif

[PcdsFixedAtBuild.X64]
!if $(SMM_REQUIRE) == TRUE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
!endif

>
> !if $(SECURE_BOOT_ENABLE) == TRUE
>diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>index 6ea135d..fe4889a 100644
>--- a/OvmfPkg/OvmfPkgX64.dsc
>+++ b/OvmfPkg/OvmfPkgX64.dsc
>@@ -406,6 +406,7 @@ [PcdsFixedAtBuild]
>
> !if $(SMM_REQUIRE) == TRUE
>   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>+  gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|2
> !endif
>
> !if $(SECURE_BOOT_ENABLE) == TRUE
>--
>1.8.3.1
>
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 04/52] UefiCpuPkg: CpuDxe: broadcast MTRR changes to APs

2015-10-16 Thread Kinney, Michael D 
Laszlo,

Here is a proposed fix on top of your v3 patch services to address a race 
condition that was introduced by the v3 patch series call to StartupAllAPs() 
with SingleThread set to TRUE in the MP initialization.  If the APs have not 
entered their idle loop before StartupAllAPs() is called, then some of the APs 
will be in an unexpected state, and StartupAllAPs() will hang.  This is the 
hang condition that is only seen when SingleThread parameter is set to TRUE and 
StartupAllAPs() is called very shortly after mAPsAlreadyInitFinished is set to 
TRUE that releases the APs to complete their initialization.

I added an internal function that checks to see if all APs are in the sleeping 
state in the idle loop.  On the first call to StartupAllAPs(), this internal 
function is used to make sure the APs are in a state that is compatible with 
StartupAllAPs() being used.  I put this check in the first call to 
StartupAllAPs(), so we do not take the delay to wait for the APs 
unconditionally in the MP init code.  Other work can get done while the APs 
work their way to their idle loop sleeping state.  

The one item remaining is to have a timeout with an ASSERT() if timeout is 
triggered in first call to StartupAllAPs() waiting for the APs to enter idle 
loop. 

I also reordered some of the actions InitializeMpSupport(), so the MP Services 
Protocol and call to StartupAllAPs() are done as late as possible to give APs 
more time to get to idle loop. 

Please merge this into your patch series.

Best regards,

Mike


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 


24d1f93110e1f73c70ff3705950c119693328deb
 UefiCpuPkg/CpuDxe/CpuMp.c | 93 ++-
 1 file changed, 75 insertions(+), 18 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index fbe43f5..4af0361 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -29,6 +29,7 @@ VOID *mApStackStart = 0;
 
 volatile BOOLEAN mAPsAlreadyInitFinished = FALSE;
 volatile BOOLEAN mStopCheckAllAPsStatus = TRUE;
+BOOLEAN  mStartupAllAPsCalled= FALSE;
 
 EFI_MP_SERVICES_PROTOCOL  mMpServicesTemplate = {
   GetNumberOfProcessors,
@@ -313,6 +314,47 @@ CheckAndUpdateAllAPsToIdleState (
 }
 
 /**
+  Check if all APs are in state CpuStateSleeping.
+  
+  Return TRUE if all APs are in the CpuStateSleeping state.  Do not 
+  check the state of the BSP or any disabled APs.
+
+  @retval TRUE   All APs are in CpuStateSleeping state.
+  @retval FALSE  One or more APs are not in CpuStateSleeping state.
+  
+**/
+BOOLEAN
+CheckAllAPsSleeping (
+  VOID
+  )
+{
+  UINTN   ProcessorNumber;
+  CPU_DATA_BLOCK  *CpuData;
+
+  for (ProcessorNumber = 0; ProcessorNumber < 
mMpSystemData.NumberOfProcessors; ProcessorNumber++) {
+CpuData = [ProcessorNumber];
+if (TestCpuStatusFlag (CpuData, PROCESSOR_AS_BSP_BIT)) {
+  //
+  // Skip BSP
+  //
+  continue;
+}
+
+if (!TestCpuStatusFlag (CpuData, PROCESSOR_ENABLED_BIT)) {
+  //
+  // Skip Disabled processors
+  //
+  continue;
+}
+
+if (GetApState (CpuData) != CpuStateSleeping) {
+  return FALSE;
+}
+  }
+  return TRUE;
+}  
+
+/**
   If the timeout expires before all APs returns from Procedure,
   we should forcibly terminate the executing AP and fill FailedList back
   by StartupAllAPs().
@@ -647,6 +689,18 @@ StartupAllAPs (
   //
   mStopCheckAllAPsStatus = TRUE;
 
+  if (!mStartupAllAPsCalled) {
+//
+// If this is first call to StartAllAPs(), then 
+// wait for all APs to enter idle loop.
+//
+while (!CheckAllAPsSleeping ()) {
+  CpuPause();
+}
+  
+mStartupAllAPsCalled = TRUE;
+  }
+
   for (Number = 0; Number < mMpSystemData.NumberOfProcessors; Number++) {
 CpuData = [Number];
 if (TestCpuStatusFlag (CpuData, PROCESSOR_AS_BSP_BIT)) {
@@ -1700,6 +1754,9 @@ InitializeMpSupport (
  sizeof (CPU_DATA_BLOCK) * 
mMpSystemData.NumberOfProcessors,
  mMpSystemData.CpuDatas);
 
+  //
+  // Release all APs to complete initialization and enter idle loop
+  //  
   mAPsAlreadyInitFinished = TRUE;
 
   //
@@ -1707,6 +1764,23 @@ InitializeMpSupport (
   //
   CollectBistDataFromHob ();
 
+  if (mMpSystemData.NumberOfProcessors > 1 && mMpSystemData.NumberOfProcessors 
< gMaxLogicalProcessorNumber) {
+if (mApStackStart != NULL) {
+  FreePages (mApStackStart, EFI_SIZE_TO_PAGES (
+  (gMaxLogicalProcessorNumber - 
mMpSystemData.NumberOfProcessors) *
+  gApStackSize));
+}
+  }
+
+  Status = gBS->CreateEvent (
+  EVT_SIGNAL_EXIT_BOOT_SERVICES,
+  TPL_CALLBACK,
+  ExitBootServicesCallback,
+  NULL,
+  
+  );
+  ASSERT_EFI_ERROR (Status);
+  
   //
   // Synchronize MTRR

Re: [edk2] [PATCH v2 00/16] UefiCpuPkg: Add CPU SMM and SecCore

2015-10-16 Thread Fan, Jeff 
It's good!

Reviewed-by: Jeff Fan 

-Original Message-
From: Kinney, Michael D 
Sent: Friday, October 16, 2015 10:54 AM
To: Kinney, Michael D; edk2-devel@lists.01.org; Fan, Jeff
Cc: Paolo Bonzini; Laszlo Ersek
Subject: RE: [edk2] [PATCH v2 00/16] UefiCpuPkg: Add CPU SMM and SecCore

Hi Jeff,

I have addressed the feedback you sent on code style issues.  

1) Fix function header comment blocks.  
2) Added missing .h files to PisSmmCpuDxeSmm INF file.

I have posted a V3 public branch with your feedback incorporated at:

https://github.com/mdkinney/edk2/tree/AddSmmUefiCpuPkg_V3

Please review and let me know if I missed any of the issues.

Thanks,

Mike

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

Re: [edk2] SMM core problems

2015-10-16 Thread Dimitri 
Hi,

Fist of all: I am DimitRi.

UDK2014/UEFI 2.4 does not have EFI_PROPERTIES_TABLE feature. Anyway we have 
a bug by default.

Collegue from Apple confirmed that os x boot loader move EfiRuntime memory 
to different physical address. I did not find in UEFI spec that it is 
prohibited. So we should clarify UEFI spec or change code. My opinion - 
Andrew is right and we should change code.


Also:

1. SmmCommunicationCommunicate function should be "virtualized" by 
SmmIplSetVirtualAddressNotify ()

2. We have this Runtime memory problem with SMBios tables in SmbiosDxe.

Regards,
Dimitri

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

Re: [edk2] [PATCH v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not execute RSM from 64-bit mode

2015-10-16 Thread Paolo Bonzini 


On 15/10/2015 03:31, Fan, Jeff wrote:
> Ersek & Bonzini,
> 
> From SDM 34.5.2, SMI Handler Operating Mode Switching.
> "Any required change to operating mode is performed by the RSM instruction; 
> there is no need for the SMI
> handler to change modes explicitly prior to executing RSM."
> 
> So, I don't think we need to go back to 32-bit PM before RSM.

The instruction set reference, however, says that RSM is invalid in
64-bit mode.

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

Re: [edk2] [PATCH v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not execute RSM from 64-bit mode

2015-10-16 Thread Yao, Jiewen 
Hello
According to "IA32 SDM, page 1428, 4-330 Vol. 2B, RSM?Resume from System 
Management Mode", I do not find word say: 64bit mode is invalid.
Would you please point out where you find "RSM is invalid in 64-bit mode "?

===
Operation
ReturnFromSMM;
IF (IA-32e mode supported) or (CPUID DisplayFamily_DisplayModel = 06H_0CH )
THEN
ProcessorState ← Restore(SMMDump(IA-32e SMM STATE MAP));
Else
ProcessorState ← Restore(SMMDump(Non-32-Bit-Mode SMM STATE MAP));
FI

64-Bit Mode Exceptions
Same exceptions as in protected mode.
===


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Paolo 
Bonzini
Sent: Friday, October 16, 2015 3:19 PM
To: Fan, Jeff; Laszlo Ersek; edk2-de...@ml01.01.org
Cc: Kinney, Michael D
Subject: Re: [edk2] [PATCH v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not execute 
RSM from 64-bit mode



On 15/10/2015 03:31, Fan, Jeff wrote:
> Ersek & Bonzini,
> 
> From SDM 34.5.2, SMI Handler Operating Mode Switching.
> "Any required change to operating mode is performed by the RSM 
> instruction; there is no need for the SMI handler to change modes explicitly 
> prior to executing RSM."
> 
> So, I don't think we need to go back to 32-bit PM before RSM.

The instruction set reference, however, says that RSM is invalid in 64-bit mode.

Paolo
___
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 v3 03/52] UefiCpuPkg: PiSmmCpuDxeSmm: do not execute RSM from 64-bit mode

2015-10-16 Thread Paolo Bonzini 


On 16/10/2015 09:41, Yao, Jiewen wrote:
> Hello According to "IA32 SDM, page 1428, 4-330 Vol. 2B, RSM?Resume
> from System Management Mode", I do not find word say: 64bit mode is
> invalid. Would you please point out where you find "RSM is invalid in
> 64-bit mode "?

It's in the heading.  It says

64-bit mode   Invalid
Compat/Leg mode   Valid

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

Re: [edk2] List Archive

2015-10-16 Thread Laszlo Ersek 
On 10/16/15 06:17, Narinder Dhillon wrote:
> Where can I go to look at the old posts for this list ?

In decreasing order of preference, and "chronological reach":

- Current archive (for the 01.org-based list):
  http://news.gmane.org/gmane.comp.bios.edk2.devel

- Recommended archive for the old (= sf.net-based) list:
  http://news.gmane.org/gmane.comp.bios.tianocore.devel

- Archive for the old (= sf.net-based) list, from before GMANE started
  archiving it:
  http://sourceforge.net/p/edk2/mailman/edk2-devel/

Thanks
Laszlo


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

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

[edk2] [PATCH] [PATCH] ArmVirtPkg: include BaseStackCheckLib also for AARCH64

2015-10-16 Thread Mark Rutland 
Some AArch64 toolchains also invoke the software stack checker
functions on certain code - so include BaseStackCheckLib for
AARCH64 as well as for ARM. Since this was the only difference
between [LibraryClasses.ARM] and [LibraryClasses.AARCH64],
merge the two into a single [LibraryClasses.common].

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mark Rutland 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index b75499f..a14d0d3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -229,21 +229,17 @@
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
 !endif
 
-[LibraryClasses.ARM]
+[LibraryClasses.common]
   #
   # It is not possible to prevent the ARM compiler for generic intrinsic 
functions.
   # This library provides the instrinsic functions generate by a given 
compiler.
-  # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
+  # [LibraryClasses.common] and NULL mean link this library into all images.
   #
   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
 
   # Add support for GCC stack protector
   NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
 
-[LibraryClasses.AARCH64]
-  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
-
-
 [BuildOptions]
   RVCT:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
 
-- 
1.9.1

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

Re: [edk2] [PATCH] [PATCH] ArmVirtPkg: include BaseStackCheckLib also for AARCH64

2015-10-16 Thread Mark Rutland 
On Fri, Oct 16, 2015 at 12:48:37PM +0200, Laszlo Ersek wrote:
> On 10/16/15 12:15, Mark Rutland wrote:
> > Some AArch64 toolchains also invoke the software stack checker
> > functions on certain code - so include BaseStackCheckLib for
> > AARCH64 as well as for ARM. Since this was the only difference
> > between [LibraryClasses.ARM] and [LibraryClasses.AARCH64],
> > merge the two into a single [LibraryClasses.common].
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Mark Rutland 
> > Cc: Ard Biesheuvel 
> > Cc: Laszlo Ersek 
> > Cc: Leif Lindholm 
> > ---
> >  ArmVirtPkg/ArmVirt.dsc.inc | 8 ++--
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index b75499f..a14d0d3 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -229,21 +229,17 @@
> >BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >  !endif
> >  
> > -[LibraryClasses.ARM]
> > +[LibraryClasses.common]
> >#
> ># It is not possible to prevent the ARM compiler for generic intrinsic 
> > functions.
> ># This library provides the instrinsic functions generate by a given 
> > compiler.
> > -  # [LibraryClasses.ARM] and NULL mean link this library into all ARM 
> > images.
> > +  # [LibraryClasses.common] and NULL mean link this library into all 
> > images.
> >#
> >NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >  
> ># Add support for GCC stack protector
> >NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> >  
> > -[LibraryClasses.AARCH64]
> > -  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > -
> > -
> >  [BuildOptions]
> >RVCT:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
> >  
> > 
> 
> I'll leave it to Ard & Leif to judge whether BaseStackCheckLib is
> appropriate for AARCH64 modules.
> 
> If it is, then the idea to move it to [LibraryClasses.common] is sane of
> course.

I was encountering build issues without it, but I'll defer to Ard and
Leif's judgement as to whether the above is the correct fix.

> One request though: this file already has a section called
> [LibraryClasses.common]; can you please move both CompilerIntrinsicsLib
> and BaseStackCheckLib up there?

Sure. I've tested that locally and I have working release and debug
builds.

> While at it, can you please fix up the English grammar in the comments?
> As in "prevent ... *from inserting*",  "*generated* by".

Done.

Expect a v2 shortly.

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

Re: [edk2] [PATCH 0/2] OvmfPkg: VirtioScsiDxe, VirtioBlkDxe: reset device at ExitBootServices()

2015-10-16 Thread Laszlo Ersek 
On 10/16/15 13:25, Laszlo Ersek wrote:
> I've been aware for a while that I should have added such callbacks to
> the virtio-blk and virtio-scsi drivers too. Because there had been no
> error reports, I could convince myself to code them up only now, due to
> .
> 
> Cc: Jordan Justen 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   OvmfPkg: VirtioScsiDxe: reset device at ExitBootServices()
>   OvmfPkg: VirtioBlkDxe: reset device at ExitBootServices()
> 
>  OvmfPkg/VirtioBlkDxe/VirtioBlk.h   |  1 +
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.h |  1 +
>  OvmfPkg/VirtioBlkDxe/VirtioBlk.c   | 44 
> +-
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 40 --
>  4 files changed, 83 insertions(+), 3 deletions(-)
> 

I meant to mention in the blurb that the VirtioXxxxExitBoot() functions
have differently styled comments between patch #1 and patch #2 because
the relevant source files use slightly different comment styles in
general; and now I wanted to stick with each one's own style.

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

[edk2] [PATCH 0/2] OvmfPkg: VirtioScsiDxe, VirtioBlkDxe: reset device at ExitBootServices()

2015-10-16 Thread Laszlo Ersek 
I've been aware for a while that I should have added such callbacks to
the virtio-blk and virtio-scsi drivers too. Because there had been no
error reports, I could convince myself to code them up only now, due to
.

Cc: Jordan Justen 

Thanks
Laszlo

Laszlo Ersek (2):
  OvmfPkg: VirtioScsiDxe: reset device at ExitBootServices()
  OvmfPkg: VirtioBlkDxe: reset device at ExitBootServices()

 OvmfPkg/VirtioBlkDxe/VirtioBlk.h   |  1 +
 OvmfPkg/VirtioScsiDxe/VirtioScsi.h |  1 +
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c   | 44 +-
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 40 --
 4 files changed, 83 insertions(+), 3 deletions(-)

-- 
1.8.3.1

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

[edk2] [PATCH 2/2] OvmfPkg: VirtioBlkDxe: reset device at ExitBootServices()

2015-10-16 Thread Laszlo Ersek 
(1) VirtioLib allocates the virtio ring in EfiBootServicesData memory.
(This is intentional.) Code that executes after ExitBootServices() is
permitted to reuse such memory.

(2) The hypervisor is allowed to look at, and act upon, a live virtio ring
at any time, even without explicit virtio kicks from the guest.

Should boot loader code or kernel code, running between ExitBootServices()
and the kernel's own virtio drivers resetting the device, overwrite the
pages that used to contain the virtio ring before ExitBootServices(), QEMU
could theoretically interpret that unrelated data as garbage ring
contents, and abort the guest.

Although we have seen no such reports, better be prudent and reset the
device in an ExitBootServices() event handler. Among other things, this
causes QEMU to forget about the device's virtio ring.

Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.h |  1 +
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 44 +++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
index 789caf9..ca4b7a0 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h
@@ -35,10 +35,11 @@ typedef struct {
   //
   // fieldinit function   init dpth
   // ---  -
   UINT32 Signature;// DriverBindingStart  0
   VIRTIO_DEVICE_PROTOCOL *VirtIo;  // DriverBindingStart  0
+  EFI_EVENT  ExitBoot; // DriverBindingStart  0
   VRING  Ring; // VirtioRingInit  2
   EFI_BLOCK_IO_PROTOCOL  BlockIo;  // VirtioBlkInit   1
   EFI_BLOCK_IO_MEDIA BlockIoMedia; // VirtioBlkInit   1
 } VBLK_DEV;
 
diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
index 862957c..75f85ca 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
@@ -841,10 +841,41 @@ VirtioBlkUninit (
 }
 
 
 /**
 
+  Event notification function enqueued by ExitBootServices().
+
+  @param[in] EventEvent whose notification function is being invoked.
+
+  @param[in] Context  Pointer to the VBLK_DEV structure.
+
+**/
+
+STATIC
+VOID
+EFIAPI
+VirtioBlkExitBoot (
+  IN  EFI_EVENT Event,
+  IN  VOID  *Context
+  )
+{
+  VBLK_DEV *Dev;
+
+  //
+  // Reset the device. This causes the hypervisor to forget about the virtio
+  // ring.
+  //
+  // We allocated said ring in EfiBootServicesData type memory, and code
+  // executing after ExitBootServices() is permitted to overwrite it.
+  //
+  Dev = Context;
+  Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
+}
+
+/**
+
   After we've pronounced support for a specific device in
   DriverBindingSupported(), we start managing said device (passed in by the
   Driver Exeuction Environment) with the following service.
 
   See DriverBindingSupported() for specification references.
@@ -899,23 +930,32 @@ VirtioBlkDriverBindingStart (
   Status = VirtioBlkInit (Dev);
   if (EFI_ERROR (Status)) {
 goto CloseVirtIo;
   }
 
+  Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+  , Dev, >ExitBoot);
+  if (EFI_ERROR (Status)) {
+goto UninitDev;
+  }
+
   //
   // Setup complete, attempt to export the driver instance's BlockIo interface.
   //
   Dev->Signature = VBLK_SIG;
   Status = gBS->InstallProtocolInterface (,
   , EFI_NATIVE_INTERFACE,
   >BlockIo);
   if (EFI_ERROR (Status)) {
-goto UninitDev;
+goto CloseExitBoot;
   }
 
   return EFI_SUCCESS;
 
+CloseExitBoot:
+  gBS->CloseEvent (Dev->ExitBoot);
+
 UninitDev:
   VirtioBlkUninit (Dev);
 
 CloseVirtIo:
   gBS->CloseProtocol (DeviceHandle, ,
@@ -985,10 +1025,12 @@ VirtioBlkDriverBindingStop (
   , >BlockIo);
   if (EFI_ERROR (Status)) {
 return Status;
   }
 
+  gBS->CloseEvent (Dev->ExitBoot);
+
   VirtioBlkUninit (Dev);
 
   gBS->CloseProtocol (DeviceHandle, ,
  This->DriverBindingHandle, DeviceHandle);
 
-- 
1.8.3.1

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

[edk2] [PATCH 1/2] OvmfPkg: VirtioScsiDxe: reset device at ExitBootServices()

2015-10-16 Thread Laszlo Ersek 
(1) VirtioLib allocates the virtio ring in EfiBootServicesData memory.
(This is intentional.) Code that executes after ExitBootServices() is
permitted to reuse such memory.

(2) The hypervisor is allowed to look at, and act upon, a live virtio ring
at any time, even without explicit virtio kicks from the guest.

Should boot loader code or kernel code, running between ExitBootServices()
and the kernel's own virtio drivers resetting the device, overwrite the
pages that used to contain the virtio ring before ExitBootServices(), QEMU
could theoretically interpret that unrelated data as garbage ring
contents, and abort the guest.

Although we have seen no such reports, better be prudent and reset the
device in an ExitBootServices() event handler. Among other things, this
causes QEMU to forget about the device's virtio ring.

Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioScsiDxe/VirtioScsi.h |  1 +
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 40 --
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h 
b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
index 0d3181d..80840d3 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h
@@ -50,10 +50,11 @@ typedef struct {
   //
   //  field  init function   init 
depth
   //     --  
--
   UINT32  Signature;  // DriverBindingStart  0
   VIRTIO_DEVICE_PROTOCOL  *VirtIo;// DriverBindingStart  0
+  EFI_EVENT   ExitBoot;   // DriverBindingStart  0
   BOOLEAN InOutSupported; // VirtioScsiInit  1
   UINT16  MaxTarget;  // VirtioScsiInit  1
   UINT32  MaxLun; // VirtioScsiInit  1
   UINT32  MaxSectors; // VirtioScsiInit  1
   VRING   Ring;   // VirtioRingInit  2
diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c 
b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
index 2cb3f43..e1e1203 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
@@ -931,11 +931,10 @@ Failed:
 
   return Status; // reached only via Failed above
 }
 
 
-
 STATIC
 VOID
 EFIAPI
 VirtioScsiUninit (
   IN OUT VSCSI_DEV *Dev
@@ -959,10 +958,36 @@ VirtioScsiUninit (
   SetMem (>PassThruMode, sizeof Dev->PassThruMode, 0x00);
 }
 
 
 //
+// Event notification function enqueued by ExitBootServices().
+//
+
+STATIC
+VOID
+EFIAPI
+VirtioScsiExitBoot (
+  IN  EFI_EVENT Event,
+  IN  VOID  *Context
+  )
+{
+  VSCSI_DEV *Dev;
+
+  //
+  // Reset the device. This causes the hypervisor to forget about the virtio
+  // ring.
+  //
+  // We allocated said ring in EfiBootServicesData type memory, and code
+  // executing after ExitBootServices() is permitted to overwrite it.
+  //
+  Dev = Context;
+  Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
+}
+
+
+//
 // Probe, start and stop functions of this driver, called by the DXE core for
 // specific devices.
 //
 // The following specifications document these interfaces:
 // - Driver Writer's Guide for UEFI 2.3.1 v1.01, 9 Driver Binding Protocol
@@ -1048,24 +1073,33 @@ VirtioScsiDriverBindingStart (
   Status = VirtioScsiInit (Dev);
   if (EFI_ERROR (Status)) {
 goto CloseVirtIo;
   }
 
+  Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+  , Dev, >ExitBoot);
+  if (EFI_ERROR (Status)) {
+goto UninitDev;
+  }
+
   //
   // Setup complete, attempt to export the driver instance's PassThru
   // interface.
   //
   Dev->Signature = VSCSI_SIG;
   Status = gBS->InstallProtocolInterface (,
   , EFI_NATIVE_INTERFACE,
   >PassThru);
   if (EFI_ERROR (Status)) {
-goto UninitDev;
+goto CloseExitBoot;
   }
 
   return EFI_SUCCESS;
 
+CloseExitBoot:
+  gBS->CloseEvent (Dev->ExitBoot);
+
 UninitDev:
   VirtioScsiUninit (Dev);
 
 CloseVirtIo:
   gBS->CloseProtocol (DeviceHandle, ,
@@ -1112,10 +1146,12 @@ VirtioScsiDriverBindingStop (
   , >PassThru);
   if (EFI_ERROR (Status)) {
 return Status;
   }
 
+  gBS->CloseEvent (Dev->ExitBoot);
+
   VirtioScsiUninit (Dev);
 
   gBS->CloseProtocol (DeviceHandle, ,
  This->DriverBindingHandle, DeviceHandle);
 
-- 
1.8.3.1


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

[edk2] [PATCHv2] ArmVirtPkg: include BaseStackCheckLib also for AARCH64

2015-10-16 Thread Mark Rutland 
Some AArch64 toolchains also invoke the software stack checker
functions on certain code - so include BaseStackCheckLib for
AARCH64 as well as for ARM. Since this was the only difference
between [LibraryClasses.ARM] and [LibraryClasses.AARCH64],
merge the two into the existing [LibraryClasses.common].

At the same time, fix the grammar for the related comments.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mark Rutland 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

Since v1:
* Fold into existing [LibraryClasses.common] section.
* Correct grammar.
* Drop redundant comment line.

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index b75499f..8626919 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -70,6 +70,15 @@
   UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
   IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
 
+  #
+  # It is not possible to prevent the ARM compiler from inserting calls to 
intrinsic functions.
+  # This library provides the instrinsic functions such a compiler may 
generate calls to.
+  #
+  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+
+  # Add support for GCC stack protector
+  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+
   # ARM Architectural Libraries
   
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
   
DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
@@ -229,21 +238,6 @@
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
 !endif
 
-[LibraryClasses.ARM]
-  #
-  # It is not possible to prevent the ARM compiler for generic intrinsic 
functions.
-  # This library provides the instrinsic functions generate by a given 
compiler.
-  # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
-  #
-  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
-
-  # Add support for GCC stack protector
-  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
-
-[LibraryClasses.AARCH64]
-  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
-
-
 [BuildOptions]
   RVCT:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
 
-- 
1.9.1

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

Re: [edk2] [PATCH] [PATCH] ArmVirtPkg: include BaseStackCheckLib also for AARCH64

2015-10-16 Thread Leif Lindholm 
On Fri, Oct 16, 2015 at 12:48:37PM +0200, Laszlo Ersek wrote:
> On 10/16/15 12:15, Mark Rutland wrote:
> > Some AArch64 toolchains also invoke the software stack checker
> > functions on certain code - so include BaseStackCheckLib for
> > AARCH64 as well as for ARM. Since this was the only difference
> > between [LibraryClasses.ARM] and [LibraryClasses.AARCH64],
> > merge the two into a single [LibraryClasses.common].
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Mark Rutland 
> > Cc: Ard Biesheuvel 
> > Cc: Laszlo Ersek 
> > Cc: Leif Lindholm 
> > ---
> >  ArmVirtPkg/ArmVirt.dsc.inc | 8 ++--
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index b75499f..a14d0d3 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -229,21 +229,17 @@
> >BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> >  !endif
> >  
> > -[LibraryClasses.ARM]
> > +[LibraryClasses.common]
> >#
> ># It is not possible to prevent the ARM compiler for generic intrinsic 
> > functions.
> ># This library provides the instrinsic functions generate by a given 
> > compiler.
> > -  # [LibraryClasses.ARM] and NULL mean link this library into all ARM 
> > images.
> > +  # [LibraryClasses.common] and NULL mean link this library into all 
> > images.
> >#
> >NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >  
> ># Add support for GCC stack protector
> >NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> >  
> > -[LibraryClasses.AARCH64]
> > -  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > -
> > -
> >  [BuildOptions]
> >RVCT:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
> >  
> > 
> 
> I'll leave it to Ard & Leif to judge whether BaseStackCheckLib is
> appropriate for AARCH64 modules.

There is nothing wrong with using BaseStackCheckLib for AArch64,
although looking through the code called from there I have some
concerns with regards to the suitability of the existing
CpuBreakpoint() and CpuDeadLoop() implementations for AArch64, and at
least the other for AArch32 as well. 

These are however already called from elsewhere, so that shouldn't
hold back this patch.

/
Leif

> If it is, then the idea to move it to [LibraryClasses.common] is sane of
> course.
> 
> One request though: this file already has a section called
> [LibraryClasses.common]; can you please move both CompilerIntrinsicsLib
> and BaseStackCheckLib up there?
> 
> While at it, can you please fix up the English grammar in the comments?
> As in "prevent ... *from inserting*",  "*generated* by".
> 
> Thanks!
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCHv2] ArmVirtPkg: include BaseStackCheckLib also for AARCH64

2015-10-16 Thread Laszlo Ersek 
On 10/16/15 14:52, Mark Rutland wrote:
> Some AArch64 toolchains also invoke the software stack checker
> functions on certain code - so include BaseStackCheckLib for
> AARCH64 as well as for ARM. Since this was the only difference
> between [LibraryClasses.ARM] and [LibraryClasses.AARCH64],
> merge the two into the existing [LibraryClasses.common].
> 
> At the same time, fix the grammar for the related comments.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Mark Rutland 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 24 +---
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> Since v1:
> * Fold into existing [LibraryClasses.common] section.
> * Correct grammar.
> * Drop redundant comment line.
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index b75499f..8626919 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -70,6 +70,15 @@
>UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
>IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
>  
> +  #
> +  # It is not possible to prevent the ARM compiler from inserting calls to 
> intrinsic functions.
> +  # This library provides the instrinsic functions such a compiler may 
> generate calls to.
> +  #
> +  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> +
> +  # Add support for GCC stack protector
> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +
># ARM Architectural Libraries
>
> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
>
> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
> @@ -229,21 +238,6 @@
>BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>  !endif
>  
> -[LibraryClasses.ARM]
> -  #
> -  # It is not possible to prevent the ARM compiler for generic intrinsic 
> functions.
> -  # This library provides the instrinsic functions generate by a given 
> compiler.
> -  # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
> -  #
> -  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> -
> -  # Add support for GCC stack protector
> -  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> -
> -[LibraryClasses.AARCH64]
> -  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> -
> -
>  [BuildOptions]
>RVCT:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
>  
> 

Thanks Leif for the help in verifying this!

I regression-tested the AARCH64 build of ArmVirtQemu.dsc and
ArmVirtXen.dsc, and also booted the former. (I used Linaro's gcc-4.8
cross compiler, release 2015.06.)

Reviewed-by: Laszlo Ersek 

Committed to SVN as rev 18617.

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

Re: [edk2] [PATCH] Update the ACPI device information for ARM Juno.

2015-10-16 Thread Leif Lindholm 
Hi Jeremy,

Thanks for this.

On Wed, Oct 14, 2015 at 10:10:06AM -0500, Jeremy Linton wrote:
> These patches correct a number of problems with the JUNO ACPI tables.
> 
> First, put CCA attributes on the devices which can do DMA. This is
> because the linux kernel now requires ARM64 devices specify a coherency
> model. Without CCA the devices are unable to perform DMA.
> 
> Update the EHCI window to a full 64k as documented in the
> Juno Platform SoC TRM. This makes it match the values used in
> other places.
> 
> Finally, add the MALI GPU resources to the tables, and add some _DSD
> entries for the SMSC ethernet chip. The latter changes are required
> for the mainline kernels to use the adapter.

So ... I'm not sure there are any ACPI-aware drivers for the Mali bits
yet - I would be happier to leave that out until we've verified it
works.

Could you resubmit with that single change please?

> Signed-off-by: Jeremy Linton 
> ---
>  .../ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl  |  1 +
>  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl  | 27 
> --
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl 
> b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> index 7d50a5f..645ba93 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> +++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
> @@ -51,6 +51,7 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", 
> "ARM-JUNO", EFI_ACPI_ARM_OEM
>   Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
>   Name(_SEG, Zero) // PCI Segment Group number
>   Name(_BBN, Zero) // PCI Base Bus Number
> + Name(_CCA, 1)// Initially mark the PCI coherent (for JunoR1)
>  
>  // Root Complex 0
>  Device (RP0) {
> diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl 
> b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
> index 7a56f00..987186f 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
> +++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
> @@ -68,6 +68,15 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 1, "ARMLTD", 
> "ARM-JUNO", EFI_ACPI_ARM_O
>Memory32Fixed(ReadWrite, 0x1A00, 0x1000)
>Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 
> 192 }
>})
> +  Name(_DSD, Package() {
> +   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +   Package() {
> +   Package(2) {"phy-mode", "mii"},
> +   Package(2) {"reg-io-width", 4 },
> +   Package(2) {"smsc,irq-active-high",1},
> +   Package(2) {"smsc,irq-push-pull",1}
> +  }
> +  }) // _DSD()
>  }
>  
>  // UART PL011
> @@ -82,16 +91,30 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 1, "ARMLTD", 
> "ARM-JUNO", EFI_ACPI_ARM_O
>  }
>  
>  //
> -// USB Host Controller
> +// Mali GPU
> +//
> +Device(GPU0){
> +Name(_HID, "ARMH0624")
> +Name(_UID, Zero)
> +Name(_CCA, 1)
> +Name(_CRS, ResourceTemplate() {
> +   Memory32Fixed(ReadWrite, 0x2D00, 0x1)
> +   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) 
> { 64 } // GPU interrupt 64, job interrupt 65, MMU interrupt 66
> +  })
> +}
> +
> +//
> +// USB EHCI Host Controller
>  //
>  Device(USB0){
>  Name(_HID, "ARMH0D20")
>  Name(_CID, "PNP0D20")
>  Name(_UID, 2)
> +Name(_CCA, 0) //EHCI on this platform is not coherent!
>  
>  Method(_CRS, 0x0, Serialized){
>  Name(RBUF, ResourceTemplate(){
> -Memory32Fixed(ReadWrite, 0x7FFC, 0x00B0)
> +Memory32Fixed(ReadWrite, 0x7FFC, 0x1)
>  Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) 
> {149}  // INT ID=149 GIC IRQ ID=117 for Juno SoC USB EHCI Controller
>  })
>  Return(RBUF)
> -- 
> 2.4.3
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] Update the ACPI device information for ARM Juno.

2015-10-16 Thread Jeremy Linton 

On 10/16/2015 09:19 AM, Leif Lindholm wrote:

So ... I'm not sure there are any ACPI-aware drivers for the Mali bits
yet - I would be happier to leave that out until we've verified it
works.

Could you resubmit with that single change please?


Leif,

I will pull them, those changes were a little preemptive anyway.


Thanks,






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

Re: [edk2] [PATCH v2 0/8] serial and DEBUG output tweaks

2015-10-16 Thread Laszlo Ersek 
On 10/14/15 14:30, Laszlo Ersek wrote:
> Addressing v1 review feedback, and adding an early hello message for
> ARM. Changes are noted per patch.
> 
> Testing on Xen and/or physical ARM|AARCH64 platforms would be
> appreciated. (The patches should be easy to apply from the mailing list
> -- no new files are created, so just use "git am --keep-cr".)
> 
> Thanks!
> Laszlo
> 
> Cc: Ard Biesheuvel 
> Cc: Liming Gao 
> Cc: Drew Jones 
> Cc: Leif Lindholm 
> Cc: Yehuda Yitschak 
> Cc: Star Zeng 
> 
> Laszlo Ersek (8):
>   ArmPlatformPkg: NorFlashDxe: mellow DEBUG messages about flash reinit
>   MdeModulePkg: FaultTolerantWriteDxe: mellow DEBUGs about workspace
> reinit
>   MdeModulePkg: FaultTolerantWriteDxe: clean up some "success" messages
>   MdeModulePkg: SmbiosDxe: soften DEBUG messages about table
> reallocation
>   ArmPlatformPkg: introduce fixed PCD for early hello message
>   ArmPlatformPkg: PrePeiCore: write early hello message to the serial
> port
>   ArmPlatformPkg: PrePi: write early hello message to the serial port
>   ArmVirtPkg: set early hello message
> 
>  ArmPlatformPkg/ArmPlatformPkg.dec |  7 
> +++
>  ArmVirtPkg/ArmVirt.dsc.inc|  1 +
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf|  2 ++
>  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf   |  2 ++
>  ArmPlatformPkg/PrePi/PeiMPCore.inf|  2 ++
>  ArmPlatformPkg/PrePi/PeiUniCore.inf   |  2 ++
>  ArmPlatformPkg/PrePeiCore/PrePeiCore.h|  1 +
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c   | 19 
> +--
>  ArmPlatformPkg/PrePeiCore/MainMPCore.c|  5 +
>  ArmPlatformPkg/PrePeiCore/MainUniCore.c   |  5 +
>  ArmPlatformPkg/PrePi/MainMPCore.c |  5 +
>  ArmPlatformPkg/PrePi/MainUniCore.c|  5 +
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c |  6 +++---
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c|  6 --
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c |  2 +-
>  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c  |  6 --
>  16 files changed, 62 insertions(+), 14 deletions(-)
> 

I committed the first four patches to SVN (r18618..r18621, inclusive).

I'd appreciate some guidance with regard to the other half of the series.

I'm also fine if that guidance is "we don't want this, go away"; that
gives me excuse to do it downstream only.

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

[edk2] [PATCH] Update the ACPI device information for ARM Juno.

2015-10-16 Thread Jeremy Linton 
These patches correct a number of problems with the JUNO ACPI tables.

First, put CCA attributes on the devices which can do DMA. This is
because the linux kernel now requires ARM64 devices specify a coherency
model. Without CCA the devices are unable to perform DMA.

Update the EHCI window to a full 64k as documented in the
Juno Platform SoC TRM. This makes it match the values used in some
other places.

Finally, add some _DSD entries for the SMSC ethernet chip.
The latter changes are required for the mainline kernels to use the adapter.

Signed-off-by: Jeremy Linton 
---
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl |  1 +
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl| 14 --
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
index 7d50a5f..645ba93 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
@@ -51,6 +51,7 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_OEM
Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge
Name(_SEG, Zero) // PCI Segment Group number
Name(_BBN, Zero) // PCI Base Bus Number
+   Name(_CCA, 1)// Initially mark the PCI coherent (for JunoR1)
 
 // Root Complex 0
 Device (RP0) {
diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
index 7a56f00..c80f46a 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl
@@ -68,6 +68,15 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_O
   Memory32Fixed(ReadWrite, 0x1A00, 0x1000)
   Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 192 }
   })
+  Name(_DSD, Package() {
+   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+   Package() {
+   Package(2) {"phy-mode", "mii"},
+   Package(2) {"reg-io-width", 4 },
+   Package(2) {"smsc,irq-active-high",1},
+   Package(2) {"smsc,irq-push-pull",1}
+  }
+  }) // _DSD()
 }
 
 // UART PL011
@@ -82,16 +91,17 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_O
 }
 
 //
-// USB Host Controller
+// USB EHCI Host Controller
 //
 Device(USB0){
 Name(_HID, "ARMH0D20")
 Name(_CID, "PNP0D20")
 Name(_UID, 2)
+Name(_CCA, 0) //EHCI on this platform is not coherent!
 
 Method(_CRS, 0x0, Serialized){
 Name(RBUF, ResourceTemplate(){
-Memory32Fixed(ReadWrite, 0x7FFC, 0x00B0)
+Memory32Fixed(ReadWrite, 0x7FFC, 0x1)
 Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) 
{149}  // INT ID=149 GIC IRQ ID=117 for Juno SoC USB EHCI Controller
 })
 Return(RBUF)
-- 
2.4.3


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

[edk2] [PATCHv2] ArmPlatformPkg/ArmJunoPkg correct ACPI tables

2015-10-16 Thread Jeremy Linton 
v2 
Removed mali definition as its not been completely tested yet.

v1
This set of patches updates the ACPI tables for the JunoR1 in keeping with
recent changes to the linux kernel.

These changes allow both the RHEL and mainline kernels to boot with a functional
USB and network adapter. Given the previous PCIe change posted by Supreeth, and 
this
one, the PCIe host bridge works with RHEL and hopefully future version of linux.

Jeremy Linton (1):
  Update the ACPI device information for ARM Juno.

 ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl |  1 +
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Dsdt.asl| 14 --
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.4.3


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

Re: [edk2] [PATCH] OvmfPkg: Sec: Fix SOURCE_DEBUG_ENABLE ASSERT()

2015-10-16 Thread Laszlo Ersek 
On 10/15/15 00:18, Michael Kinney wrote:
> The update to the LocalApicLib instances to make sure the
> Local APIC is initialize before use generates an ASSERT()
> when SOURCE_DEBUG_ENABLE is enabled for OVMF.
> 
> The fix is to initialize the Local APIC Timer and mask it
> before initialing the DebugAgent.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Kinney 
> ---
>  OvmfPkg/Sec/SecMain.c   | 10 +-
>  OvmfPkg/Sec/SecMain.inf |  3 ++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index b7df549..2d5e51b 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -1,7 +1,7 @@
>  /** @file
>Main SEC phase code.  Transitions to PEI.
>  
> -  Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.
> +  Copyright (c) 2008 - 2015, 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
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -767,6 +768,13 @@ SecCoreStartupWithStack (
>//
>IoWrite8 (0x21, 0xff);
>IoWrite8 (0xA1, 0xff);
> +
> +  //
> +  // Initialize Local APIC Timer hardware and disable Local APIC Timer 
> interrupts
> +  // before initializing the Debug Agent and the debug timer is enabled
> +  //
> +  InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
> +  DisableApicTimerInterrupt ();

So, reviewing this took forever :) I'll make several points.

(1) This addition is consistent with the IoWrite8() calls just above it.
The context doesn't show, but there's a comment up there explaining the
IoWrite8() calls. Those do the same thing, just for the i8259 PIC.

(2) The assertion is triggered on the following path:

SecCoreStartupWithStack()
  InitializeDebugAgent(DEBUG_AGENT_INIT_PREMEM_SEC)
InitializeDebugTimer()
  GetApicTimerState()
ASSERT()

(3) The InitializeApicTimer() call sets the exact bit that the ASSERT()
checks.

(4) There will be a small window where the APIC timer is ticking /
armed, but the timer interrupt is disabled very soon after. There is no
library API that allows one to initialize the timer without arming it,
so this is what we have to do.

(5) Regarding the arguments in the InitializeApicTimer() call.
DivideValue=0 means "use the current divisor", MAX_UINT32 is the safest
InitCount (I guess the counting is downwards), PeriodicMode=TRUE is
irrelevant, and the timer interrupt vector (5) is also irrelevant. I
grepped the source for similar calls, and this looks to be a copy-paste
more or less, which is fine.

On the following path, the timer is also initialized (if we pass the
ASSERT()):

SecCoreStartupWithStack()
  InitializeDebugAgent(DEBUG_AGENT_INIT_PREMEM_SEC)
InitializeDebugTimer()
  GetApicTimerState()
  InitializeApicTimer()

and in *that* call, the vector number (DEBUG_TIMER_VECTOR=32) matters,
and is sane.

Therefore,

Reviewed-by: Laszlo Ersek 

(6) I lightly regression tested this change, it doesn't seem to break
things.

Regression-tested-by: Laszlo Ersek 

I committed this patch to SVN (r18622) with the following tweaks:
- I rewrapped the commit message to 74 chars and fixed a typo in it.
- I rewrapped the new code comment at 79 chars and added a period.
- I located the exact patch in the history that had introduced the
  ASSERT(), and added a reference to it in the commit message.

Thanks!
Laszlo

>
>//
>// Initialize Debug Agent to support source level debug in SEC/PEI phases 
> before memory ready.
> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> index fce99fb..2f78f3c 100644
> --- a/OvmfPkg/Sec/SecMain.inf
> +++ b/OvmfPkg/Sec/SecMain.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  SEC Driver
>  #
> -#  Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2008 - 2015, 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
> @@ -55,6 +55,7 @@
>PeCoffGetEntryPointLib
>PeCoffExtraActionLib
>ExtractGuidedSectionLib
> +  LocalApicLib
>  
>  [Ppis]
>gEfiTemporaryRamSupportPpiGuid# PPI ALWAYS_PRODUCED
> 

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