From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web12.11364.1643198001081434171 for ; Wed, 26 Jan 2022 03:53:21 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ckTx9los; spf=pass (domain: redhat.com, ip: 170.10.129.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1643198000; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RrciadzB8UP1mkYKAm5F/cqeAGCGdAWc7ck1homEcEU=; b=ckTx9los2jKpaH74ekKMXpucloaC9WjfxgaBIVHb3Y5aS4waUU7EYz0fUn/92+EEpxMK0i AGy9WJ0rFBP0n7Aw95SvrjPcwwqQxic93MAhCokLv+g5lpmQ4+YGhrQBfhknD3KLKop9My d3uhWVbxr1ptUz2jO2iY8BOZgFIvGkw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-359-kdVv7uFRMI6VyAfAaHEQLQ-1; Wed, 26 Jan 2022 06:53:09 -0500 X-MC-Unique: kdVv7uFRMI6VyAfAaHEQLQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 66067814407; Wed, 26 Jan 2022 11:53:07 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.193.47]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EE80E73177; Wed, 26 Jan 2022 11:53:06 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 2BCBA18000AA; Wed, 26 Jan 2022 12:53:05 +0100 (CET) Date: Wed, 26 Jan 2022 12:53:05 +0100 From: "Gerd Hoffmann" To: Min Xu Cc: devel@edk2.groups.io, Ard Biesheuvel , Jordan Justen , Brijesh Singh , Erdem Aktas , James Bottomley , Jiewen Yao , Tom Lendacky Subject: Re: [PATCH V5 17/33] OvmfPkg: Update Sec to support Tdx Message-ID: <20220126115305.xyctbbbctqqvfyft@sirius.home.kraxel.org> References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, > 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 ... > 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? > + } 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 ... > @@ -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? take care, Gerd