[edk2-devel] [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test

[Eric Jin]

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

In the ExitBootServices() test, after ExitBootServices() call, all
boot services are forbidden. The original design is to save the return
status value of ExitBootServices() in variable using variable service
and reset, but this needs one additional execution of the test to
retrieve the value from variable and this design was not straightforward
from end user perspective.
This patch enhances the test by leveraging RecoveryLib to restore
execution after reset automatically, thus requiring only one execution.

Cc: Supreeth Venkatesh 
Signed-off-by: Eric Jin 
---
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
  |   3 ++-
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
|   9 -
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
 | 121 
+
 3 files changed, 115 insertions(+), 18 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
index 49ad79915934..3de43a20e8a4 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
@@ -1,7 +1,7 @@
 ## @file
 #
 #  Copyright 2006 - 2012 Unified EFI, Inc.
-#  Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.
+#  Copyright (c) 2010 - 2019, 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
@@ -53,4 +53,5 @@
 
 [Protocols]
   gEfiTestProfileLibraryGuid
+  gEfiTestRecoveryLibraryGuid
   gBlackBoxEfiHIIPackageListProtocolGuid
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
index b1c35fee7435..008584577ed1 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright 2006 - 2017 Unified EFI, Inc.
-  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2019, 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
@@ -35,6 +35,13 @@ Abstract:
 #include EFI_PROTOCOL_DEFINITION (LoadFile)
 
 #include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
+#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
+
+typedef struct _RESET_DATA {
+  UINTN   Step;
+  UINTN   TplIndex;
+  UINT32  RepeatTimes;
+} RESET_DATA;
 
 #if (EFI_SPECIFICATION_VERSION >= 0x0002000A)
 
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
index 0a26d46847da..6ce1d2d72669 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright 2006 - 2016 Unified EFI, Inc.
-  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2019, 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
@@ -30,7 +30,8 @@ Abstract:
 #define TEST_VENDOR1_GUID \
   { 0xF6FAB04F, 0xACAF, 0x4af3, { 0xB9, 0xFA, 0xDC, 0xF9, 0x7F, 0xB4, 0x42, 
0x6F } }
 
-#define MAX_BUFFER_SIZE  10
+#define STATUS_BUFFER_SIZE   8
+#define RECOVER_BUFFER_SIZE  1024
 
 EFI_GUID gTestVendor1Guid = TEST_VENDOR1_GUID;
 
@@ -778,19 +779,23 @@ BBTestExitBootServicesConsistencyTest (
   )
 {
   EFI_STANDARD_TEST_LIBRARY_PROTOCOL   *StandardLib;
+  EFI_TEST_RECOVERY_LIBRARY_PROTOCOL   *RecoveryLib;
   EFI_STATUS   Status;
   EFI_TEST_ASSERTION   AssertionType;
   UINTNMapKey;
-
+  UINTNSize;
   UINTNNumbers;
   UINTNDataSize;
-  UINT8Data[MAX_BUFFER_SIZE];
+  RESET_DATA   *ResetData;
+  UINT8

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Yao, Jiewen]

Thank you Mike!

That is good reference on the real hardware behavior. (Glad it is public.)

For threat model, the unique part in virtual environment is temp RAM.
The temp RAM in real platform is per CPU cache, while the temp RAM in virtual 
platform is global memory.
That brings one more potential attack surface in virtual environment, if 
hot-added CPU need run code with stack or heap before SMI rebase.

Other threats, such as SMRAM or DMA, are same.

Thank you
Yao Jiewen


> -Original Message-
> From: Kinney, Michael D
> Sent: Friday, August 23, 2019 9:03 AM
> To: Paolo Bonzini ; Laszlo Ersek
> ; r...@edk2.groups.io; Yao, Jiewen
> ; Kinney, Michael D 
> Cc: Alex Williamson ; devel@edk2.groups.io;
> qemu devel list ; Igor Mammedov
> ; Chen, Yingwen ;
> Nakajima, Jun ; Boris Ostrovsky
> ; Joao Marcal Lemos Martins
> ; Phillip Goerl 
> Subject: RE: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with
> QEMU+OVMF
> 
> Paolo,
> 
> I find the following links related to the discussions here
> along with one example feature called GENPROTRANGE.
> 
> https://csrc.nist.gov/CSRC/media/Presentations/The-Whole-is-Greater/ima
> ges-media/day1_trusted-computing_200-250.pdf
> https://cansecwest.com/slides/2017/CSW2017_Cuauhtemoc-Rene_CPU_Ho
> t-Add_flow.pdf
> https://www.mouser.com/ds/2/612/5520-5500-chipset-ioh-datasheet-1131
> 292.pdf
> 
> Best regards,
> 
> Mike
> 
> > -Original Message-
> > From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> > Sent: Thursday, August 22, 2019 4:12 PM
> > To: Kinney, Michael D ;
> > Laszlo Ersek ; r...@edk2.groups.io;
> > Yao, Jiewen 
> > Cc: Alex Williamson ;
> > devel@edk2.groups.io; qemu devel list  > de...@nongnu.org>; Igor Mammedov ;
> > Chen, Yingwen ; Nakajima, Jun
> > ; Boris Ostrovsky
> > ; Joao Marcal Lemos Martins
> > ; Phillip Goerl
> > 
> > Subject: Re: [edk2-rfc] [edk2-devel] CPU hotplug using
> > SMM with QEMU+OVMF
> >
> > On 23/08/19 00:32, Kinney, Michael D wrote:
> > > Paolo,
> > >
> > > It is my understanding that real HW hot plug uses the
> > SDM defined
> > > methods.  Meaning the initial SMI is to 3000:8000 and
> > they rebase to
> > > TSEG in the first SMI.  They must have chipset specific
> > methods to
> > > protect 3000:8000 from DMA.
> >
> > It would be great if you could check.
> >
> > > Can we add a chipset feature to prevent DMA to 64KB
> > range from
> > > 0x3-0x3 and the UEFI Memory Map and ACPI
> > content can be
> > > updated so the Guest OS knows to not use that range for
> > DMA?
> >
> > If real hardware does it at the chipset level, we will
> > probably use Igor's suggestion of aliasing A-seg to
> > 3000:.  Before starting the new CPU, the SMI handler
> > can prepare the SMBASE relocation trampoline at
> > A000:8000 and the hot-plugged CPU will find it at
> > 3000:8000 when it receives the initial SMI.  Because this
> > is backed by RAM at 0xA-0xA, DMA cannot access it
> > and would still go through to RAM at 0x3.
> >
> > Paolo

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

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

Re: [edk2-devel] [edk2-test][Patch 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test

[Eric Jin]

Hi Supreeth,

> -Original Message-
> From: Supreeth Venkatesh [mailto:supreeth.venkat...@arm.com]
> Sent: Friday, August 23, 2019 2:43 AM
> To: devel@edk2.groups.io; Jin, Eric 
> Subject: Re: [edk2-devel] [edk2-test][Patch 1/1] uefi-sct/SctPkg: Eliminate
> 2nd execution of ExitBootServices Test
> 
> On Wed, 2019-08-21 at 20:50 -0500, Eric Jin via Groups.Io wrote:
> > Hij Supreeth,
> Hi Eric,
> 
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > > Of Supreeth Venkatesh
> > > Sent: Thursday, August 22, 2019 12:43 AM
> > > To: Jin, Eric ; devel@edk2.groups.io
> > > Subject: Re: [edk2-devel] [edk2-test][Patch 1/1] uefi-sct/SctPkg:
> > > Eliminate
> > > 2nd execution of ExitBootServices Test
> > >
> > > On Wed, 2019-08-21 at 01:24 -0500, Eric Jin wrote:
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2098
> > >
> > > Please add the details of the patch to the commit message.
> > > "In the ExitBootServices() test, after ExitBootServices() call, all
> > > boot services are forbidden. The original design is to save the
> > > return status value of
> > > ExitBootServices() in variable using variable service and reset, but
> > > this needs multiple execution of the test to retrieve the value from
> > > variable and this design was not straightforward from end user
> > > perspective.
> > >
> >
> > I would like to change "multiple execution" to "one additional
> > execution"
> >
> > > This patch enhances the test by leveraging RecoveryLib to restore
> > > execution
> > > after reset automatically, thus requiring only one execution"
> > >
> >
> > I am ok with this suggestion when I push the code to repo
> Thanks.
> 
> >
> > > More comments inline...
> > >
> > > >
> > > > Cc: Supreeth Venkatesh 
> > > > Signed-off-by: Eric Jin 
> > > > ---
> > > >  uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.inf  |  3 ++-
> > > >  uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.h|  9 -
> > > >  uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTestConformance.c | 98
> > > >
> > >
> > >
> ++
> > > +++
> > > > ++---
> > > >  3 files changed, 93 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.inf b/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.inf
> > > > index 49ad79915934..3de43a20e8a4 100644
> > > > --- a/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.inf
> > > > +++ b/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.inf
> > > > @@ -1,7 +1,7 @@
> > > >  ## @file
> > > >  #
> > > >  #  Copyright 2006 - 2012 Unified EFI, Inc. -#  Copyright (c)
> > > > 2010
> > > > - 2012, Intel Corporation. All rights reserved.
> > > > +#  Copyright (c) 2010 - 2019, 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
> > > > @@
> > > > -53,4 +53,5 @@
> > > >
> > > >  [Protocols]
> > > >gEfiTestProfileLibraryGuid
> > > > +  gEfiTestRecoveryLibraryGuid
> > > >gBlackBoxEfiHIIPackageListProtocolGuid
> > > > diff --git a/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.h b/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.h
> > > > index b1c35fee7435..008584577ed1 100644
> > > > --- a/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.h
> > > > +++ b/uefi-
> > > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > > est/
> > > > ImageBBTest.h
> > > > @@ -1,7 +1,7 @@
> > > >  /** @file
> > > >
> > > >Copyright 2006 - 2017 Unified EFI, Inc.
> > > > -  Copyright (c) 2010 - 2017, Intel Corporation. All rights
> > > > reserved.
> > > > +  Copyright (c) 2010 - 2019, 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 @@ -35,6 +35,13 @@ Abstract:
> > > >  #include EFI_PROTOCOL_DEFINITION (LoadFile)
> > > >
> > > >  #include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
> > > > +#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
> > > > +
> > > > +typedef struct _RESET_DATA {
> > > > +  UINTN   Step;
> 

Re: [edk2-devel] [PATCH] SecurityPkg: Don't Verify the enrolled PK in setup mode

[Lin, Derek (HPS SW)]

Hi Laszlo, Chao,

Sorry for late response in this thread.

I review Mantis#1983 and this discussion again. I agree with Laszlo.
1. UEFI spec 2.8 is not very clear about PK validation in Setup mode.
2. This patch only reduce the complexity of update PK process.

Having a FeaturePCD to control this kind of behavior in EDK2 is weird. That 
only make things more complicated to me.
To simplify and make things clear, updating PK shall always be signed in both 
Setup Mode and User Mode.

Anyway, I agree with Laszlo and I'm good with current implementation now.

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

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

Re: [edk2-devel] [PATCH v5 0/4] Add SCSI Support for Storage Security Command Protocol

[Wu, Hao A]

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zurcher, Christopher J
> Sent: Friday, August 23, 2019 6:02 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D; Yao, Jiewen; Wang, Jian J; Gao, Liming; Wu, Hao A
> Subject: [edk2-devel] [PATCH v5 0/4] Add SCSI Support for Storage Security
> Command Protocol
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1546
> 
> V5 changes:
> Split MdeModulePkg changes into separate patches. Add ReinstallProtocol
> calls for StorageSecurityCommand Protocol. Restore original MediaPresent
> and ReadCapacity behavior, with new implementation for WLUN type media.
> Clear temporary aligned buffers before freeing them.


Thanks for the series.

Please grant me some time for reviewing the patches, I will give my
feedbacks before the end of next week.

Best Regards,
Hao Wu


> 
> V4 changes:
> Add SSC Protocol in addition to BlockIo instead of in place of BlockIo.
> Add error handling for (BlockSize == 0) in Read and WriteBlocks commands
> to handle partitions that do not support ReadCapacity().
> 
> V3 changes:
> Initialize AlignedBuffer variable in ScsiDiskReceiveData and
> ScsiDiskSendData functions. Remove redundant input validation and debug
> message in ScsiDiskSendData.
> 
> V2 changes:
> Split the patch into separate commits for separate packages.
> 
> To support RPMB access on UFS devices, support must be added to
> the ScsiDiskDxe driver for the Storage Security Command Protocol.
> 
> Cc: Michael D Kinney 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Cc: Hao A Wu 
> 
> Christopher J Zurcher (4):
>   MdePkg: Implement SCSI commands for Security Protocol In/Out
>   MdeModulePkg/UfsPassThruDxe: Check for RPMB W-LUN (SecurityLun)
>   MdeModulePkg/ScsiBusDxe: Clean up Peripheral Type check
>   MdeModulePkg/ScsiDiskDxe: Support Storage Security Command Protocol
> 
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf |   3 +-
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h  | 171 +-
>  MdePkg/Include/IndustryStandard/Scsi.h|  48 +-
>  MdePkg/Include/Library/UefiScsiLib.h  | 126 +++-
>  MdePkg/Include/Protocol/ScsiIo.h  |   9 +-
>  MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c|   5 +-
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c  | 614
> +++-
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c |  17 +-
>  MdePkg/Library/UefiScsiLib/UefiScsiLib.c  | 205 ++-
>  9 files changed, 1157 insertions(+), 41 deletions(-)
> 
> --
> 2.16.2.windows.1
> 
> 
> 


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

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

[edk2-devel] Upcoming Event: TianoCore Design Meeting - APAC/NAMO - Thu, 08/22/2019 6:30pm-7:30pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Design Meeting - APAC/NAMO

*When:* Thursday, 22 August 2019, 6:30pm to 7:30pm, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/969264410

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=470780 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Design%20Meeting%20-%20APAC%2FNAMO
 )

*Description:*

*TOPICS*
*1. Re-architecture of Intel edk2 Redfish POC code on edk2 staging (HPE)
* *https://edk2.groups.io/g/devel/files/Designs/2019/0823*

Join Zoom Meeting

https://zoom.us/j/969264410 ( https://zoom.us/j/969264410 )

One tap mobile

+16465588656,,969264410# US (New York)

+17207072699,,969264410# US

Dial by your location

+1 646 558 8656 US (New York)

+1 720 707 2699 US

Meeting ID: 969 264 410

Find your local number: https://zoom.us/u/abOtdJckxL

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

View/Reply Online (#46249): https://edk2.groups.io/g/devel/message/46249
Mute This Topic: https://groups.io/mt/32996683/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Michael D Kinney]

Paolo,

I find the following links related to the discussions here
along with one example feature called GENPROTRANGE.

https://csrc.nist.gov/CSRC/media/Presentations/The-Whole-is-Greater/images-media/day1_trusted-computing_200-250.pdf
https://cansecwest.com/slides/2017/CSW2017_Cuauhtemoc-Rene_CPU_Hot-Add_flow.pdf
https://www.mouser.com/ds/2/612/5520-5500-chipset-ioh-datasheet-1131292.pdf

Best regards,

Mike

> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, August 22, 2019 4:12 PM
> To: Kinney, Michael D ;
> Laszlo Ersek ; r...@edk2.groups.io;
> Yao, Jiewen 
> Cc: Alex Williamson ;
> devel@edk2.groups.io; qemu devel list  de...@nongnu.org>; Igor Mammedov ;
> Chen, Yingwen ; Nakajima, Jun
> ; Boris Ostrovsky
> ; Joao Marcal Lemos Martins
> ; Phillip Goerl
> 
> Subject: Re: [edk2-rfc] [edk2-devel] CPU hotplug using
> SMM with QEMU+OVMF
> 
> On 23/08/19 00:32, Kinney, Michael D wrote:
> > Paolo,
> >
> > It is my understanding that real HW hot plug uses the
> SDM defined
> > methods.  Meaning the initial SMI is to 3000:8000 and
> they rebase to
> > TSEG in the first SMI.  They must have chipset specific
> methods to
> > protect 3000:8000 from DMA.
> 
> It would be great if you could check.
> 
> > Can we add a chipset feature to prevent DMA to 64KB
> range from
> > 0x3-0x3 and the UEFI Memory Map and ACPI
> content can be
> > updated so the Guest OS knows to not use that range for
> DMA?
> 
> If real hardware does it at the chipset level, we will
> probably use Igor's suggestion of aliasing A-seg to
> 3000:.  Before starting the new CPU, the SMI handler
> can prepare the SMBASE relocation trampoline at
> A000:8000 and the hot-plugged CPU will find it at
> 3000:8000 when it receives the initial SMI.  Because this
> is backed by RAM at 0xA-0xA, DMA cannot access it
> and would still go through to RAM at 0x3.
> 
> Paolo

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

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

Re: [edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 08/22/2019 5:00pm-5:30pm #cal-reminder

[Michael D Kinney]

The zoom meeting started late, so please try to rejoin if you could not get in 
a few minutes ago.

Mike

From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
Sent: Thursday, August 22, 2019 4:45 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 
08/22/2019 5:00pm-5:30pm #cal-reminder


Reminder: TianoCore Bug Triage - APAC / NAMO

When: Thursday, 22 August 2019, 5:00pm to 5:30pm, (GMT-07:00) America/Los 
Angeles

Where:https://zoom.us/j/251103409

View Event

Organizer: Stephano Cetola 
stephano.cet...@intel.com

Description:
Join Zoom Meeting
https://zoom.us/j/251103409

One tap mobile
+17207072699,,251103409# US
+16465588656,,251103409# US (New York)

Dial by your location
+1 720 707 2699 US
+1 646 558 8656 US (New York)
Meeting ID: 251 103 409
Find your local number: https://zoom.us/u/acEIbOAUyA


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

View/Reply Online (#46247): https://edk2.groups.io/g/devel/message/46247
Mute This Topic: https://groups.io/mt/32995830/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Upcoming Event: TianoCore Bug Triage - APAC / NAMO - Thu, 08/22/2019 5:00pm-5:30pm #cal-reminder

[devel@edk2.groups.io Calendar]

*Reminder:* TianoCore Bug Triage - APAC / NAMO

*When:* Thursday, 22 August 2019, 5:00pm to 5:30pm, (GMT-07:00) America/Los 
Angeles

*Where:* https://zoom.us/j/251103409

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=470778 )

*Organizer:* Stephano Cetola stephano.cet...@intel.com ( 
stephano.cet...@intel.com?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

*Description:*

Join Zoom Meeting

https://zoom.us/j/251103409 ( https://zoom.us/j/251103409 )

One tap mobile

+17207072699,,251103409# US

+16465588656,,251103409# US (New York)

Dial by your location

+1 720 707 2699 US

+1 646 558 8656 US (New York)

Meeting ID: 251 103 409

Find your local number: https://zoom.us/u/acEIbOAUyA ( 
https://zoom.us/u/acEIbOAUyA )

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

View/Reply Online (#46246): https://edk2.groups.io/g/devel/message/46246
Mute This Topic: https://groups.io/mt/32995830/21656
Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch][edk2-stable201908 2/2] EmulatorPkg/Win/Host: Fix SecPrint() log line endings

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael D
> Kinney
> Sent: Thursday, August 22, 2019 10:36 AM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L ; Ni, Ray ;
> Andrew Fish ; Tim Lewis 
> Subject: [edk2-devel] [Patch][edk2-stable201908 2/2] EmulatorPkg/Win/Host:
> Fix SecPrint() log line endings
> 
> Update use of SecPrint() to consistently use \n\r for
> line endings to fix formatting issues in the debug log.
> 
> Cc: Jordan Justen 
> Cc: Ray Ni 
> Cc: Andrew Fish 
> Cc: Tim Lewis 
> Signed-off-by: Michael D Kinney 
> ---
>  EmulatorPkg/Win/Host/WinHost.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/EmulatorPkg/Win/Host/WinHost.c
> b/EmulatorPkg/Win/Host/WinHost.c
> index 9c6acac279..9aba3c8959 100644
> --- a/EmulatorPkg/Win/Host/WinHost.c
> +++ b/EmulatorPkg/Win/Host/WinHost.c
> @@ -408,7 +408,7 @@ Returns:
>MemorySizeStr  = (CHAR16 *) PcdGetPtr (PcdEmuMemorySize);
>FirmwareVolumesStr = (CHAR16 *) PcdGetPtr (PcdEmuFirmwareVolume);
> 
> -  SecPrint ("\nEDK II WIN Host Emulation Environment from
> http://www.tianocore.org/edk2/\n;);
> +  SecPrint ("\n\rEDK II WIN Host Emulation Environment from
> http://www.tianocore.org/edk2/\n\r;);
> 
>//
>// Determine the first thread available to this process.
> @@ -450,7 +450,7 @@ Returns:
>gSystemMemoryCount  = CountSeparatorsInString (MemorySizeStr, '!') + 1;
>gSystemMemory   = calloc (gSystemMemoryCount, sizeof
> (NT_SYSTEM_MEMORY));
>if (gSystemMemory == NULL) {
> -SecPrint ("ERROR : Can not allocate memory for %S.  Exiting.\n",
> MemorySizeStr);
> +SecPrint ("ERROR : Can not allocate memory for %S.  Exiting.\n\r",
> MemorySizeStr);
>  exit (1);
>}
> 
> @@ -460,13 +460,13 @@ Returns:
>gFdInfoCount  = CountSeparatorsInString (FirmwareVolumesStr, '!') + 1;
>gFdInfo   = calloc (gFdInfoCount, sizeof (NT_FD_INFO));
>if (gFdInfo == NULL) {
> -SecPrint ("ERROR : Can not allocate memory for %S.  Exiting.\n",
> FirmwareVolumesStr);
> +SecPrint ("ERROR : Can not allocate memory for %S.  Exiting.\n\r",
> FirmwareVolumesStr);
>  exit (1);
>}
>//
>// Setup Boot Mode.
>//
> -  SecPrint ("  BootMode 0x%02x\n", PcdGet32 (PcdEmuBootMode));
> +  SecPrint ("  BootMode 0x%02x\n\r", PcdGet32 (PcdEmuBootMode));
> 
>//
>//  Allocate 128K memory to emulate temp memory for PEI.
> @@ -476,12 +476,12 @@ Returns:
>TemporaryRamSize = TEMPORARY_RAM_SIZE;
>TemporaryRam = VirtualAlloc (NULL, (SIZE_T) (TemporaryRamSize),
> MEM_COMMIT, PAGE_EXECUTE_READWRITE);
>if (TemporaryRam == NULL) {
> -SecPrint ("ERROR : Can not allocate enough space for SecStack\n");
> +SecPrint ("ERROR : Can not allocate enough space for SecStack\n\r");
>  exit (1);
>}
>SetMem32 (TemporaryRam, TemporaryRamSize, PcdGet32
> (PcdInitValueInTempStack));
> 
> -  SecPrint ("  OS Emulator passing in %u KB of temp RAM at 0x%08lx to SEC\n",
> +  SecPrint ("  OS Emulator passing in %u KB of temp RAM at 0x%08lx to 
> SEC\n\r",
>  TemporaryRamSize / SIZE_1KB,
>  TemporaryRam
>  );
> @@ -503,7 +503,7 @@ Returns:
>
>);
>  if (EFI_ERROR (Status)) {
> -  SecPrint ("ERROR : Could not allocate PeiServicesTablePage @ %p\n",
> EmuMagicPage);
> +  SecPrint ("ERROR : Could not allocate PeiServicesTablePage @ %p\n\r",
> EmuMagicPage);
>return EFI_DEVICE_ERROR;
>  }
>}
> @@ -514,7 +514,7 @@ Returns:
>//
>FileNamePtr = AllocateCopyPool (StrSize (FirmwareVolumesStr),
> FirmwareVolumesStr);
>if (FileNamePtr == NULL) {
> -SecPrint ("ERROR : Can not allocate memory for firmware volume 
> string\n");
> +SecPrint ("ERROR : Can not allocate memory for firmware volume
> string\n\r");
>  exit (1);
>}
> 
> @@ -540,11 +540,11 @@ Returns:
>[Index].Size
>);
>  if (EFI_ERROR (Status)) {
> -  SecPrint ("ERROR : Can not open Firmware Device File %S (0x%X).
> Exiting.\n", FileName, Status);
> +  SecPrint ("ERROR : Can not open Firmware Device File %S (0x%X).
> Exiting.\n\r", FileName, Status);
>exit (1);
>  }
> 
> -SecPrint ("  FD loaded from %S\n", FileName);
> +SecPrint ("  FD loaded from %S", FileName);
> 
>  if (SecFile == NULL) {
>//
> @@ -565,7 +565,7 @@ Returns:
>}
>  }
> 
> -SecPrint ("\n");
> +SecPrint ("\n\r");
>}
>//
>// Calculate memory regions and store the information in the gSystemMemory
> @@ -590,7 +590,7 @@ Returns:
>  MemorySizeStr = MemorySizeStr + Index1 + 1;
>}
> 
> -  SecPrint ("\n");
> +  SecPrint ("\n\r");
> 
>//
>// Hand off to SEC Core
> @@ -601,7 +601,7 @@ Returns:
>// If we get here, then the SEC Core returned. This is an error as SEC 
> should
>//  always hand off to PEI Core and then on to DXE Core.
>//
> -  SecPrint ("ERROR : SEC returned\n");
> +  

Re: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Host: Fix image unload regression

[Ni, Ray]

Mike,
Thanks for fixing this regression issue.

I also did a comparison between this and the Nt32 accordingly code.
They are almost the same.

I also noticed your unit test steps in Bugzilla and the behavior is expected.

I agree and also suggest that this fix to be included in the coming stable tag 
release.

Reviewed-by: Ray Ni 



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael D
> Kinney
> Sent: Thursday, August 22, 2019 10:36 AM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L ; Ni, Ray ;
> Andrew Fish ; Tim Lewis 
> Subject: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Host:
> Fix image unload regression
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2104
> 
> When UEFI Applications or UEFI Drivers are unloaded, the
> PeCoffLoaderUnloadImageExtraAction() needs to unload the image using
> FreeLibrary() if the image was successfully loaded using LoadLibrrayEx().
> 
> This is a regression from the Nt32Pkg that supported unloading applications 
> and
> drivers as well as loading the same application or driver multiple times.
> 
> Cc: Jordan Justen 
> Cc: Ray Ni 
> Cc: Andrew Fish 
> Cc: Tim Lewis 
> Signed-off-by: Michael D Kinney 
> ---
>  EmulatorPkg/Win/Host/WinHost.c | 167
> +++--
>  1 file changed, 161 insertions(+), 6 deletions(-)
> 
> diff --git a/EmulatorPkg/Win/Host/WinHost.c
> b/EmulatorPkg/Win/Host/WinHost.c index dd52075f98..9c6acac279 100644
> --- a/EmulatorPkg/Win/Host/WinHost.c
> +++ b/EmulatorPkg/Win/Host/WinHost.c
> @@ -19,6 +19,25 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define SE_TIME_ZONE_NAME TEXT("SeTimeZonePrivilege")
>  #endif
> 
> +//
> +// The growth size for array of module handle entries // #define
> +MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE 0x100
> +
> +//
> +// Module handle entry structure
> +//
> +typedef struct {
> +  CHAR8   *PdbPointer;
> +  VOID*ModHandle;
> +} PDB_NAME_TO_MOD_HANDLE;
> +
> +//
> +// An Array to hold the module handles
> +//
> +PDB_NAME_TO_MOD_HANDLE  *mPdbNameModHandleArray = NULL;
> +UINTN   mPdbNameModHandleArraySize = 0;
> +
>  //
>  // Default information about where the FD is located.
>  //  This array gets filled in with information from PcdWinNtFirmwareVolume
> @@ -840,6 +859,120 @@ Returns:
>return Count;
>  }
> 
> +/**
> +  Store the ModHandle in an array indexed by the Pdb File name.
> +  The ModHandle is needed to unload the image.
> +  @param ImageContext - Input data returned from PE Laoder Library. Used to
> find the
> + .PDB file name of the PE Image.
> +  @param ModHandle- Returned from LoadLibraryEx() and stored for call to
> + FreeLibrary().
> +  @return   return EFI_SUCCESS when ModHandle was stored.
> +--*/
> +EFI_STATUS
> +AddModHandle (
> +  IN  PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext,
> +  IN  VOID *ModHandle
> +  )
> +
> +{
> +  UINTN   Index;
> +  PDB_NAME_TO_MOD_HANDLE  *Array;
> +  UINTN   PreviousSize;
> +  PDB_NAME_TO_MOD_HANDLE  *TempArray;
> +  HANDLE  Handle;
> +  UINTN   Size;
> +
> +  //
> +  // Return EFI_ALREADY_STARTED if this DLL has already been loaded  //
> + Array = mPdbNameModHandleArray;  for (Index = 0; Index <
> + mPdbNameModHandleArraySize; Index++, Array++) {
> +if (Array->PdbPointer != NULL && Array->ModHandle == ModHandle) {
> +  return EFI_ALREADY_STARTED;
> +}
> +  }
> +
> +  Array = mPdbNameModHandleArray;
> +  for (Index = 0; Index < mPdbNameModHandleArraySize; Index++, Array++) {
> +if (Array->PdbPointer == NULL) {
> +  //
> +  // Make a copy of the stirng and store the ModHandle
> +  //
> +  Handle = GetProcessHeap ();
> +  Size = AsciiStrLen (ImageContext->PdbPointer) + 1;
> +  Array->PdbPointer = HeapAlloc ( Handle, HEAP_ZERO_MEMORY, Size);
> +  ASSERT (Array->PdbPointer != NULL);
> +
> +  AsciiStrCpyS (Array->PdbPointer, Size, ImageContext->PdbPointer);
> +  Array->ModHandle = ModHandle;
> +  return EFI_SUCCESS;
> +}
> +  }
> +
> +  //
> +  // No free space in mPdbNameModHandleArray so grow it by  //
> + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE entires.
> +  //
> +  PreviousSize = mPdbNameModHandleArraySize * sizeof
> + (PDB_NAME_TO_MOD_HANDLE);  mPdbNameModHandleArraySize +=
> + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE;
> +  //
> +  // re-allocate a new buffer and copy the old values to the new locaiton.
> +  //
> +  TempArray = HeapAlloc (GetProcessHeap (),
> +HEAP_ZERO_MEMORY,
> +mPdbNameModHandleArraySize * sizeof
> (PDB_NAME_TO_MOD_HANDLE)
> +   );
> +
> +  CopyMem ((VOID *) (UINTN) TempArray, (VOID *)
> + (UINTN)mPdbNameModHandleArray, PreviousSize);
> +
> +  HeapFree (GetProcessHeap (), 0, mPdbNameModHandleArray);
> +
> +  mPdbNameModHandleArray = TempArray;
> +  if 

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Michael D Kinney]

Paolo,

It is my understanding that real HW hot plug uses the SDM defined
methods.  Meaning the initial SMI is to 3000:8000 and they rebase
to TSEG in the first SMI.  They must have chipset specific methods
to protect 3000:8000 from DMA.

Can we add a chipset feature to prevent DMA to 64KB range from
0x3-0x3 and the UEFI Memory Map and ACPI content can be
updated so the Guest OS knows to not use that range for DMA?

Thanks,

Mike

> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, August 22, 2019 3:18 PM
> To: Kinney, Michael D ;
> Laszlo Ersek ; r...@edk2.groups.io;
> Yao, Jiewen 
> Cc: Alex Williamson ;
> devel@edk2.groups.io; qemu devel list  de...@nongnu.org>; Igor Mammedov ;
> Chen, Yingwen ; Nakajima, Jun
> ; Boris Ostrovsky
> ; Joao Marcal Lemos Martins
> ; Phillip Goerl
> 
> Subject: Re: [edk2-rfc] [edk2-devel] CPU hotplug using
> SMM with QEMU+OVMF
> 
> On 22/08/19 22:06, Kinney, Michael D wrote:
> > The SMBASE register is internal and cannot be directly
> accessed by any
> > CPU.  There is an SMBASE field that is member of the
> SMM Save State
> > area and can only be modified from SMM and requires the
> execution of
> > an RSM instruction from SMM for the SMBASE register to
> be updated from
> > the current SMBASE field value.  The new SMBASE
> register value is only
> > used on the next SMI.
> 
> Actually there is also an SMBASE MSR, even though in
> current silicon it's read-only and its use is
> theoretically limited to SMM-transfer monitors.  If that
> MSR could be made accessible somehow outside SMM, that
> would be great.
> 
> > Once all the CPUs have been initialized for SMM, the
> CPUs that are not
> > needed can be hot removed.  As noted above, the SMBASE
> value does not
> > change on an INIT.  So as long as the hot add operation
> does not do a
> > RESET, the SMBASE value must be preserved.
> 
> IIRC, hot-remove + hot-add will unplugs/plugs a
> completely different CPU.
> 
> > Another idea is to emulate this behavior.  If the hot
> plug controller
> > provide registers (only accessible from SMM) to assign
> the SMBASE
> > address for every CPU.  When a CPU is hot added, QEMU
> can set the
> > internal SMBASE register value from the hot plug
> controller register
> > value.  If the SMM Monarch sends an INIT or an SMI from
> the Local APIC
> > to the hot added CPU, then the SMBASE register should
> not be modified
> > and the CPU starts execution within TSEG the first time
> it receives an SMI.
> 
> Yes, this would work.  But again---if the issue is real
> on current hardware too, I'd rather have a matching
> solution for virtual platforms.
> 
> If the current hardware for example remembers INIT-
> preserved across hot-remove/hot-add, we could emulate
> that.
> 
> I guess the fundamental question is: how do bare metal
> platforms avoid this issue, or plan to avoid this issue?
> Once we know that, we can use that information to find a
> way to implement it in KVM.  Only if it is impossible
> we'll have a different strategy that is specific to our
> platform.
> 
> Paolo
> 
> > Jiewen and I can collect specific questions on this
> topic and continue
> > the discussion here.  For example, I do not think there
> is any method
> > other than what I referenced above to program the
> SMBASE register, but
> > I can ask if there are any other methods.


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

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

Re: [edk2-devel] [edk2-platform patch 1/7] SimicsX58SktPkg: Add CPU Pkg for SimicsX58

[David Wei]

Hi Mike,
Please see the updates online below. Please let me know if you have any more 
comments.

Thanks
David

-Original Message-
From: Kubacki, Michael A 
Sent: Monday, August 19, 2019 6:04 PM
To: Wei, David Y ; devel@edk2.groups.io
Cc: Wu, Hao A ; Gao, Liming ; Sinha, 
Ankit ; Agyeman, Prince ; 
Desimone, Nathaniel L ; Kinney, Michael D 

Subject: RE: [edk2-platform patch 1/7] SimicsX58SktPkg: Add CPU Pkg for 
SimicsX58

Feedback I could not find already noted elsewhere:

1. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c:
 There's some style issues throughout the file related to a missing space 
before opening parenthesis and parameters than span multiple lines.
 Some of these are pre-existing in OvmfPkg source. I don't think this is a 
must have.
Ydwei: done
2. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c - Line 265:
 "Confirm if QEMU supports SMRAM." to "Confirm if Simics supports SMRAM."
Ydwei: done
3. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c - Line 275:
 "DEBUG((EFI_D_ERROR, "TopOfLowRam =0x%x; TopOfLowRamMb =0x%x \n", 
TopOfLowRam, TopOfLowRamMb));"
 Should be an informational message.
