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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
