> +// PageSize is mapped to PageLevel like below:
> +// 4KB - 0, 2MB - 1
> +UINT64 mTdxAcceptPageLevelMap[2] = {
> + SIZE_4KB,
> + SIZE_2MB
No 1G pages?
> +UINTN
> +GetGpaPageLevel (
> + UINT64 PageSize
Uh, UINT32 is not enough? Keep the door open for 512G pages?
> +{
> + UINTN Index;
> +
> + for (Index = 0; Index < sizeof (mTdxAcceptPageLevelMap) / sizeof
> (mTdxAcceptPageLevelMap[0]); Index++) {
There is an ARRAY_SIZE() macro, no need to open code the sizeof() trick.
> + if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> + break;
> + }
> + }
> +
> + return Index;
> +}
No error handling (invalid PageSize) here?
> +/**
> + This function accept a pending private page, and initialize the page to
> + all-0 using the TD ephemeral private key.
> +
> + Sometimes TDCALL [TDG.MEM.PAGE.ACCEPT] may return
> + TDX_EXIT_REASON_PAGE_SIZE_MISMATCH. It indicates the input PageLevel is
> + not workable. In this case we need to try to fallback to a smaller
> + PageLevel if possible.
> +
> + @param[in] StartAddress Guest physical address of the private
> + page to accept.
> + @param[in] NumberOfPages Number of the pages to be accepted.
> + @param[in] PageSize GPA page size. Only accept 1G/2M/4K size.
> +
> + @return EFI_SUCCESS Accept successfully
> + @return others Indicate other errors
> +**/
> +EFI_STATUS
> +EFIAPI
> +TdAcceptPages (
> + IN UINT64 StartAddress,
> + IN UINT64 NumberOfPages,
> + IN UINT64 PageSize
> + )
> +{
> + EFI_STATUS Status;
> + UINT64 Address;
> + UINT64 TdxStatus;
> + UINT64 Index;
> + UINT64 GpaPageLevel;
> + UINT64 PageSize2;
> +
> + Address = StartAddress;
> +
> + GpaPageLevel = (UINT64) GetGpaPageLevel (PageSize);
Why cast?
> + if (GpaPageLevel > sizeof (mTdxAcceptPageLevelMap) / sizeof
> (mTdxAcceptPageLevelMap[0])) {
> + DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size
> - 0x%llx\n", PageSize));
Ah. Errors are catched here. Well, no. The check is wrong,
it should be ">=" not ">".
Better would be GetGpaPageLevel explicitly returning a specific value
(for example -1) on error.
> + Status = EFI_SUCCESS;
> + for (Index = 0; Index < NumberOfPages; Index++) {
> + TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel, 0, 0,
> 0);
> + if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
> + if ((TdxStatus & ~0xFFFFULL) ==
> TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
> + //
> + // Already accepted
> + //
> + mNumberOfDuplicatedAcceptedPages++;
Hmm. When this happens we have a bug somewhere, right?
So should this be an assert()?
Or should we at least log the address?
> +#define RTMR_COUNT 4
> +#define TD_EXTEND_BUFFER_LEN (64 + 64)
> +#define EXTEND_BUFFER_ADDRESS_MASK 0x3f
> +
> +
> +#pragma pack(16)
> +typedef struct {
> + UINT8 Buffer[TD_EXTEND_BUFFER_LEN];
> +} TDX_EXTEND_BUFFER;
> +#pragma pack()
> +
> +UINT8 *mExtendBufferAddress = NULL;
> +TDX_EXTEND_BUFFER mExtendBuffer;
> +
> +/**
> + TD.RTMR.EXTEND requires 64B-aligned guest physical address of
> + 48B-extension data. In runtime we walk thru the Buffer to find
> + out a 64B-aligned start address.
Can't you just use __attribute__((aligned(64))) for that?
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81810): https://edk2.groups.io/g/devel/message/81810
Mute This Topic: https://groups.io/mt/86085729/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