Ydwei: done
3. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c - Line 283:
 "DEBUG((EFI_D_ERROR, "MCH_TOLUD =0x%x; \n", 
PciRead32(DRAMC_REGISTER_X58(MCH_TOLUD;"
 Should be an informational message.
Ydwei: done
4. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c - Line 300 & Line 
301 - Should be informational messages as well.
Ydwei: done
5. Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf: Is a dependency 
on OvmfPkg/OvmfPkg.dec required? This dependency should be avoided.
Ydwei: done
6. Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore: I don't think 
this should be an override but should exist in a similar manner
 as edk2/OvmfPkg/Sec. The code cannot be upstreamed as-is.
Ydwei: done
7. /Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore/SecMain.c: Some 
of this messages should probably be verbose not informational.
Ydwei: done
8. /Silicon/Intel/SimicsX58SktPkg/SktPei.dsc: This file naming convention is 
unusual. SktPkgPei.dsc is more consistent. 
Ydwei: done
> -Original Message-
> From: Wei, David Y
> Sent: Friday, August 9, 2019 3:47 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A ; Gao, Liming ;
> Sinha, Ankit ; Agyeman, Prince
> ; Kubacki, Michael A
> ; Desimone, Nathaniel L
> ; Kinney, Michael D
> 
> Subject: [edk2-platform patch 1/7] SimicsX58SktPkg: Add CPU Pkg for
> SimicsX58
> 
> Add CPU Pkg for SimicsX58. It is added for simics QSP project support
> 
> Cc: Hao Wu 
> Cc: Liming Gao 
> Cc: Ankit Sinha 
> Cc: Agyeman Prince 
> Cc: Kubacki Michael A 
> Cc: Nate DeSimone 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Signed-off-by: David Wei 
> ---
>  .../Override/UefiCpuPkg/SecCore/SecMain.c  | 956
> +
>  .../SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c | 148 
>  .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c  | 353 
>  .../SimicsX58SktPkg/Smm/Access/SmramInternal.c | 199 +
>  .../Override/UefiCpuPkg/SecCore/Ia32/SecEntry.nasm |  45 +
>  .../Override/UefiCpuPkg/SecCore/SecMain.inf|  71 ++
>  .../Override/UefiCpuPkg/SecCore/X64/SecEntry.nasm  |  45 +
>  Silicon/Intel/SimicsX58SktPkg/SktPei.dsc   |  18 +
>  .../Intel/SimicsX58SktPkg/SktPostMemoryInclude.fdf |   9 +
>  .../Intel/SimicsX58SktPkg/SktPreMemoryInclude.fdf  |  10 +
>  Silicon/Intel/SimicsX58SktPkg/SktSecInclude.fdf|  17 +
>  .../Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf   |  16 +
>  .../SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf   |  52 ++
>  .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf|  64 ++
>  .../SimicsX58SktPkg/Smm/Access/SmramInternal.h |  81 ++
>  15 files changed, 2084 insertions(+)
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore/SecMain.c
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore/Ia32/SecEntry.nas
> m
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore/SecMain.inf
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Override/UefiCpuPkg/SecCore/X64/SecEntry.nas
> m
>  create mode 100644 Silicon/Intel/SimicsX58SktPkg/SktPei.dsc
>  create mode 100644 Silicon/Intel/SimicsX58SktPkg/SktPostMemoryInclude.fdf
>  create mode 100644 Silicon/Intel/SimicsX58SktPkg/SktPreMemoryInclude.fdf
>  create mode 100644 Silicon/Intel/SimicsX58SktPkg/SktSecInclude.fdf
>  create mode 100644 Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
>  create mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
>  create mode 100644
> 

[edk2-devel] [PATCH v5 3/4] MdeModulePkg/ScsiBusDxe: Clean up Peripheral Type check

[Zurcher, Christopher J]

Replacing "magic numbers" in the Peripheral Type check with defines for
the reserved range from IndustryStandard/Scsi.h

Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Christopher J Zurcher 
---
 MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c 
b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
index c4069aec0f..1caffd38cd 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
@@ -2,7 +2,7 @@
   SCSI Bus driver that layers on every SCSI Pass Thru and
   Extended SCSI Pass Thru protocol in the system.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -1368,7 +1368,8 @@ DiscoverScsiDevice (
 goto Done;
   }
 
-  if (0x1e >= InquiryData->Peripheral_Type && InquiryData->Peripheral_Type >= 
0xa) {
+  if ((InquiryData->Peripheral_Type >= EFI_SCSI_TYPE_RESERVED_LOW) &&
+  (InquiryData->Peripheral_Type <= EFI_SCSI_TYPE_RESERVED_HIGH)) {
 ScsiDeviceFound = FALSE;
 goto Done;
   }
-- 
2.16.2.windows.1


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

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

[edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI commands for Security Protocol In/Out

[Zurcher, Christopher J]

This patch implements the Security Protocol In and Security Protocol Out
commands in UefiScsiLib to prepare support for the Storage Security
Command Protocol.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Christopher J Zurcher 
---
 MdePkg/Include/IndustryStandard/Scsi.h   |  48 +++--
 MdePkg/Include/Library/UefiScsiLib.h | 126 +++-
 MdePkg/Include/Protocol/ScsiIo.h |   9 +-
 MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 205 +++-
 4 files changed, 366 insertions(+), 22 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Scsi.h 
b/MdePkg/Include/IndustryStandard/Scsi.h
index cbe5709fe5..10d7b49ba7 100644
--- a/MdePkg/Include/IndustryStandard/Scsi.h
+++ b/MdePkg/Include/IndustryStandard/Scsi.h
@@ -1,7 +1,7 @@
 /** @file
   Support for SCSI-2 standard
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -163,6 +163,12 @@
 #define EFI_SCSI_OP_SEND_MESSAGE10  0x2a
 #define EFI_SCSI_OP_SEND_MESSAGE12  0xaa
 
+//
+// Additional commands for Secure Transactions
+//
+#define EFI_SCSI_OP_SECURITY_PROTOCOL_IN  0xa2
+#define EFI_SCSI_OP_SECURITY_PROTOCOL_OUT 0xb5
+
 //
 // SCSI Data Transfer Direction
 //
@@ -172,22 +178,30 @@
 //
 // Peripheral Device Type Definitions
 //
-#define EFI_SCSI_TYPE_DISK  0x00  ///< Direct-access device (e.g. 
magnetic disk)
-#define EFI_SCSI_TYPE_TAPE  0x01  ///< Sequential-access device (e.g. 
magnetic tape)
-#define EFI_SCSI_TYPE_PRINTER   0x02  ///< Printer device
-#define EFI_SCSI_TYPE_PROCESSOR 0x03  ///< Processor device
-#define EFI_SCSI_TYPE_WORM  0x04  ///< Write-once device (e.g. some 
optical disks)
-#define EFI_SCSI_TYPE_CDROM 0x05  ///< CD-ROM device
-#define EFI_SCSI_TYPE_SCANNER   0x06  ///< Scanner device
-#define EFI_SCSI_TYPE_OPTICAL   0x07  ///< Optical memory device (e.g. 
some optical disks)
-#define EFI_SCSI_TYPE_MEDIUMCHANGER 0x08  ///< Medium changer device (e.g. 
jukeboxes)
-#define EFI_SCSI_TYPE_COMMUNICATION 0x09  ///< Communications device
-#define EFI_SCSI_TYPE_ASCIT8_1  0x0A  ///< Defined by ASC IT8 (Graphic 
arts pre-press devices)
-#define EFI_SCSI_TYPE_ASCIT8_2  0x0B  ///< Defined by ASC IT8 (Graphic 
arts pre-press devices)
-//
-// 0Ch - 1Eh are reserved
-//
-#define EFI_SCSI_TYPE_UNKNOWN   0x1F  ///< Unknown or no device type
+#define EFI_SCSI_TYPE_DISK0x00  ///< Direct-access device (e.g. 
magnetic disk)
+#define EFI_SCSI_TYPE_TAPE0x01  ///< Sequential-access device 
(e.g. magnetic tape)
+#define EFI_SCSI_TYPE_PRINTER 0x02  ///< Printer device
+#define EFI_SCSI_TYPE_PROCESSOR   0x03  ///< Processor device
+#define EFI_SCSI_TYPE_WORM0x04  ///< Write-once device (e.g. some 
optical disks)
+#define EFI_SCSI_TYPE_CDROM   0x05  ///< CD/DVD device
+#define EFI_SCSI_TYPE_SCANNER 0x06  ///< Scanner device (obsolete)
+#define EFI_SCSI_TYPE_OPTICAL 0x07  ///< Optical memory device (e.g. 
some optical disks)
+#define EFI_SCSI_TYPE_MEDIUMCHANGER   0x08  ///< Medium changer device (e.g. 
jukeboxes)
+#define EFI_SCSI_TYPE_COMMUNICATION   0x09  ///< Communications device 
(obsolete)
+#define EFI_SCSI_TYPE_A   0x0A  ///< Obsolete
+#define EFI_SCSI_TYPE_B   0x0B  ///< Obsolete
+#define EFI_SCSI_TYPE_RAID0x0C  ///< Storage array controller 
device (e.g., RAID)
+#define EFI_SCSI_TYPE_SES 0x0D  ///< Enclosure services device
+#define EFI_SCSI_TYPE_RBC 0x0E  ///< Simplified direct-access 
device (e.g., magnetic disk)
+#define EFI_SCSI_TYPE_OCRW0x0F  ///< Optical card reader/writer 
device
+#define EFI_SCSI_TYPE_BRIDGE  0x10  ///< Bridge Controller Commands
+#define EFI_SCSI_TYPE_OSD 0x11  ///< Object-based Storage Device
+#define EFI_SCSI_TYPE_AUTOMATION  0x12  ///< Automation/Drive Interface
+#define EFI_SCSI_TYPE_SECURITYMANAGER 0x13  ///< Security manager device
+#define EFI_SCSI_TYPE_RESERVED_LOW0x14  ///< Reserved (low)
+#define EFI_SCSI_TYPE_RESERVED_HIGH   0x1D  ///< Reserved (high)
+#define EFI_SCSI_TYPE_WLUN0x1E  ///< Well known logical unit
+#define EFI_SCSI_TYPE_UNKNOWN 0x1F  ///< Unknown or no device type
 
 //
 // Page Codes for INQUIRY command
diff --git a/MdePkg/Include/Library/UefiScsiLib.h 
b/MdePkg/Include/Library/UefiScsiLib.h
index 10dd81902b..a0d99e703a 100644
--- a/MdePkg/Include/Library/UefiScsiLib.h
+++ b/MdePkg/Include/Library/UefiScsiLib.h
@@ -5,7 +5,7 @@
   for hard drive, CD and DVD devices that are the most common SCSI boot 
targets used by UEFI platforms.
   This library class depends on SCSI I/O Protocol defined in UEFI 
Specification and SCSI-2 industry standard.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights 

[edk2-devel] [PATCH v5 0/4] Add SCSI Support for Storage Security Command Protocol

[Zurcher, Christopher J]

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

V5 changes:
Split MdeModulePkg changes into separate patches. Add ReinstallProtocol
calls for StorageSecurityCommand Protocol. Restore original MediaPresent
and ReadCapacity behavior, with new implementation for WLUN type media.
Clear temporary aligned buffers before freeing them.

V4 changes:
Add SSC Protocol in addition to BlockIo instead of in place of BlockIo.
Add error handling for (BlockSize == 0) in Read and WriteBlocks commands
to handle partitions that do not support ReadCapacity().

V3 changes:
Initialize AlignedBuffer variable in ScsiDiskReceiveData and
ScsiDiskSendData functions. Remove redundant input validation and debug
message in ScsiDiskSendData.

V2 changes:
Split the patch into separate commits for separate packages.

To support RPMB access on UFS devices, support must be added to
the ScsiDiskDxe driver for the Storage Security Command Protocol.

Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Hao A Wu 

Christopher J Zurcher (4):
  MdePkg: Implement SCSI commands for Security Protocol In/Out
  MdeModulePkg/UfsPassThruDxe: Check for RPMB W-LUN (SecurityLun)
  MdeModulePkg/ScsiBusDxe: Clean up Peripheral Type check
  MdeModulePkg/ScsiDiskDxe: Support Storage Security Command Protocol

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf |   3 +-
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h  | 171 +-
 MdePkg/Include/IndustryStandard/Scsi.h|  48 +-
 MdePkg/Include/Library/UefiScsiLib.h  | 126 +++-
 MdePkg/Include/Protocol/ScsiIo.h  |   9 +-
 MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c|   5 +-
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c  | 614 +++-
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c |  17 +-
 MdePkg/Library/UefiScsiLib/UefiScsiLib.c  | 205 ++-
 9 files changed, 1157 insertions(+), 41 deletions(-)

-- 
2.16.2.windows.1


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

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

[edk2-devel] [PATCH v5 4/4] MdeModulePkg/ScsiDiskDxe: Support Storage Security Command Protocol

[Zurcher, Christopher J]

This patch implements the EFI_STORAGE_SECURITY_COMMAND_PROTOCOL in the
ScsiDiskDxe driver.

Support is currently limited to the RPMB Well-known LUN for UFS devices.

Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Christopher J Zurcher 
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf |   3 +-
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h  | 171 +-
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c  | 614 +++-
 3 files changed, 772 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
index 5500d828e9..40818e669b 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
@@ -3,7 +3,7 @@
 #  It detects the SCSI disk media and installs Block I/O and Block I/O2 
Protocol on
 #  the device handle.
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -52,6 +52,7 @@
   gEfiBlockIoProtocolGuid   ## BY_START
   gEfiBlockIo2ProtocolGuid  ## BY_START
   gEfiEraseBlockProtocolGuid## BY_START
+  gEfiStorageSecurityCommandProtocolGuid## BY_START
   gEfiScsiIoProtocolGuid## TO_START
   gEfiScsiPassThruProtocolGuid  ## TO_START
   gEfiExtScsiPassThruProtocolGuid   ## TO_START
diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
index 42c095..2d8679ec6f 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for SCSI Disk Driver.
 
-Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -22,6 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 
 #include 
@@ -38,6 +39,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #define IS_DEVICE_FIXED(a)(a)->FixedDevice ? 1 : 0
 
+#define IS_ALIGNED(addr, size)(((UINTN) (addr) & (size - 1)) == 0)
+
+#define UFS_WLUN_RPMB 0xC4
+
 typedef struct {
   UINT32MaxLbaCnt;
   UINT32MaxBlkDespCnt;
@@ -51,6 +56,8 @@ typedef struct {
 
   EFI_HANDLEHandle;
 
+  EFI_STORAGE_SECURITY_COMMAND_PROTOCOL   StorageSecurity;
+
   EFI_BLOCK_IO_PROTOCOL BlkIo;
   EFI_BLOCK_IO2_PROTOCOLBlkIo2;
   EFI_BLOCK_IO_MEDIABlkIoMedia;
@@ -95,6 +102,7 @@ typedef struct {
 #define SCSI_DISK_DEV_FROM_BLKIO(a)  CR (a, SCSI_DISK_DEV, BlkIo, 
SCSI_DISK_DEV_SIGNATURE)
 #define SCSI_DISK_DEV_FROM_BLKIO2(a)  CR (a, SCSI_DISK_DEV, BlkIo2, 
SCSI_DISK_DEV_SIGNATURE)
 #define SCSI_DISK_DEV_FROM_ERASEBLK(a)  CR (a, SCSI_DISK_DEV, EraseBlock, 
SCSI_DISK_DEV_SIGNATURE)
+#define SCSI_DISK_DEV_FROM_STORSEC(a)  CR (a, SCSI_DISK_DEV, StorageSecurity, 
SCSI_DISK_DEV_SIGNATURE)
 
 #define SCSI_DISK_DEV_FROM_DISKINFO(a) CR (a, SCSI_DISK_DEV, DiskInfo, 
SCSI_DISK_DEV_SIGNATURE)
 
@@ -638,6 +646,151 @@ ScsiDiskEraseBlocks (
   );
 
 
+/**
+  Send a security protocol command to a device that receives data and/or the 
result
+  of one or more commands sent by SendData.
+
+  The ReceiveData function sends a security protocol command to the given 
MediaId.
+  The security protocol command sent is defined by SecurityProtocolId and 
contains
+  the security protocol specific data SecurityProtocolSpecificData. The 
function
+  returns the data from the security protocol command in PayloadBuffer.
+
+  For devices supporting the SCSI command set, the security protocol command 
is sent
+  using the SECURITY PROTOCOL IN command defined in SPC-4.
+
+  If PayloadBufferSize is too small to store the available data from the 
security
+  protocol command, the function shall copy PayloadBufferSize bytes into the
+  PayloadBuffer and return EFI_WARN_BUFFER_TOO_SMALL.
+
+  If PayloadBuffer or PayloadTransferSize is NULL and PayloadBufferSize is 
non-zero,
+  the function shall return EFI_INVALID_PARAMETER.
+
+  If the given MediaId does not support security protocol commands, the 
function shall
+  return EFI_UNSUPPORTED. If there is no media in the device, the function 
returns
+  EFI_NO_MEDIA. If the MediaId is not the ID for the current media in the 
device,
+  the function returns EFI_MEDIA_CHANGED.
+
+  If the security protocol fails to complete within the Timeout period, the 
function
+  shall return EFI_TIMEOUT.
+
+  If the security protocol command completes without an error, the function 
shall
+  return EFI_SUCCESS. If the security protocol command completes with an 
error, the
+  function shall return EFI_DEVICE_ERROR.
+
+  @param  This  

[edk2-devel] [PATCH v5 2/4] MdeModulePkg/UfsPassThruDxe: Check for RPMB W-LUN (SecurityLun)

[Zurcher, Christopher J]

Currently UfsPassThru only checks for 8 common LUNs. This adds a check
for the RPMB Well-known LUN and sets the corresponding bit-mask. Further
handling of the WLUN is already present in the driver.

Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Christopher J Zurcher 
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c 
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index b12404aacb..26c5a8b855 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -822,7 +822,9 @@ UfsPassThruDriverBindingStart (
   UINTN UfsHcBase;
   UINT32Index;
   UFS_UNIT_DESC UnitDescriptor;
+  UFS_DEV_DESC  DeviceDescriptor;
   UINT32UnitDescriptorSize;
+  UINT32DeviceDescriptorSize;
 
   Status= EFI_SUCCESS;
   UfsHc = NULL;
@@ -916,7 +918,6 @@ UfsPassThruDriverBindingStart (
 
   //
   // Check if 8 common luns are active and set corresponding bit mask.
-  // TODO: Parse device descriptor to decide if exposing RPMB LUN to upper 
layer for authentication access.
   //
   UnitDescriptorSize = sizeof (UFS_UNIT_DESC);
   for (Index = 0; Index < 8; Index++) {
@@ -931,6 +932,20 @@ UfsPassThruDriverBindingStart (
 }
   }
 
+  //
+  // Check if RPMB WLUN is supported and set corresponding bit mask.
+  //
+  DeviceDescriptorSize = sizeof (UFS_DEV_DESC);
+  Status = UfsRwDeviceDesc (Private, TRUE, UfsDeviceDesc, 0, 0, 
, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to read device descriptor, status = %r\n", 
Status));
+  } else {
+if (DeviceDescriptor.SecurityLun == 0x1) {
+  DEBUG ((DEBUG_INFO, "UFS WLUN RPMB is supported\n"));
+  Private->Luns.BitMask |= BIT11;
+}
+  }
+
   //
   // Start the asynchronous interrupt monitor
   //
-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting

[Jordan Justen]

On 2019-08-22 06:46:07, Laszlo Ersek wrote:
> On 08/21/19 23:51, Jordan Justen wrote:
> > On 2019-08-21 07:21:25, Laszlo Ersek wrote:
> >> On 08/19/19 23:35, Lendacky, Thomas wrote:
> >>> From: Tom Lendacky 
> >>>
> >>> +  //
> >>> +  // Enable caching
> >>> +  //
> >>> +  AsmEnableCache ();
> >>> +
> >>>DEBUG ((EFI_D_INFO,
> >>>  "SecCoreStartupWithStack(0x%x, 0x%x)\n",
> >>>  (UINT32)(UINTN)BootFv,
> >>>
> >>
> >> This makes me uncomfortable. There used to be problems related to
> >> caching when VFIO device assignment were used. My concern is admittedly
> >> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
> >> "well what could break here", I'd answer "you never know, and the burden
> >> of proof is not on me". :) Can we make this change conditional on SEV-ES?
> > 
> > This was also raised as an issue by Peter for the ACRN hypervisor and
> > Scott for the bhyve hypervisor.
> > 
> > I think it is rare for a platform to enable cache at this early of a
> > stage, but it is also rare to decompress a firmware volume at this
> > point.
> > 
> > It appears that it could be helpful to figure out how to safely enable
> > cache by default here, since it does seem to be impacting several
> > hypervisors.
> 
> I can't think of anything better than "trial and error".

Maybe we could try to detect kvm, and enable caching if !kvm.

Maybe we could enable it during the decompress of the PEI FV and
disable it afterward?

> The issues that
> used to pop up in the past, due to host kernel (KVM) changes,
> particularly in connection with VFIO device assignment, have been
> completely obscure and unpenetrable to me.

Don't we eventually enable caching during the boot, so how is VFIO not
affected by that?

> Even though I've contributed
> at least one KVM patch to mitigate those problems, they remain a mistery
> to me, and I remain unable to *reason* about the problems or the fixes.

If VFIO requires uncached access, then what mechanisms does kvm
support for accessing memory ranges uncached? I thought kvm simply
ignored the cache setting and always enabled caching, because this
section of boot is not particularly slow with kvm. But, if enabling
caching causes issues, then I guess it does something.

Does kvm support mtrr to uncache i/o ranges? I didn't think kvm
supported mtrrs in the past.

I hope we wouldn't have to use paging to disable caching for the
affected regions.

-Jordan

> So I think we could only flip the behavior (enable cache by default) and
> collect bug reports. But that's extremely annoying for end-users, and I
> see "no regressions" as one of my top responsibilities.
> 
> Even if we provided an fw_cfg knob to disable the change, using
> QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
> PlatformPei: take no-exec DXE settings from the QEMU command line",
> 2015-09-15), such fw_cfg knobs are difficult to use through layered
> products, such as libvirt, proxmox, etc. And of course fw_cfg is only
> available on QEMU.
> 
> Laszlo

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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Michael D Kinney]

Laszlo,

I believe all the code for the AP startup vector
is already in edk2.

It is a combination of the reset vector code in
UefiCpuPkg/ResetVecor/Vtf0 and an IA32/X64 specific
feature in the GenFv tool.  It sets up a 4KB aligned
location near 4GB which can be used to start an AP
using INIT-SIPI-SIPI.

DI is set to 'AP' if the processor is not the BSP.
This can be used to choose to put the APs into a
wait loop executing from the protected FLASH region.

The SMM Monarch on a hot add event can use the Local
APIC to send an INIT-SIPI-SIPI to wake the AP at the 4KB 
startup vector in FLASH.  Later the SMM Monarch
can sent use the Local APIC to send an SMI to pull the 
hot added CPU into SMM.  It is not clear if we have to
do both SIPI followed by the SMI or if we can just do
the SMI.

Best regards,

Mike

> -Original Message-
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Thursday, August 22, 2019 11:29 AM
> To: Paolo Bonzini ; Kinney,
> Michael D ;
> r...@edk2.groups.io; Yao, Jiewen 
> Cc: Alex Williamson ;
> devel@edk2.groups.io; qemu devel list  de...@nongnu.org>; Igor Mammedov ;
> Chen, Yingwen ; Nakajima, Jun
> ; Boris Ostrovsky
> ; Joao Marcal Lemos Martins
> ; Phillip Goerl
> 
> Subject: Re: [edk2-rfc] [edk2-devel] CPU hotplug using
> SMM with QEMU+OVMF
> 
> On 08/22/19 08:18, Paolo Bonzini wrote:
> > On 21/08/19 22:17, Kinney, Michael D wrote:
> >> Paolo,
> >>
> >> It makes sense to match real HW.
> >
> > Note that it'd also be fine to match some kind of
> official Intel
> > specification even if no processor (currently?)
> supports it.
> 
> I agree, because...
> 
> >> That puts us back to the reset vector and handling
> the initial SMI at
> >> 3000:8000.  That is all workable from a FW
> implementation
> >> perspective.
> 
> that would suggest that matching reset vector code
> already exists, and it would "only" need to be
> upstreamed to edk2. :)
> 
> >> It look like the only issue left is DMA.
> >>
> >> DMA protection of memory ranges is a chipset
> feature. For the current
> >> QEMU implementation, what ranges of memory are
> guaranteed to be
> >> protected from DMA?  Is it only A/B seg and TSEG?
> >
> > Yes.
> 
> (
> 
> This thread (esp. Jiewen's and Mike's messages) are the
> first time that I've heard about the *existence* of
> such RAM ranges / the chipset feature. :)
> 
> Out of interest (independently of virtualization), how
> is a general purpose OS informed by the firmware,
> "never try to set up DMA to this RAM area"? Is this
> communicated through ACPI _CRS perhaps?
> 
> ... Ah, almost: ACPI 6.2 specifies _DMA, in "6.2.4 _DMA
> (Direct Memory Access)". It writes,
> 
> For example, if a platform implements a PCI bus
> that cannot access
> all of physical memory, it has a _DMA object under
> that PCI bus that
> describes the ranges of physical memory that can be
> accessed by
> devices on that bus.
> 
> Sorry about the digression, and also about being late
> to this thread, continually -- I'm primarily following
> and learning.
> 
> )
> 
> Thanks!
> Laszlo
> 
> 


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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Michael D Kinney]

Paolo,

The SMBASE register is internal and cannot be directly accessed 
by any CPU.  There is an SMBASE field that is member of the SMM Save
State area and can only be modified from SMM and requires the
execution of an RSM instruction from SMM for the SMBASE register to
be updated from the current SMBASE field value.  The new SMBASE
register value is only used on the next SMI.

https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

Vol 3C - Section 34.11

  The default base address for the SMRAM is 3H. This value is contained in 
an internal processor register called
  the SMBASE register. The operating system or executive can relocate the SMRAM 
by setting the SMBASE field in the
  saved state map (at offset 7EF8H) to a new value (see Figure 34-4). The RSM 
instruction reloads the internal
  SMBASE register with the value in the SMBASE field each time it exits SMM. 
All subsequent SMI requests will use
  the new SMBASE value to find the starting address for the SMI handler (at 
SMBASE + 8000H) and the SMRAM state
  save area (from SMBASE + FE00H to SMBASE + H). (The processor resets the 
value in its internal SMBASE
  register to 3H on a RESET, but does not change it on an INIT.)

One idea to work around these issues is to startup OVMF with the maximum number 
of
CPUs.  All the CPUs will be assigned an SMBASE address and at a safe time to 
assign
the SMBASE values using the initial 3000:8000 SMI vector because there is a 
guarantee
of no DMA at that point in the FW init.

Once all the CPUs have been initialized for SMM, the CPUs that are not needed
can be hot removed.  As noted above, the SMBASE value does not change on
an INIT.  So as long as the hot add operation does not do a RESET, the
SMBASE value must be preserved.

Of course, this is not a good idea from a boot performance perspective, 
especially if the max CPUs is a large value.

Another idea is to emulate this behavior.  If the hot plug controller
provide registers (only accessible from SMM) to assign the SMBASE address
for every CPU.  When a CPU is hot added, QEMU can set the internal SMBASE
register value from the hot plug controller register value.  If the SMM
Monarch sends an INIT or an SMI from the Local APIC to the hot added CPU,
then the SMBASE register should not be modified and the CPU starts execution
within TSEG the first time it receives an SMI.

Jiewen and I can collect specific questions on this topic and continue
the discussion here.  For example, I do not think there is any method
other than what I referenced above to program the SMBASE register, but
I can ask if there are any other methods.

Thanks,

Mike

> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, August 22, 2019 11:43 AM
> To: Laszlo Ersek ; Kinney, Michael D
> ; r...@edk2.groups.io; Yao,
> Jiewen 
> Cc: Alex Williamson ;
> devel@edk2.groups.io; qemu devel list  de...@nongnu.org>; Igor Mammedov ;
> Chen, Yingwen ; Nakajima, Jun
> ; Boris Ostrovsky
> ; Joao Marcal Lemos Martins
> ; Phillip Goerl
> 
> Subject: Re: [edk2-rfc] [edk2-devel] CPU hotplug using
> SMM with QEMU+OVMF
> 
> On 22/08/19 19:59, Laszlo Ersek wrote:
> > The firmware and QEMU could agree on a formula, which
> would compute
> > the CPU-specific SMBASE from a value pre-programmed by
> the firmware,
> > and the initial APIC ID of the hot-added CPU.
> >
> > Yes, it would duplicate code -- the calculation --
> between QEMU and
> > edk2. While that's not optimal, it wouldn't be a first.
> 
> No, that would be unmaintainable.  The best solution to
> me seems to be to make SMBASE programmable from non-SMM
> code if some special conditions hold.  Michael, would it
> be possible to get in contact with the Intel architects?
> 
> Paolo

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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Paolo Bonzini]

On 22/08/19 20:29, Laszlo Ersek wrote:
> On 08/22/19 08:18, Paolo Bonzini wrote:
>> On 21/08/19 22:17, Kinney, Michael D wrote:
>>> DMA protection of memory ranges is a chipset feature. For the current
>>> QEMU implementation, what ranges of memory are guaranteed to be
>>> protected from DMA?  Is it only A/B seg and TSEG?
>>
>> Yes.
> 
> This thread (esp. Jiewen's and Mike's messages) are the first time that
> I've heard about the *existence* of such RAM ranges / the chipset
> feature. :)
> 
> Out of interest (independently of virtualization), how is a general
> purpose OS informed by the firmware, "never try to set up DMA to this
> RAM area"? Is this communicated through ACPI _CRS perhaps?
> 
> ... Ah, almost: ACPI 6.2 specifies _DMA, in "6.2.4 _DMA (Direct Memory
> Access)". It writes,
> 
> For example, if a platform implements a PCI bus that cannot access
> all of physical memory, it has a _DMA object under that PCI bus that
> describes the ranges of physical memory that can be accessed by
> devices on that bus.
> 
> Sorry about the digression, and also about being late to this thread,
> continually -- I'm primarily following and learning.

It's much simpler: these ranges are not in e820, for example

kernel: BIOS-e820: [mem 0x00059000-0x0008bfff] usable
kernel: BIOS-e820: [mem 0x0008c000-0x000f] reserved

The ranges are not special-cased in any way by QEMU.  Simply, AB-segs
and TSEG RAM are not part of the address space except when in SMM.
Therefore, DMA to those ranges ends up respectively to low VGA RAM[1]
and to the bit bucket.  When AB-segs are open, for example, DMA to that
area becomes possible.

Paolo

[1] old timers may remember DEF SEG=: BLOAD "foo.img",0.  It still
works with some disk device models.

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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Paolo Bonzini]

On 22/08/19 19:59, Laszlo Ersek wrote:
> The firmware and QEMU could agree on a formula, which would compute the
> CPU-specific SMBASE from a value pre-programmed by the firmware, and the
> initial APIC ID of the hot-added CPU.
> 
> Yes, it would duplicate code -- the calculation -- between QEMU and
> edk2. While that's not optimal, it wouldn't be a first.

No, that would be unmaintainable.  The best solution to me seems to be
to make SMBASE programmable from non-SMM code if some special conditions
hold.  Michael, would it be possible to get in contact with the Intel
architects?

Paolo

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

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

Re: [edk2-devel] [edk2-test][Patch 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test

[Supreeth Venkatesh]

On Wed, 2019-08-21 at 20:50 -0500, Eric Jin via Groups.Io wrote:
> Hij Supreeth,
Hi Eric,

> 
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> > Of
> > Supreeth Venkatesh
> > Sent: Thursday, August 22, 2019 12:43 AM
> > To: Jin, Eric ; devel@edk2.groups.io
> > Subject: Re: [edk2-devel] [edk2-test][Patch 1/1] uefi-sct/SctPkg:
> > Eliminate
> > 2nd execution of ExitBootServices Test
> > 
> > On Wed, 2019-08-21 at 01:24 -0500, Eric Jin wrote:
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2098
> > 
> > Please add the details of the patch to the commit message.
> > "In the ExitBootServices() test, after ExitBootServices() call, all
> > boot services
> > are forbidden. The original design is to save the return status
> > value of
> > ExitBootServices() in variable using variable service and reset,
> > but this needs
> > multiple execution of the test to retrieve the value from variable
> > and this
> > design was not straightforward from end user perspective.
> > 
> 
> I would like to change "multiple execution" to "one additional
> execution" 
> 
> > This patch enhances the test by leveraging RecoveryLib to restore
> > execution
> > after reset automatically, thus requiring only one execution"
> > 
> 
> I am ok with this suggestion when I push the code to repo
Thanks.

> 
> > More comments inline...
> > 
> > > 
> > > Cc: Supreeth Venkatesh 
> > > Signed-off-by: Eric Jin 
> > > ---
> > >  uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.inf  |  3 ++-
> > >  uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.h|  9 -
> > >  uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTestConformance.c | 98
> > > 
> > 
> > ++
> > +++
> > > ++---
> > >  3 files changed, 93 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.inf b/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.inf
> > > index 49ad79915934..3de43a20e8a4 100644
> > > --- a/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.inf
> > > +++ b/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.inf
> > > @@ -1,7 +1,7 @@
> > >  ## @file
> > >  #
> > >  #  Copyright 2006 - 2012 Unified EFI, Inc. -#  Copyright (c)
> > > 2010
> > > - 2012, Intel Corporation. All rights reserved.
> > > +#  Copyright (c) 2010 - 2019, 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
> > > @@
> > > -53,4 +53,5 @@
> > > 
> > >  [Protocols]
> > >gEfiTestProfileLibraryGuid
> > > +  gEfiTestRecoveryLibraryGuid
> > >gBlackBoxEfiHIIPackageListProtocolGuid
> > > diff --git a/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.h b/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.h
> > > index b1c35fee7435..008584577ed1 100644
> > > --- a/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.h
> > > +++ b/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTest.h
> > > @@ -1,7 +1,7 @@
> > >  /** @file
> > > 
> > >Copyright 2006 - 2017 Unified EFI, Inc.
> > > -  Copyright (c) 2010 - 2017, Intel Corporation. All rights
> > > reserved.
> > > +  Copyright (c) 2010 - 2019, 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 @@ -35,6 +35,13 @@ Abstract:
> > >  #include EFI_PROTOCOL_DEFINITION (LoadFile)
> > > 
> > >  #include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
> > > +#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
> > > +
> > > +typedef struct _RESET_DATA {
> > > +  UINTN   Step;
> > > +  UINTN   TplIndex;
> > > +  UINT32  RepeatTimes;
> > > +} RESET_DATA;
> > > 
> > >  #if (EFI_SPECIFICATION_VERSION >= 0x0002000A)
> > > 
> > > diff --git a/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTestConformance.c b/uefi-
> > > sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxT
> > > est/
> > > ImageBBTestConformance.c
> > > index 0a26d46847da..e90afe7ecae0 100644
> > > --- a/uefi-
> > > 

Re: [edk2-devel] [edk2-platforms][PATCH V2 2/2] ClevoOpenBoardPkg: Fix GCC Build Failures

[Sinha, Ankit]

Reviewed-by: Ankit Sinha 

-Original Message-
From: Kubacki, Michael A 
Sent: Thursday, August 22, 2019 11:17 AM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Gao, Liming 
; Sinha, Ankit 
Subject: [edk2-platforms][PATCH V2 2/2] ClevoOpenBoardPkg: Fix GCC Build 
Failures

Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.

Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 
---
 
Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
index 260c5bb6a2..cc70f15c24 100644
--- 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
+++ 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
@@ -70,12 +70,11 @@ GpioExpGetRegister (
   IN UINT8 Register
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[1];
   UINT8 ReBuf[1] = {0};
 
   WriBuf[0] = Register;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1, 
ReBuf, WAIT_1_SECOND);
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf, 
WAIT_1_SECOND);
 
   return ReBuf[0];
 }
@@ -99,13 +98,11 @@ GpioExpSetRegister (
   IN UINT8 Value
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[2];
 
   WriBuf[0] = Register;
   WriBuf[1] = Value;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0, 
NULL, WAIT_1_SECOND);
-
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL, 
WAIT_1_SECOND);
 }
 /**
   Set the input register to a give value mentioned in the function.
-- 
2.16.2.windows.1


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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Laszlo Ersek]

On 08/22/19 08:18, Paolo Bonzini wrote:
> On 21/08/19 22:17, Kinney, Michael D wrote:
>> Paolo,
>>
>> It makes sense to match real HW.
>
> Note that it'd also be fine to match some kind of official Intel
> specification even if no processor (currently?) supports it.

I agree, because...

>> That puts us back to the reset vector and handling the initial SMI at
>> 3000:8000.  That is all workable from a FW implementation
>> perspective.

that would suggest that matching reset vector code already exists, and
it would "only" need to be upstreamed to edk2. :)

>> It look like the only issue left is DMA.
>>
>> DMA protection of memory ranges is a chipset feature. For the current
>> QEMU implementation, what ranges of memory are guaranteed to be
>> protected from DMA?  Is it only A/B seg and TSEG?
>
> Yes.

(

This thread (esp. Jiewen's and Mike's messages) are the first time that
I've heard about the *existence* of such RAM ranges / the chipset
feature. :)

Out of interest (independently of virtualization), how is a general
purpose OS informed by the firmware, "never try to set up DMA to this
RAM area"? Is this communicated through ACPI _CRS perhaps?

... Ah, almost: ACPI 6.2 specifies _DMA, in "6.2.4 _DMA (Direct Memory
Access)". It writes,

For example, if a platform implements a PCI bus that cannot access
all of physical memory, it has a _DMA object under that PCI bus that
describes the ranges of physical memory that can be accessed by
devices on that bus.

Sorry about the digression, and also about being late to this thread,
continually -- I'm primarily following and learning.

)

Thanks!
Laszlo

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

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

[edk2-devel] [edk2-platforms][PATCH V2 0/2] Fix KBL-Based Platform GCC Build Failures

[Kubacki, Michael A]

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

These patches build failures found in Kaby Lake based platforms.

V2 Changes:
 * Extended copyright date in BaseGpioExpanderLib.c to 2019.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 

Michael Kubacki (2):
  KabylakeOpenBoardPkg: Fix GCC Build Failures
  ClevoOpenBoardPkg: Fix GCC Build Failures

 
Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
| 7 ++-
 
Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 | 2 +-
 
Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 8 +++-
 3 files changed, 6 insertions(+), 11 deletions(-)

-- 
2.16.2.windows.1


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

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

[edk2-devel] [edk2-platforms][PATCH V2 1/2] KabylakeOpenBoardPkg: Fix GCC Build Failures

[Kubacki, Michael A]

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

Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
 
Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 | 2 +-
 
Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 8 +++-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git 
a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 
b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
index d73fc77f69..d40eecae95 100644
--- 
a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
+++ 
b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
@@ -174,7 +174,7 @@ SecPlatformMain (
 //
 CopyMem (CopyDestinationPointer, mPeiCoreFvLocationPpiList, sizeof 
(mPeiCoreFvLocationPpiList));
 TopOfTemporaryRamPpiIndex = 1;
-(UINT8 *) CopyDestinationPointer += sizeof(mPeiCoreFvLocationPpiList);
+CopyDestinationPointer += sizeof (mPeiCoreFvLocationPpiList);
   }
   CopyMem (CopyDestinationPointer, mPeiSecPlatformPpi, 
sizeof(mPeiSecPlatformPpi));
   //
diff --git 
a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 
b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
index ead1e6df19..3d1856d89e 100644
--- 
a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
+++ 
b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
@@ -1,7 +1,7 @@
 /** @file
   Support for IO expander TCA6424.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -70,12 +70,11 @@ GpioExpGetRegister (
   IN UINT8 Register
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[1];
   UINT8 ReBuf[1] = {0};
 
   WriBuf[0] = Register;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1, 
ReBuf, WAIT_1_SECOND);
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf, 
WAIT_1_SECOND);
 
   return ReBuf[0];
 }
@@ -99,13 +98,12 @@ GpioExpSetRegister (
   IN UINT8 Value
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[2];
 
   WriBuf[0] = Register;
   WriBuf[1] = Value;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0, 
NULL, WAIT_1_SECOND);
 
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL, 
WAIT_1_SECOND);
 }
 /**
   Set the input register to a give value mentioned in the function.
-- 
2.16.2.windows.1


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

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

[edk2-devel] [edk2-platforms][PATCH V2 2/2] ClevoOpenBoardPkg: Fix GCC Build Failures

[Kubacki, Michael A]

Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.

Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 
---
 
Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
index 260c5bb6a2..cc70f15c24 100644
--- 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
+++ 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
@@ -70,12 +70,11 @@ GpioExpGetRegister (
   IN UINT8 Register
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[1];
   UINT8 ReBuf[1] = {0};
 
   WriBuf[0] = Register;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1, 
ReBuf, WAIT_1_SECOND);
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf, 
WAIT_1_SECOND);
 
   return ReBuf[0];
 }
@@ -99,13 +98,11 @@ GpioExpSetRegister (
   IN UINT8 Value
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[2];
 
   WriBuf[0] = Register;
   WriBuf[1] = Value;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0, 
NULL, WAIT_1_SECOND);
-
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL, 
WAIT_1_SECOND);
 }
 /**
   Set the input register to a give value mentioned in the function.
-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/2] KabylakeOpenBoardPkg: Fix GCC Build Failures

[Chiu, Chasel]

BaseGpioExpanderLib.c still having 2017 in copyright, please help to extend it.
With above change Reviewed-by: Chasel Chiu 

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Kubacki, Michael A
> Sent: Friday, August 23, 2019 1:40 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Gao, Liming 
> Subject: [edk2-devel] [edk2-platforms][PATCH V1 1/2] KabylakeOpenBoardPkg:
> Fix GCC Build Failures
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2109
> 
> Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
> 
> Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapper
> PlatformSecLib/FspWrapperPlatformSecLib.c | 2 +-
> 
> Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseG
> pioExpanderLib.c | 6 ++
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapp
> erPlatformSecLib/FspWrapperPlatformSecLib.c
> b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapp
> erPlatformSecLib/FspWrapperPlatformSecLib.c
> index d73fc77f69..d40eecae95 100644
> ---
> a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapp
> erPlatformSecLib/FspWrapperPlatformSecLib.c
> +++
> b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapp
> erPlatformSecLib/FspWrapperPlatformSecLib.c
> @@ -174,7 +174,7 @@ SecPlatformMain (
>  //
>  CopyMem (CopyDestinationPointer, mPeiCoreFvLocationPpiList, sizeof
> (mPeiCoreFvLocationPpiList));
>  TopOfTemporaryRamPpiIndex = 1;
> -(UINT8 *) CopyDestinationPointer += sizeof(mPeiCoreFvLocationPpiList);
> +CopyDestinationPointer += sizeof (mPeiCoreFvLocationPpiList);
>}
>CopyMem (CopyDestinationPointer, mPeiSecPlatformPpi,
> sizeof(mPeiSecPlatformPpi));
>//
> diff --git
> a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/Bas
> eGpioExpanderLib.c
> b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/Bas
> eGpioExpanderLib.c
> index ead1e6df19..acab1326ff 100644
> ---
> a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/Bas
> eGpioExpanderLib.c
> +++
> b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/Bas
> eGpioExpanderLib.c
> @@ -70,12 +70,11 @@ GpioExpGetRegister (
>IN UINT8 Register
>)
>  {
> -  EFI_STATUS Status;
>UINT8 WriBuf[1];
>UINT8 ReBuf[1] = {0};
> 
>WriBuf[0] = Register;
> -  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1,
> ReBuf, WAIT_1_SECOND);
> +  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf,
> WAIT_1_SECOND);
> 
>return ReBuf[0];
>  }
> @@ -99,13 +98,12 @@ GpioExpSetRegister (
>IN UINT8 Value
>)
>  {
> -  EFI_STATUS Status;
>UINT8 WriBuf[2];
> 
>WriBuf[0] = Register;
>WriBuf[1] = Value;
> -  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0,
> NULL, WAIT_1_SECOND);
> 
> +  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL,
> WAIT_1_SECOND);
>  }
>  /**
>Set the input register to a give value mentioned in the function.
> --
> 2.16.2.windows.1
> 
> 
> 


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

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

Re: [edk2-devel] [edk2-platforms PATCH 3/4] Platform/MinPlatformPkg.dsc: Add build option

[Chiu, Chasel]

This file still having 2018 in copyright, so please help to extend it.
With above change Reviewed-by: Chasel Chiu 

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zhang, Shenglei
> Sent: Thursday, August 22, 2019 3:08 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Gao, Liming ;
> Zhang, Shenglei 
> Subject: [edk2-devel] [edk2-platforms PATCH 3/4]
> Platform/MinPlatformPkg.dsc: Add build option
> 
> Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in DSC file
> to make sure that the deprecated APIs will not be used in our code.
> https://bugzilla.tianocore.org/show_bug.cgi?id=2111
> 
> Cc: Michael Kubacki 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Signed-off-by: Shenglei Zhang 
> ---
>  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
> index 8a3638b7..9bbb7963 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
> @@ -187,3 +187,6 @@
>MinPlatformPkg/Tcg/Tcg2PlatformPei/Tcg2PlatformPei.inf
>MinPlatformPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf
>  !endif
> +
> +[BuildOptions]
> +  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> \ No newline at end of file
> --
> 2.18.0.windows.1
> 
> 
> 


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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Laszlo Ersek]

On 08/21/19 19:05, Paolo Bonzini wrote:
> On 21/08/19 17:48, Kinney, Michael D wrote:
>> Perhaps there is a way to avoid the 3000:8000 startup
>> vector.
>>
>> If a CPU is added after a cold reset, it is already in a
>> different state because one of the active CPUs needs to
>> release it by interacting with the hot plug controller.
>>
>> Can the SMRR for CPUs in that state be pre-programmed to
>> match the SMRR in the rest of the active CPUs?
>>
>> For OVMF we expect all the active CPUs to use the same
>> SMRR value, so a check can be made to verify that all 
>> the active CPUs have the same SMRR value.  If they do,
>> then any CPU released through the hot plug controller 
>> can have its SMRR pre-programmed and the initial SMI
>> will start within TSEG.
>>
>> We just need to decide what to do in the unexpected 
>> case where all the active CPUs do not have the same
>> SMRR value.
>>
>> This should also reduce the total number of steps.
> 
> The problem is not the SMRR but the SMBASE.  If the SMBASE area is
> outside TSEG, it is vulnerable to DMA attacks independent of the SMRR.
> SMBASE is also different for all CPUs, so it cannot be preprogrammed.

The firmware and QEMU could agree on a formula, which would compute the
CPU-specific SMBASE from a value pre-programmed by the firmware, and the
initial APIC ID of the hot-added CPU.

Yes, it would duplicate code -- the calculation -- between QEMU and
edk2. While that's not optimal, it wouldn't be a first.

Thanks
Laszlo

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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Laszlo Ersek]

On 08/21/19 17:48, Kinney, Michael D wrote:
> Perhaps there is a way to avoid the 3000:8000 startup
> vector.
>
> If a CPU is added after a cold reset, it is already in a
> different state because one of the active CPUs needs to
> release it by interacting with the hot plug controller.
>
> Can the SMRR for CPUs in that state be pre-programmed to
> match the SMRR in the rest of the active CPUs?
>
> For OVMF we expect all the active CPUs to use the same
> SMRR value, so a check can be made to verify that all
> the active CPUs have the same SMRR value.  If they do,
> then any CPU released through the hot plug controller
> can have its SMRR pre-programmed and the initial SMI
> will start within TSEG.

Yes, that is what I proposed here:

* effa5e32-be1e-4703-4419-8866b7754e2d@redhat.com">http://mid.mail-archive.com/effa5e32-be1e-4703-4419-8866b7754e2d@redhat.com
* https://edk2.groups.io/g/devel/message/45570

Namely:

> When the SMM setup quiesces during normal firmware boot, OVMF could
> use existent (finalized) SMBASE infomation to *pre-program* some
> virtual QEMU hardware, with such state that would be expected, as
> "final" state, of any new hotplugged CPU. Afterwards, if / when the
> hotplug actually happens, QEMU could blanket-apply this state to the
> new CPU, and broadcast a hardware SMI to all CPUs except the new one.

(I know that Paolo didn't like it; I'm just confirming that I had the
same, or at least a very similar, idea.)

Thanks!
Laszlo

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

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

[edk2-devel] [edk2-platforms][PATCH V1 0/2] Fix KBL-Based Platform GCC Build Failures

[Kubacki, Michael A]

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

These patches build failures found in Kaby Lake based platforms.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 

Michael Kubacki (2):
  KabylakeOpenBoardPkg: Fix GCC Build Failures
  ClevoOpenBoardPkg: Fix GCC Build Failures

 
Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
| 7 ++-
 
Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 | 2 +-
 
Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 6 ++
 3 files changed, 5 insertions(+), 10 deletions(-)

-- 
2.16.2.windows.1


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

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

[edk2-devel] [edk2-platforms][PATCH V1 2/2] ClevoOpenBoardPkg: Fix GCC Build Failures

[Kubacki, Michael A]

Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.

Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Ankit Sinha 
Signed-off-by: Michael Kubacki 
---
 
Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
index 260c5bb6a2..cc70f15c24 100644
--- 
a/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
+++ 
b/Platform/Intel/ClevoOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
@@ -70,12 +70,11 @@ GpioExpGetRegister (
   IN UINT8 Register
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[1];
   UINT8 ReBuf[1] = {0};
 
   WriBuf[0] = Register;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1, 
ReBuf, WAIT_1_SECOND);
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf, 
WAIT_1_SECOND);
 
   return ReBuf[0];
 }
@@ -99,13 +98,11 @@ GpioExpSetRegister (
   IN UINT8 Value
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[2];
 
   WriBuf[0] = Register;
   WriBuf[1] = Value;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0, 
NULL, WAIT_1_SECOND);
-
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL, 
WAIT_1_SECOND);
 }
 /**
   Set the input register to a give value mentioned in the function.
-- 
2.16.2.windows.1


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

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

[edk2-devel] [edk2-platforms][PATCH V1 1/2] KabylakeOpenBoardPkg: Fix GCC Build Failures

[Kubacki, Michael A]

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

Fixes build failures on GCC7.3.0. Tested on Ubunutu 18.04.1 LTS.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
 
Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 | 2 +-
 
Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 | 6 ++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git 
a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
 
b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
index d73fc77f69..d40eecae95 100644
--- 
a/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
+++ 
b/Platform/Intel/KabylakeOpenBoardPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FspWrapperPlatformSecLib.c
@@ -174,7 +174,7 @@ SecPlatformMain (
 //
 CopyMem (CopyDestinationPointer, mPeiCoreFvLocationPpiList, sizeof 
(mPeiCoreFvLocationPpiList));
 TopOfTemporaryRamPpiIndex = 1;
-(UINT8 *) CopyDestinationPointer += sizeof(mPeiCoreFvLocationPpiList);
+CopyDestinationPointer += sizeof (mPeiCoreFvLocationPpiList);
   }
   CopyMem (CopyDestinationPointer, mPeiSecPlatformPpi, 
sizeof(mPeiSecPlatformPpi));
   //
diff --git 
a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
 
b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
index ead1e6df19..acab1326ff 100644
--- 
a/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
+++ 
b/Platform/Intel/KabylakeOpenBoardPkg/Library/BaseGpioExpanderLib/BaseGpioExpanderLib.c
@@ -70,12 +70,11 @@ GpioExpGetRegister (
   IN UINT8 Register
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[1];
   UINT8 ReBuf[1] = {0};
 
   WriBuf[0] = Register;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 1, WriBuf, 1, 
ReBuf, WAIT_1_SECOND);
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 1, WriBuf, 1, ReBuf, 
WAIT_1_SECOND);
 
   return ReBuf[0];
 }
@@ -99,13 +98,12 @@ GpioExpSetRegister (
   IN UINT8 Value
   )
 {
-  EFI_STATUS Status;
   UINT8 WriBuf[2];
 
   WriBuf[0] = Register;
   WriBuf[1] = Value;
-  Status = I2cWriteRead( Bar0, TCA6424_I2C_ADDRESS+Address, 2, WriBuf, 0, 
NULL, WAIT_1_SECOND);
 
+  I2cWriteRead (Bar0, TCA6424_I2C_ADDRESS + Address, 2, WriBuf, 0, NULL, 
WAIT_1_SECOND);
 }
 /**
   Set the input register to a give value mentioned in the function.
-- 
2.16.2.windows.1


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

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

Re: [edk2-devel] [PATCH] Intel/* clean up duplicated files in Edk2Platforms

[Chiu, Chasel]

I'm ok with the change but next time please help to take care:
1. Use subject-prefix="edk2-platforms:PATCH" for edk2-platforms related patches.
2. suggest to generate separate patches in a series for cross package changes.

Reviewed-by: Chasel Chiu 


> -Original Message-
> From: Chen, Marc W
> Sent: Thursday, August 22, 2019 1:28 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V ; Chiu, Chasel
> ; Gao, Liming ; Desimone,
> Nathaniel L ; Steele, Kelly
> ; Gillispie, Thad ; Bu,
> Daocheng ; Oram, Isaac W
> 
> Subject: [PATCH] Intel/* clean up duplicated files in Edk2Platforms
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
> 
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
> 
> The following duplicated header file can be clean up.
> Edk2Platforms\Platform\Intel\MinPlatformPkg\Include\Guid\SmramMemory
> Reserve.h
> Edk2Platforms\Silicon\Intel\KabylakeSiliconPkg\SampleCode\IntelFramewor
> kPkg\Include\Guid\SmramMemoryReserve.h
> Edk2Platforms\Silicon\Intel\PurleySktPkg\Include\Guid\SmramMemoryReser
> ve.h
> Edk2Platforms\Silicon\Intel\QuarkSocPkg\QuarkNorthCluster\Include\Guid\
> SmramMemoryReserve.h
> Edk2Platforms\Silicon\Intel\CoffeelakeSiliconPkg\SampleCode\IntelFramewo
> rkPkg\Include\Guid\SmramMemoryReserve.h
> 
> Signed-off-by: Marc W Chen 
> Cc: Sai Chaganty 
> Cc: Chasel Chiu 
> Cc: Liming Gao 
> Cc: Nate DeSimone 
> Cc: Kelly Steele 
> Cc: Thad Gillispie 
> Cc: Daocheng Bu 
> Cc: Isaac W Oram 
> ---
>  .../Include/Guid/SmramMemoryReserve.h  | 54 
> --
>  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec   |  5 --
>  .../PlatformInitPei/PlatformInitPostMem.c  |  4 +-
>  .../PlatformInitPei/PlatformInitPostMem.inf|  4 +-
>  .../Library/TestPointCheckLib/PeiCheckSmmInfo.c|  6 +--
>  .../Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatform.c  |  4 +-
>  .../Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatform.inf|  2 +-
>  .../Platform/Pei/PlatformInit/MrcWrapper.c |  8 ++--
>  .../Pei/PlatformInit/PlatformEarlyInit.inf |  2 +-
>  .../Include/Guid/SmramMemoryReserve.h  | 51 
>  Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec   |  5 --
>  .../SystemAgent/SmmAccess/Dxe/SmmAccess.inf|  2 +-
>  .../SystemAgent/SmmAccess/Dxe/SmmAccessDriver.c|  2 +-
>  .../Include/Guid/SmramMemoryReserve.h  | 54 
> --
>  Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec |  4 --
>  .../SystemAgent/SmmAccess/Dxe/SmmAccess.inf|  4 +-
>  .../SystemAgent/SmmAccess/Dxe/SmmAccessDriver.c|  4 +-
>  .../PurleySktPkg/Include/Guid/SmramMemoryReserve.h | 43 -
>  Silicon/Intel/PurleySktPkg/SocketPkg.dec   |  3 +-
>  .../Include/Guid/SmramMemoryReserve.h  | 54 
> --
>  .../Smm/Dxe/SmmAccessDxe/SmmAccess.inf |  2 +-
>  .../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c |  2 +-
>  .../Smm/Pei/SmmAccessPei/SmmAccessPei.c|  4 +-
>  .../Smm/Pei/SmmAccessPei/SmmAccessPei.inf  |  2 +-
>  Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dec  |  1 -
>  25 files changed, 27 insertions(+), 299 deletions(-)  delete mode 100644
> Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
>  delete mode 100644
> Silicon/Intel/CoffeelakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/
> Guid/SmramMemoryReserve.h
>  delete mode 100644
> Silicon/Intel/KabylakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/G
> uid/SmramMemoryReserve.h
>  delete mode 100644
> Silicon/Intel/PurleySktPkg/Include/Guid/SmramMemoryReserve.h
>  delete mode 100644
> Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Include/Guid/SmramMemory
> Reserve.h
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> b/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> deleted file mode 100644
> index 9918c768ba..00
> --- a/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -/** @file
> -  Definition of GUIDed HOB for reserving SMRAM regions.
> -
> -  This file defines:
> -  * the GUID used to identify the GUID HOB for reserving SMRAM regions.
> -  * the data structure of SMRAM descriptor to describe SMRAM candidate
> regions
> -  * values of state of SMRAM candidate regions
> -  * the GUID specific data structure of HOB for reserving SMRAM regions.
> -  This GUIDed HOB can be used to convey the existence of the T-SEG
> reservation and H-SEG usage
> -
> -Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -  @par Revision Reference:
> -  GUIDs defined in SmmCis spec version 0.9.
> -
> -**/
> -
> -#ifndef _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -#define _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -
> -#define EFI_SMM_PEI_SMRAM_MEMORY_RESERVE \
> -  { \
> -0x6dadf1d1, 0xd4cc, 0x4910, {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff, 
> 0x3d 

Re: [edk2-devel] [RFC PATCH 05/28] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase

[Lendacky, Thomas]

On 8/22/19 9:12 AM, Laszlo Ersek wrote:
> On 08/21/19 23:42, Lendacky, Thomas wrote:
>> On 8/21/19 9:31 AM, Laszlo Ersek wrote:
>>> On 08/19/19 23:35, Lendacky, Thomas wrote:
 From: Tom Lendacky 

 Allocate memory for the GHCB pages during SEV initialization for use
 during Pei and Dxe phases. Since the GHCB pages must be mapped as shared
 pages, modify CreateIdentityMappingPageTables() so that pagetable entries
 are created without the encryption bit set.

 Signed-off-by: Tom Lendacky 
 ---
  UefiCpuPkg/UefiCpuPkg.dec |  4 ++
  OvmfPkg/OvmfPkgX64.dsc|  4 ++
  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf   |  3 +
  OvmfPkg/PlatformPei/PlatformPei.inf   |  2 +
  .../Core/DxeIplPeim/X64/VirtualMemory.h   | 12 +++-
  .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c|  4 +-
  .../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +++-
  .../Core/DxeIplPeim/X64/VirtualMemory.c   | 49 ++
  .../MemEncryptSevLibInternal.c|  1 -
  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  | 33 --
  OvmfPkg/PlatformPei/AmdSev.c  | 64 +++
  11 files changed, 164 insertions(+), 23 deletions(-)
>>>
>>> Should be split to at least four patches (UefiCpuPkg, MdeModulePkg,
>>> OvmfPkg/BaseMemEncryptSevLib, OvmfPkg/PlatformPei).
>>>
>>> In addition, MdeModulePkg content must not depend on UefiCpuPkg content
>>> -- if modules under both packages need to consume a new PCD, then the
>>> PCD should be declared under MdeModulePkg. The rough dependency order is:
>>>
>>> - MdePkg (must be self-contained)
>>> - MdeModulePkg (may consume MdePkg)
>>> - UefiCpuPkg (may consume everything above, to my knowledge)
>>> - OvmfPkg (may consume everything above)
>>>
>>
>> Ok, thanks for the guidance.
>>
>> Ideally, I just would like to modify the newly created page tables after
>> the call to CreateIdentityMappingPageTables() in MdeModulePkg/Core/
>> DxeIplPeim/Ia32/DxeLoadFunc.c. Is there a preferred way to add a listener
>> or callback or notification service so that the main changes would be
>> limited to the OvmfPkg files and would that be acceptable?
> 
> * https://bugzilla.tianocore.org/show_bug.cgi?id=623
> 
>   Reported on 2017-07-07, resolved as WONTFIX on 2019-07-30 ("no
>   resources").
> 
>   And it's not like patches had not been proposed -- Leo had implemented
>   a notification service --; they were rejected.
> 
> * https://bugzilla.tianocore.org/show_bug.cgi?id=847
> 
>   Reported on 2018-01-11, marked "not high priority" as of 2019-07-23
>   .
> 
> I don't know what to tell you. While nobody seems to disagree with the
> necessity of such a service and/or library, core maintainers have
> rejected all the code proposals thus far (= "don't do that"). And I'm
> unaware of any constructive guidance (= "do this instead").

This isn't on the level of a "notify every time something changes" type
of thing. This is more of a "hey, we built some new pagetables and are
about to make them active, but before we do have a look and change what
you think should be changed."

With that, I'd be able to remove the GhcbBase and GhcbSize that is
propogated on the ToSplit and Split functions.

I'll take a look and see what it would look like and go from there.

> 
> I suggest filing a Feature Request BZ for SEV-ES enablement (for
> OvmfPkg), and referencing that as "dependent bug" in both of the
> above-mentioned BZs. It might also help to dial in to the APAC/NAMO
> design / bug triage meeting, and campaign for the feature there.

Yes, I need to file that Feature Request BZ anyway.

Thanks,
Tom

> 
> https://github.com/tianocore/tianocore.github.io/wiki/Bug-Triage
> 
> I have a bad track record at convincing core maintainers to do what they
> don't want to do. And I see escalating such problems from email to phone
> as a work-around, sort of "wear down your opponent by sheer
> persistence". So I avoid that. But, I've seen the approach work for
> others, so you might have better luck.
> 
> (The APAC/NAMO call is also at a bad time for me, in UTC+1 / UTC+2.)
> 
> I think the present RFC patches are a good way to re-raise these topics.
> 
> Laszlo
> 

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

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

Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing MAT update

[Liming Gao]

Laszlo:

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, August 22, 2019 7:56 PM
> To: Gao, Liming 
> Cc: devel@edk2.groups.io; Kinney, Michael D ; 
> Mike Turner ; Wang, Jian J
> ; Wu, Hao A ; Bi, Dandan 
> ; Anthony Perard
> ; Justen, Jordan L 
> Subject: Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing MAT 
> update
> 
> (+Anthony, +Jordan)
> 
> On 08/21/19 16:14, Gao, Liming wrote:
> > Laszlo:
> >
> >> -Original Message-
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> >> Laszlo Ersek
> >> Sent: Wednesday, August 21, 2019 4:46 PM
> >> To: Gao, Liming ; devel@edk2.groups.io; Kinney, 
> >> Michael D 
> >> Cc: Mike Turner ; Wang, Jian J 
> >> ; Wu, Hao A ; Bi, Dandan
> >> 
> >> Subject: Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing 
> >> MAT update
> >>
> >> Hi Liming,
> >>
> >> On 08/19/19 02:40, Gao, Liming wrote:
> >>
>  ... Ugh, wait. I've actually implemented this for OVMF almost 2 years
>  ago! And the answer to my question is "yes":
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=386
>   https://lists.01.org/pipermail/edk2-devel/2017-November/018312.html
> 
>  See the OnReadOnlyVariable2Available() function:
> 
>  +  //
>  +  // Check if the "MemoryTypeInformation" variable exists, in the
>  +  // gEfiMemoryTypeInformationGuid namespace.
>  +  //
>  +  ReadOnlyVariable2 = Ppi;
>  +  DataSize = 0;
>  +  Status = ReadOnlyVariable2->GetVariable (
>  +ReadOnlyVariable2,
>  +
>  EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME,
>  +,
>  +NULL,
>  +,
>  +NULL
>  +);
>  +  if (Status == EFI_BUFFER_TOO_SMALL) {
>  +//
>  +// The variable exists; the DXE IPL PEIM will build the HOB from it.
>  +//
>  +return EFI_SUCCESS;
>  +  }
>  +  //
>  +  // Install the default memory type information HOB.
>  +  //
>  +  BuildMemTypeInfoHob ();
> 
>  Apologies for forgetting about this; you are right.
> 
>  Too bad my work for TianoCore#386 was rejected. :(
> 
> >>>
> >>> So, this is one value to add PEI variable support in OVMF.
> >>> Do you consider to approve this feature request?
> >>
> >> Sorry, I don't understand your question.
> >>
> >> Are you asking me if, in my opinion, we should fix TianoCore#386 (= add
> >> PEI variable support to OVMF)?
> >
> > Yes. Fix TianoCore#386 is my suggestion.
> >
> >>
> >> If that is your question, then my answer is -- trivially -- "yes". If
> >> you read through TianoCore#386, you will see that I submitted patches
> >> for solving the BZ, twice (comment 6, comment 9).
> >
> > I see your patch link in BZ.
> >
> >>
> >> Jordan rejected my patches both times, because he thought that the same
> >> feature should be implemented in OVMF for Xen as well. I disagreed (and
> >> still disagree) -- my patches depend on a build-time flag that produces
> >> an OVMF binary that is only compatible with QEMU, and not Xen. The
> >> reason for that is that the feature (PEI variable support) cannot be
> >> implemented *sensibly* on Xen. (Xen offers no spec-conformant variable
> >> storage, and in the PEI phase, the variable store would always appear
> >> empty.) Later, Jordan tried to add dynamic pflash detection to PEI, but
> >> it didn't work in all cases, because OVMF's PEI phase doesn't run in
> >> SMM, while QEMU restricts pflash access to SMM. (In other words, pflash
> >> protection in QEMU is less flexible than SMRAM protection -- SMRAM is
> >> unlocked in PEI, but pflash is always locked.) Please see the threads
> >> linked in the BZ for the discussion.
> >
> > Thanks for you detail information. I miss them before.
> >
> >>
> >> With TianoCore#1689, Anthony has started separating OVMF into different
> >> firmware platforms for Xen and QEMU. In a year or so, "OVMF for QEMU"
> >> will likely have nothing Xen-related in it (because "OVMF for Xen" will
> >> exist separately). Then we can reopen TianoCore#386 just for "OVMF for
> >> QEMU", and solve this feature.
> >
> > TianoCore#1689 is in edk2 stable tag 201908 feature planning.
> > Dose it still catch 201808 stable tag?
> 
> Yes, I pushed Anthony's v5 series yesterday, and closed TianoCore#1689.
> 
> TianoCore#1689 is also listed on the feature planning page, as "New
> OvmfXen platform with Xen PVH support":
> 
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
> 
Thanks. I have seen this feature is done. 

> >
> >>
> >> In summary, I have had patches for TianoCore#386 ready for 2+ years,
> >> they've been blocked because they only target QEMU (with a build-time
> >> flag), and I expect to upstream them finally as 

Re: [edk2-devel] [Patch V2][edk2-stable201908] BaseTools: Incorrect error message for library instance not found

[Liming Gao]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Feng, Bob C
> Sent: Thursday, August 22, 2019 12:01 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: [Patch V2][edk2-stable201908] BaseTools: Incorrect error message for 
> library instance not found
> 
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2099
> This is a regression issue introduced by commit e8449e.
> 
> This patch is to fix this issue.
> 
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> ---
> V2: We need to check if a inf is a Library or a Module.
>  BaseTools/Source/Python/AutoGen/DataPipe.py |  2 +-
>  BaseTools/Source/Python/AutoGen/PlatformAutoGen.py  |  2 +-
>  BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py |  4 +++-
>  .../Source/Python/Workspace/WorkspaceCommon.py  | 13 +++--
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/DataPipe.py 
> b/BaseTools/Source/Python/AutoGen/DataPipe.py
> index 2ca4f9ff4a..8b8cfd1c51 100755
> --- a/BaseTools/Source/Python/AutoGen/DataPipe.py
> +++ b/BaseTools/Source/Python/AutoGen/DataPipe.py
> @@ -87,11 +87,11 @@ class MemoryDataPipe(DataPipe):
>  #Module's Library Instance
>  ModuleLibs = {}
>  libModules = {}
>  for m in PlatformInfo.Platform.Modules:
>  module_obj = 
> BuildDB.BuildObject[m,PlatformInfo.Arch,PlatformInfo.BuildTarget,PlatformInfo.ToolChain]
> -Libs = GetModuleLibInstances(module_obj, PlatformInfo.Platform, 
> BuildDB.BuildObject,
> PlatformInfo.Arch,PlatformInfo.BuildTarget,PlatformInfo.ToolChain)
> +Libs = GetModuleLibInstances(module_obj, PlatformInfo.Platform, 
> BuildDB.BuildObject,
> PlatformInfo.Arch,PlatformInfo.BuildTarget,PlatformInfo.ToolChain,PlatformInfo.MetaFile,EdkLogger)
>  for lib in Libs:
>  try:
> 
> libModules[(lib.MetaFile.File,lib.MetaFile.Root,lib.Arch,lib.MetaFile.Path)].append((m.File,m.Root,module_obj.Arch,m.Path))
>  except:
>  
> libModules[(lib.MetaFile.File,lib.MetaFile.Root,lib.Arch,lib.MetaFile.Path)] =
> [(m.File,m.Root,module_obj.Arch,m.Path)]
> diff --git a/BaseTools/Source/Python/AutoGen/PlatformAutoGen.py 
> b/BaseTools/Source/Python/AutoGen/PlatformAutoGen.py
> index dd629ba2fa..565424a95e 100644
> --- a/BaseTools/Source/Python/AutoGen/PlatformAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/PlatformAutoGen.py
> @@ -1087,11 +1087,11 @@ class PlatformAutoGen(AutoGen):
>  def GetAllModuleInfo(self,WithoutPcd=True):
>  ModuleLibs = set()
>  for m in self.Platform.Modules:
>  module_obj = 
> self.BuildDatabase[m,self.Arch,self.BuildTarget,self.ToolChain]
>  if not bool(module_obj.LibraryClass):
> -Libs = GetModuleLibInstances(module_obj, self.Platform, 
> self.BuildDatabase, self.Arch,self.BuildTarget,self.ToolChain)
> +Libs = GetModuleLibInstances(module_obj, self.Platform, 
> self.BuildDatabase,
> self.Arch,self.BuildTarget,self.ToolChain,self.MetaFile,EdkLogger)
>  else:
>  Libs = []
> 
> ModuleLibs.update( 
> set([(l.MetaFile.File,l.MetaFile.Root,l.MetaFile.Path,l.MetaFile.BaseName,l.MetaFile.OriginalPath,l.Arch,True)
>  for l in
> Libs]))
>  if WithoutPcd and module_obj.PcdIsDriver:
>  continue
> diff --git a/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py 
> b/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> index ea0d8f8bfb..2494267472 100644
> --- a/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> @@ -246,11 +246,13 @@ class WorkspaceAutoGen(AutoGen):
>  if BuildData.MetaFile.Ext == '.inf' and str(BuildData) in 
> Platform.Modules :
>  Libs.extend(GetModuleLibInstances(BuildData, Platform,
>   self.BuildDatabase,
>   Arch,
>   self.BuildTarget,
> - self.ToolChain
> + self.ToolChain,
> + self.Platform.MetaFile,
> + EdkLogger
>   ))
>  for BuildData in list(self.BuildDatabase._CACHE_.values()):
>  if BuildData.Arch != Arch:
>  continue
>  if BuildData.MetaFile.Ext == '.inf':
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py 
> b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> index 76583f46e5..0b11ec2d59 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> @@ -13,10 +13,11 @@ from .BuildClassObject import LibraryClassObject
>  import Common.GlobalData as GlobalData
>  from Workspace.BuildClassObject import 

Re: [edk2-devel] [RFC PATCH 05/28] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase

[Laszlo Ersek]

On 08/21/19 23:42, Lendacky, Thomas wrote:
> On 8/21/19 9:31 AM, Laszlo Ersek wrote:
>> On 08/19/19 23:35, Lendacky, Thomas wrote:
>>> From: Tom Lendacky 
>>>
>>> Allocate memory for the GHCB pages during SEV initialization for use
>>> during Pei and Dxe phases. Since the GHCB pages must be mapped as shared
>>> pages, modify CreateIdentityMappingPageTables() so that pagetable entries
>>> are created without the encryption bit set.
>>>
>>> Signed-off-by: Tom Lendacky 
>>> ---
>>>  UefiCpuPkg/UefiCpuPkg.dec |  4 ++
>>>  OvmfPkg/OvmfPkgX64.dsc|  4 ++
>>>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf   |  3 +
>>>  OvmfPkg/PlatformPei/PlatformPei.inf   |  2 +
>>>  .../Core/DxeIplPeim/X64/VirtualMemory.h   | 12 +++-
>>>  .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c|  4 +-
>>>  .../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +++-
>>>  .../Core/DxeIplPeim/X64/VirtualMemory.c   | 49 ++
>>>  .../MemEncryptSevLibInternal.c|  1 -
>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  | 33 --
>>>  OvmfPkg/PlatformPei/AmdSev.c  | 64 +++
>>>  11 files changed, 164 insertions(+), 23 deletions(-)
>>
>> Should be split to at least four patches (UefiCpuPkg, MdeModulePkg,
>> OvmfPkg/BaseMemEncryptSevLib, OvmfPkg/PlatformPei).
>>
>> In addition, MdeModulePkg content must not depend on UefiCpuPkg content
>> -- if modules under both packages need to consume a new PCD, then the
>> PCD should be declared under MdeModulePkg. The rough dependency order is:
>>
>> - MdePkg (must be self-contained)
>> - MdeModulePkg (may consume MdePkg)
>> - UefiCpuPkg (may consume everything above, to my knowledge)
>> - OvmfPkg (may consume everything above)
>>
> 
> Ok, thanks for the guidance.
> 
> Ideally, I just would like to modify the newly created page tables after
> the call to CreateIdentityMappingPageTables() in MdeModulePkg/Core/
> DxeIplPeim/Ia32/DxeLoadFunc.c. Is there a preferred way to add a listener
> or callback or notification service so that the main changes would be
> limited to the OvmfPkg files and would that be acceptable?

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

  Reported on 2017-07-07, resolved as WONTFIX on 2019-07-30 ("no
  resources").

  And it's not like patches had not been proposed -- Leo had implemented
  a notification service --; they were rejected.

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

  Reported on 2018-01-11, marked "not high priority" as of 2019-07-23
  .

I don't know what to tell you. While nobody seems to disagree with the
necessity of such a service and/or library, core maintainers have
rejected all the code proposals thus far (= "don't do that"). And I'm
unaware of any constructive guidance (= "do this instead").

I suggest filing a Feature Request BZ for SEV-ES enablement (for
OvmfPkg), and referencing that as "dependent bug" in both of the
above-mentioned BZs. It might also help to dial in to the APAC/NAMO
design / bug triage meeting, and campaign for the feature there.

https://github.com/tianocore/tianocore.github.io/wiki/Bug-Triage

I have a bad track record at convincing core maintainers to do what they
don't want to do. And I see escalating such problems from email to phone
as a work-around, sort of "wear down your opponent by sheer
persistence". So I avoid that. But, I've seen the approach work for
others, so you might have better luck.

(The APAC/NAMO call is also at a bad time for me, in UTC+1 / UTC+2.)

I think the present RFC patches are a good way to re-raise these topics.

Laszlo

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

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

Re: [edk2-devel] Memory Operation Mode

[Rafael Machado]

Hi Andrew

Thanks for the information and clarification!

The strange is that at some systems I have checked these SMBIOS tables,
have nothing that would indicate the current mode of operation in a direct
way.
The closest to this is this field, taken from "Type 17" from SMBIOS spec
3.2.0  :




*"7.18.7 Memory Device — Memory Operating Mode Capability Table 79 shows
what the word bit positions mean for the Memory Device - Memory Operating
ModeCapability field. This field indicates the supported operating mode(s);
it does not indicate the currentconfigured operating mode(s).  "*

But as mentioned at the spec, just the supported modes are present, and not
the currently used operating mode.

The strange is that there are some tools that run at OS level that present
this kind of information.
At one software (I will not say names to avoid problems), the current
operating mode is presented at a tab named Memory with the following
content:

*Type: DDR4*
*Size: 8 GBytes*
*Channel #: Single*
*NB Frequency: 2593.0 MHz*

At this software the field "Channel #" changes to "Dual" when adding the
memory devices to the correct slots.

At another software, the operating mode is presented at a tab named Chipset
on a section named "Memory Controller", with the following content:

*Type: Dual Channel (128-bit)*
*Active Mode: Single Channel (64-bit)*

At this same software their is a tab named North bridge with several
information.

So my understanding is that these softwares are getting to the conclusion
about the current operating mode in some indirect way. Maybe
considering the DIMM location and bank. At this system, the SMBIOS presents
the following information at Type17 (two memory devices attached to the
motherboard):



*Device Locator String1 - "ChannelA-DIMM0"Bank Locator String2 - "BANK 0"*


*Device Locator String1 - "ChannelB-DIMM0"Bank Locator String2 - "BANK 2"*


But I am not sure if it would be safe to consider some logic simply based
on the BANK and Channel information.
There are some registers that indicate if the platform supports dual
channel, and other register that say if the platform supports dual channel
but limits one DIMM per channel.
>From my perspective the conclusion of the current operating mode should
consider both information (smbios + chipset registers), and even this way I
am not 100% it would work correctly.
The problem with this is that the chipset registers would be different,
creating some complexity to maintain this code. (I know this is commond at
HDS (hardware dependent software aka. BIOS/UEFI, but I woud like to avoid
it if possible).

Could someone please clarify if it would be safe to consider 2 DIMM at the
same bank and at the same channel as a confirmation that the platform is
operating in dual channel, without considering
chipset registers?

Thanks and Regards
Rafael

Em qua, 21 de ago de 2019 às 16:56, Andrew Fish  escreveu:

>
>
> On Aug 21, 2019, at 11:55 AM, Rafael Machado <
> rafaelrodrigues.mach...@gmail.com> wrote:
>
> Hi everyone
>
> I would like to ask for some help regarding one question.
> How do I know how the memory communication was set by the BIOS/MRC/FSP
> during the initialization process?
> For example, I would like to know if the memory controller is working with
> the memories in single channel or dual channel mode.
>
> Didn't find anything related to this at the uefi spec.
> And seems the registers that may have this information are platform
> dependent, so no generic solution seems to be currently available.
>
> Any idea about how to have this information?
>
>
> Rafael,
>
> This is probably more of a PI kind of spec question, but the PI spec only
> really deals in how memory is discovered and registered with the PEI/DXE
> Core etc. There is not any infrastructure that abstracts this level of
> detail in PI.
>
> In general the kind of info you are looking for would only exist in SMBIOS
> records. You could start with the Type 16, 17, 18, and 19 SMBIOS records.
> The code that configure the memory knows a lot, but the only standard way
> to communicate this info is via SMBIOS.
>
> Thanks,
>
> Andrew Fish
>
> Thanks and Regards
> Rafael R. Machado
> 
>
>
>

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

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

Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting

[Laszlo Ersek]

On 08/21/19 23:51, Jordan Justen wrote:
> On 2019-08-21 07:21:25, Laszlo Ersek wrote:
>> On 08/19/19 23:35, Lendacky, Thomas wrote:
>>> From: Tom Lendacky 
>>>
>>> Currently, the OVMF code relies on the hypervisor to enable the cache
>>> support on the processor in order to improve the boot speed. However,
>>> with SEV-ES, the hypervisor is not allowed to change the CR0 register
>>> to enable caching.
>>>
>>> Update the OVMF Sec support to enable caching in order to improve the
>>> boot speed.
>>>
>>> Signed-off-by: Tom Lendacky 
>>> ---
>>>  OvmfPkg/Sec/SecMain.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>>> index 3914355cd17b..2448be0cd408 100644
>>> --- a/OvmfPkg/Sec/SecMain.c
>>> +++ b/OvmfPkg/Sec/SecMain.c
>>> @@ -739,6 +739,11 @@ SecCoreStartupWithStack (
>>>  
>>>ProcessLibraryConstructorList (NULL, NULL);
>>>  
>>> +  //
>>> +  // Enable caching
>>> +  //
>>> +  AsmEnableCache ();
>>> +
>>>DEBUG ((EFI_D_INFO,
>>>  "SecCoreStartupWithStack(0x%x, 0x%x)\n",
>>>  (UINT32)(UINTN)BootFv,
>>>
>>
>> This makes me uncomfortable. There used to be problems related to
>> caching when VFIO device assignment were used. My concern is admittedly
>> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
>> "well what could break here", I'd answer "you never know, and the burden
>> of proof is not on me". :) Can we make this change conditional on SEV-ES?
> 
> This was also raised as an issue by Peter for the ACRN hypervisor and
> Scott for the bhyve hypervisor.
> 
> I think it is rare for a platform to enable cache at this early of a
> stage, but it is also rare to decompress a firmware volume at this
> point.
> 
> It appears that it could be helpful to figure out how to safely enable
> cache by default here, since it does seem to be impacting several
> hypervisors.

I can't think of anything better than "trial and error". The issues that
used to pop up in the past, due to host kernel (KVM) changes,
particularly in connection with VFIO device assignment, have been
completely obscure and unpenetrable to me. Even though I've contributed
at least one KVM patch to mitigate those problems, they remain a mistery
to me, and I remain unable to *reason* about the problems or the fixes.

So I think we could only flip the behavior (enable cache by default) and
collect bug reports. But that's extremely annoying for end-users, and I
see "no regressions" as one of my top responsibilities.

Even if we provided an fw_cfg knob to disable the change, using
QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
PlatformPei: take no-exec DXE settings from the QEMU command line",
2015-09-15), such fw_cfg knobs are difficult to use through layered
products, such as libvirt, proxmox, etc. And of course fw_cfg is only
available on QEMU.

Laszlo

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

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

Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing MAT update

[Laszlo Ersek]

(+Anthony, +Jordan)

On 08/21/19 16:14, Gao, Liming wrote:
> Laszlo:
> 
>> -Original Message-
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo 
>> Ersek
>> Sent: Wednesday, August 21, 2019 4:46 PM
>> To: Gao, Liming ; devel@edk2.groups.io; Kinney, 
>> Michael D 
>> Cc: Mike Turner ; Wang, Jian J 
>> ; Wu, Hao A ; Bi, Dandan
>> 
>> Subject: Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing MAT 
>> update
>>
>> Hi Liming,
>>
>> On 08/19/19 02:40, Gao, Liming wrote:
>>
 ... Ugh, wait. I've actually implemented this for OVMF almost 2 years
 ago! And the answer to my question is "yes":

  https://bugzilla.tianocore.org/show_bug.cgi?id=386
  https://lists.01.org/pipermail/edk2-devel/2017-November/018312.html

 See the OnReadOnlyVariable2Available() function:

 +  //
 +  // Check if the "MemoryTypeInformation" variable exists, in the
 +  // gEfiMemoryTypeInformationGuid namespace.
 +  //
 +  ReadOnlyVariable2 = Ppi;
 +  DataSize = 0;
 +  Status = ReadOnlyVariable2->GetVariable (
 +ReadOnlyVariable2,
 +EFI_MEMORY_TYPE_INFORMATION_VARIABLE_NAME,
 +,
 +NULL,
 +,
 +NULL
 +);
 +  if (Status == EFI_BUFFER_TOO_SMALL) {
 +//
 +// The variable exists; the DXE IPL PEIM will build the HOB from it.
 +//
 +return EFI_SUCCESS;
 +  }
 +  //
 +  // Install the default memory type information HOB.
 +  //
 +  BuildMemTypeInfoHob ();

 Apologies for forgetting about this; you are right.

 Too bad my work for TianoCore#386 was rejected. :(

>>>
>>> So, this is one value to add PEI variable support in OVMF.
>>> Do you consider to approve this feature request?
>>
>> Sorry, I don't understand your question.
>>
>> Are you asking me if, in my opinion, we should fix TianoCore#386 (= add
>> PEI variable support to OVMF)?
> 
> Yes. Fix TianoCore#386 is my suggestion.
> 
>>
>> If that is your question, then my answer is -- trivially -- "yes". If
>> you read through TianoCore#386, you will see that I submitted patches
>> for solving the BZ, twice (comment 6, comment 9).
> 
> I see your patch link in BZ. 
> 
>>
>> Jordan rejected my patches both times, because he thought that the same
>> feature should be implemented in OVMF for Xen as well. I disagreed (and
>> still disagree) -- my patches depend on a build-time flag that produces
>> an OVMF binary that is only compatible with QEMU, and not Xen. The
>> reason for that is that the feature (PEI variable support) cannot be
>> implemented *sensibly* on Xen. (Xen offers no spec-conformant variable
>> storage, and in the PEI phase, the variable store would always appear
>> empty.) Later, Jordan tried to add dynamic pflash detection to PEI, but
>> it didn't work in all cases, because OVMF's PEI phase doesn't run in
>> SMM, while QEMU restricts pflash access to SMM. (In other words, pflash
>> protection in QEMU is less flexible than SMRAM protection -- SMRAM is
>> unlocked in PEI, but pflash is always locked.) Please see the threads
>> linked in the BZ for the discussion.
> 
> Thanks for you detail information. I miss them before. 
> 
>>
>> With TianoCore#1689, Anthony has started separating OVMF into different
>> firmware platforms for Xen and QEMU. In a year or so, "OVMF for QEMU"
>> will likely have nothing Xen-related in it (because "OVMF for Xen" will
>> exist separately). Then we can reopen TianoCore#386 just for "OVMF for
>> QEMU", and solve this feature.
> 
> TianoCore#1689 is in edk2 stable tag 201908 feature planning. 
> Dose it still catch 201808 stable tag?

Yes, I pushed Anthony's v5 series yesterday, and closed TianoCore#1689.

TianoCore#1689 is also listed on the feature planning page, as "New
OvmfXen platform with Xen PVH support":

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning

> 
>>
>> In summary, I have had patches for TianoCore#386 ready for 2+ years,
>> they've been blocked because they only target QEMU (with a build-time
>> flag), and I expect to upstream them finally as soon as OvmfPkg offers
>> dedicated firmware platforms for Xen and QEMU separately.
> 
> How about add BZ386 for 201911 stable tag?

That would make me very happy, but I don't think we can do it so quickly
(in three months). Here's why: TianoCore#1689 has introduced the new,
dedicated OVMF Xen platform, but it has not removed the Xen parts from
the "traditional" OVMF platform. The separation between "OVMF for Xen"
and "OVMF for QEMU" is not complete yet.

This is the current status:

- OvmfXen:
  - runs in Xen HVM guests
  - runs in Xen PVH guests

- traditional OVMF
  - runs in Xen HVM guests
  - runs in QEMU/KVM guests

The desired (and agreed 

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Paolo Bonzini]

On 21/08/19 22:17, Kinney, Michael D wrote:
> Paolo,
> 
> It makes sense to match real HW.

Note that it'd also be fine to match some kind of official Intel
specification even if no processor (currently?) supports it.

> That puts us back to
> the reset vector and handling the initial SMI at
> 3000:8000.  That is all workable from a FW implementation
> perspective.  It look like the only issue left is DMA.
> 
> DMA protection of memory ranges is a chipset feature.
> For the current QEMU implementation, what ranges of
> memory are guaranteed to be protected from DMA?  Is
> it only A/B seg and TSEG?

Yes.

Paolo

>> Yes, all of these would work.  Again, I'm interested in
>> having something that has a hope of being implemented in
>> real hardware.
>>
>> Another, far easier to implement possibility could be a
>> lockable MSR (could be the existing
>> MSR_SMM_FEATURE_CONTROL) that allows programming the
>> SMBASE outside SMM.  It would be nice if such a bit
>> could be defined by Intel.
>>
>> Paolo


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

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

Re: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Paolo Bonzini]

On 21/08/19 19:25, Kinney, Michael D wrote:
> Could we have an initial SMBASE that is within TSEG.
> 
> If we bring in hot plug CPUs one at a time, then initial
> SMBASE in TSEG can reprogram the SMBASE to the correct 
> value for that CPU.
> 
> Can we add a register to the hot plug controller that
> allows the BSP to set the initial SMBASE value for 
> a hot added CPU?  The default can be 3000:8000 for
> compatibility.
> 
> Another idea is when the SMI handler runs for a hot add
> CPU event, the SMM monarch programs the hot plug controller
> register with the SMBASE to use for the CPU that is being
> added.  As each CPU is added, a different SMBASE value can
> be programmed by the SMM Monarch.

Yes, all of these would work.  Again, I'm interested in having something
that has a hope of being implemented in real hardware.

Another, far easier to implement possibility could be a lockable MSR
(could be the existing MSR_SMM_FEATURE_CONTROL) that allows programming
the SMBASE outside SMM.  It would be nice if such a bit could be defined
by Intel.

Paolo

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

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

[edk2-devel] [edk2-platforms PATCH 4/4] Platform/UserInterfaceFeaturePkg.dsc: Add build option

[Zhang, Shenglei]

Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in
DSC file to make sure that the deprecated APIs will not be
used in our code.
https://bugzilla.tianocore.org/show_bug.cgi?id=2111

Cc: Dandan Bi 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 .../Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc   | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Platform/Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc 
b/Platform/Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc
index 2c24e43f..95c26766 100644
--- a/Platform/Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc
+++ b/Platform/Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc
@@ -76,3 +76,5 @@
   UserInterfaceFeaturePkg/UserAuthentication/UserAuthentication2Dxe.inf
   UserInterfaceFeaturePkg/UserAuthentication/UserAuthenticationSmm.inf
 
+[BuildOptions]
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
\ No newline at end of file
-- 
2.18.0.windows.1


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

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

[edk2-devel] [edk2-platforms PATCH 0/4] Add build option in DSC files

[Zhang, Shenglei]

Add build option "/D DISABLE_NEW_DEPRECATED_INTERFACES"
to make sure the deprecated APIs are not used in our code.
https://bugzilla.tianocore.org/show_bug.cgi?id=2111

Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Sai Chaganty 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
shenglei (4):
  Platform/AdvancedFeaturePkg.dsc: Add build option
  Platform/DebugFeaturePkg.dsc: Add build option
  Platform/MinPlatformPkg.dsc: Add build option
  Platform/UserInterfaceFeaturePkg.dsc: Add build option

 Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc   | 3 +++
 Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc | 3 +++
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc   | 3 +++
 .../Intel/UserInterfaceFeaturePkg/UserInterfaceFeaturePkg.dsc  | 2 ++
 4 files changed, 11 insertions(+)

-- 
2.18.0.windows.1


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

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

[edk2-devel] [edk2-platforms PATCH 2/4] Platform/DebugFeaturePkg.dsc: Add build option

[Zhang, Shenglei]

Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in
DSC file to make sure that the deprecated APIs will not be
used in our code.
https://bugzilla.tianocore.org/show_bug.cgi?id=2111

Cc: Eric Dong 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc 
b/Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc
index b88d1fae..61046f7a 100644
--- a/Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc
+++ b/Platform/Intel/DebugFeaturePkg/DebugFeaturePkg.dsc
@@ -96,3 +96,6 @@
 
   DebugFeaturePkg/AcpiDebug/AcpiDebugDxe.inf
   DebugFeaturePkg/AcpiDebug/AcpiDebugSmm.inf
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
\ No newline at end of file
-- 
2.18.0.windows.1


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

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

[edk2-devel] [edk2-platforms PATCH 1/4] Platform/AdvancedFeaturePkg.dsc: Add build option

[Zhang, Shenglei]

Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in
DSC file to make sure that the deprecated APIs will not be
used in our code.
https://bugzilla.tianocore.org/show_bug.cgi?id=2111

Cc: Michael Kubacki 
Cc: Sai Chaganty 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc 
b/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
index ea1a0072..57f28b8c 100644
--- a/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
+++ b/Platform/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
@@ -148,3 +148,6 @@
   AdvancedFeaturePkg/Ipmi/IpmiFru/IpmiFru.inf
   AdvancedFeaturePkg/Ipmi/BmcElog/BmcElog.inf
   AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.inf
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
\ No newline at end of file
-- 
2.18.0.windows.1


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

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

[edk2-devel] [edk2-platforms PATCH 3/4] Platform/MinPlatformPkg.dsc: Add build option

[Zhang, Shenglei]

Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in
DSC file to make sure that the deprecated APIs will not be
used in our code.
https://bugzilla.tianocore.org/show_bug.cgi?id=2111

Cc: Michael Kubacki 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc 
b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
index 8a3638b7..9bbb7963 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc
@@ -187,3 +187,6 @@
   MinPlatformPkg/Tcg/Tcg2PlatformPei/Tcg2PlatformPei.inf
   MinPlatformPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf
 !endif
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
\ No newline at end of file
-- 
2.18.0.windows.1


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

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