[edk2-devel] [PATCH V5 17/33] OvmfPkg: Update Sec to support Tdx

[Min Xu]

On January 26, 2022 7:53 PM, Gerd Hoffmann wrote:
> > PcdTdxAcceptPageSize is added for page accepting. Currently TDX
> > supports 4K and 2M accept page size. The default value is 4K.
> 
> Is there a good reason to not use 2M by default?  In case that fails the code will
> fallback to 4K pages, so there isn't an obvious reason for that ...
Thanks for reminder. It will be updated in the next version.
> 
> > PcdUse1GPageTable is set to FALSE by default in OvmfPkgX64.dsc. It
> > gives no chance for Intel TDX to support 1G page table. To support 1G
> > page table this PCD is set to TRUE in OvmfPkgX64.dsc.
> 
> Hmm, does this PCD allow using 1G pages (when supported), or does it require
> 1G page support?
PcdUse1GPageTable allows using 1G pages (when supported). Its default value is FALSE (in MdeModulePkg.dec). 
> 
> > +  } else if (AcceptPageSize == SIZE_2MB) {
> > +    //
> > +    // Total length is bigger than 2M and Page Accept size 2M is supported.
> > +    //
> > +    if ((PhysicalAddress & ALIGNED_2MB_MASK) == 0) {
> > +      //
> > +      // Start address is 2M aligned
> > +      //
> > +      StartAddress2 = PhysicalAddress;
> > +      Length2       = TotalLength & ~(UINT64)ALIGNED_2MB_MASK;
> > +
> > +      if (TotalLength > Length2) {
> > +        //
> > +        // There is remaining part 3)
> > +        //
> > +        StartAddress3 = StartAddress2 + Length2;
> > +        Length3       = TotalLength - Length2;
> > +        ASSERT (Length3 < SIZE_2MB);
> > +      }
> 
> I think I has some ideas to simplify all that math on the previous version of this
> series ...
Ah, yes. You presented a better solution to simplify all that math on the previous version. Sorry I missed it. It will be updated in the next version. Thanks for the reminder.
> 
> > @@ -756,13 +772,20 @@ SecCoreStartupWithStack (
> >    // we use a loop rather than CopyMem.
> >    //
> >    IdtTableInStack.PeiService = NULL;
> > +
> >    for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index++) {
> > -    UINT8  *Src;
> > -    UINT8  *Dst;
> > -    UINTN  Byte;
> > +    //
> > +    // Declare the local variables that actually move the data elements as
> > +    // volatile to prevent the optimizer from replacing this function with
> > +    // the intrinsic memcpy()
> > +    //
> > +    CONST UINT8     *Src;
> > +    volatile UINT8  *Dst;
> > +    UINTN           Byte;
> > +
> > +    Src = (CONST UINT8 *)&mIdtEntryTemplate;
> > +    Dst = (volatile UINT8 *)&IdtTableInStack.IdtTable[Index];
> >
> > -    Src = (UINT8 *)&mIdtEntryTemplate;
> > -    Dst = (UINT8 *)&IdtTableInStack.IdtTable[Index];
> >      for (Byte = 0; Byte < sizeof (mIdtEntryTemplate); Byte++) {
> >        Dst[Byte] = Src[Byte];
> >      }
> 
> This looks like an unrelated bugfix,  Move to separate patch?
Ok. I will create a separate patch.

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86135): https://edk2.groups.io/g/devel/message/86135
Mute This Topic: https://groups.io/mt/88617529/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-