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

["Liming Gao" <liming.gao@intel.com>]

Laszlo:

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, August 22, 2019 7:56 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Mike Turner <miketur@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Anthony Perard
> <anthony.perard@citrix.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> 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 <liming.gao@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> >> Cc: Mike Turner <miketur@microsoft.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan
> >> <dandan.bi@intel.com>
> >> 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,
> >>>> +                                &gEfiMemoryTypeInformationGuid,
> >>>> +                                NULL,
> >>>> +                                &DataSize,
> >>>> +                                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 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 upon) end-status is the following:
> 
> - OvmfXen:
>   - runs in Xen HVM guests
>   - runs in Xen PVH guests
> 
> - traditional OVMF
>   - runs in QEMU/KVM guests
> 
> (This will match the platform separation that we have under ArmVirtPkg too.)
> 
> Anthony plans to remove the Xen HVM code from traditional OVMF in a year
> or so, if I understand correctly. That's when "traditional OVMF" will
> become QEMU/KVM-only, completing the separation. And that's when I
> expect we can fix TianoCore#386 for QEMU/KVM *only*, without Jordan
> NACKing the patch set.
> 
Ok. Is there BZ for this change? The contributor can propose his work planning. 
Then, I will update edk2 feature planning wiki. 

> Basically, "PcdMemVarstoreEmuEnable" would be constant TRUE in OvmfXen
> (inherited from the OVMF DEC file), and it would be exposed to the
> platform builder via "-D MEM_VARSTORE_EMU_ENABLE=FALSE" in the "OVMF for
> QEMU/KVM" platform. FaultTolerantWritePei and VariablePei would be
> included in the "OVMF for QEMU/KVM" DSC files only, and so on.
> 
I think this design meets with the request BZ386. 

Thanks
Liming

> Thanks
> Laszlo