From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.9697.1672995082578294537 for ; Fri, 06 Jan 2023 00:51:22 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=ETsLVJa1; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: jiewen.yao@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1672995082; x=1704531082; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=rZZ6oijzle35mwynfeWBmjqU3nuTbbJ/QJGyBEjTTr8=; b=ETsLVJa1tk7E2TCoE5aJzf5wNZxLALjvdvS1/yWsilIO2OWGok8+y9gx vwBQRXyXpbgLeIKE4He6plgTU1pG0gpUJLIVp7U/qx9A8InBOK6nKFZoY qtWerBgOPGRln5Zf4IKezA1txtDo2kpzcEZHhTrhjFWjfw0QjejlsymxC 3V3YSbyjGLyJqMXiE2upFLpaPAx8o8r42q8Pd0rMydu2T75x/RRzzpbg3 1JqRry8Pk+fNgeSNT8t+qwRR6D/nD0Etgbx8/nM8zKOnam6qaKQwHjT3T Qq3qe8hegC7WAbNqMxxnpHciRFSO1CUiMCaxrvYvQRvkxLiPsxhlK998s A==; X-IronPort-AV: E=McAfee;i="6500,9779,10581"; a="322505664" X-IronPort-AV: E=Sophos;i="5.96,304,1665471600"; d="scan'208";a="322505664" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jan 2023 00:51:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10581"; a="829845613" X-IronPort-AV: E=Sophos;i="5.96,304,1665471600"; d="scan'208";a="829845613" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga005.jf.intel.com with ESMTP; 06 Jan 2023 00:51:21 -0800 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 6 Jan 2023 00:51:20 -0800 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 6 Jan 2023 00:51:20 -0800 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Fri, 6 Jan 2023 00:51:20 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.100) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Fri, 6 Jan 2023 00:51:20 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iKddpkSh1NiuFp+VrI59rNFjFOd0TEA3M6JpjzASgEc6kYwqh5nSolb/Sm5b3v0SVl/76y9p+TvJdt5BfKYfCKiEuLsV7/vVPlTYtU1EWkAVAoEej5dx2hSgQ8vMCv+VWKPvG86uqto+GCvVYI6OOqBPSlTb+IGLHhZPvoV5Yc9ebcq0JhI7vC8S8zIUGH9punKibsPmVzpnTEamBfa0at/bX4iB9HCb+6z2qu4Xdwrx8vsQiBFRvCEuWzyatpkF1I2oEPDsgofD6d6O7F8r10hwGTHZxF9P3KUFH0m6dl1qqu4eW+pXrlSCuHj4E6zR4LkY9IU4m2Q6+SHHppwrPQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7MMcWJR572NIg36Ph0ysRChR2YnPgHQpxD6O0FTiwu4=; b=K5E90Ke5ouhsjvaXdLKUeNmnmSekqM4EOmaLaVZlhxtj+wXLr8y9MmwgqeDGbRZIbCzCaztwDkrbHAYM9rKXWWdKZshFATpKeZDgNLL3F5M31m/Bekc6ySjjAwkiWxCaX97TpuE+EUTmOaaMUu4rHuDODRLIoH8l9KbHYsWisxCKo4ScjwYBNYc/x24kmu0Gy7STQFwYwKgM36KxeyZquVH3LHDMnh/IDfvFkkF9PYq3QUSKtc11rLeRfc9qPds5sbfxMDOQ1dRF++FEBXqEmhfJsSToMgBCmPm1rCLazAuyY0QvdTkZNt3dzzVn+tvmQOej7oCFWx7zZRMJIRw0lQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from MW4PR11MB5872.namprd11.prod.outlook.com (2603:10b6:303:169::14) by BN9PR11MB5339.namprd11.prod.outlook.com (2603:10b6:408:118::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5944.19; Fri, 6 Jan 2023 08:51:16 +0000 Received: from MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041]) by MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041%9]) with mapi id 15.20.5944.019; Fri, 6 Jan 2023 08:51:16 +0000 From: "Yao, Jiewen" To: "Xu, Min M" , "devel@edk2.groups.io" CC: "Aktas, Erdem" , James Bottomley , Gerd Hoffmann , Tom Lendacky , Ryan Afranji Subject: Re: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Thread-Topic: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Thread-Index: AQHZG2O5hloAvzjFoEGwdudyg9Kzz66RGdoQ Date: Fri, 6 Jan 2023 08:51:16 +0000 Message-ID: References: <20221229085548.476-1-min.m.xu@intel.com> <20221229085548.476-3-min.m.xu@intel.com> In-Reply-To: <20221229085548.476-3-min.m.xu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MW4PR11MB5872:EE_|BN9PR11MB5339:EE_ x-ms-office365-filtering-correlation-id: 29086f71-c4be-4627-7bf1-08daefc32c31 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: hQvw99P2sOPCzIlleM31NQLjGpXdUM9ni4rFJGtKlWaVehf2oQL0DeONdKuZSad+17rl6XOn2qDWHcuQWFBDiFVuaIhAoFhovculEE4kDtCN6/5rEjsou2jaYKEBhQTzUtm27TSnPdU96MDt6k5SKpcOJ+PpmQt0JN/G4F3HcU/CuIVWnBSkp5gdWqEDHf4lbz1lX1h1Y9NHnPeha/zPnal6PXAUtrAKHfYRkWeHRHCGsAt7NMvTcWD8XARi+dnqlU8nmCrGOD6WQDwe7rhxlWoHxmuxolXnOk48Zb34n63FQKxT6YlIy9iZholohawqV+ZLZDs6/w3X337hXM/w8UwOgiiOngMiX7fnZdIHc7WDOE73ESaTPeQfyxxeIhyTg6E1b4yDLIGV8/Lr7hscB1PiUBtqNdVG7lyO/SWJMZDdKZ75PQRugy+orsGqJRCzoCoVuLniwRsETT4NH1HKcd7Zj4c+UWAGB0CERlWqhYV5ys4R2vbYOUovv4w0Hd4Yc/5P0Gdxn2Bgw8SbXZtlySTH02YFl7b/zoo/bZ+lBPGTbcYpLlvv3og0rbf1YpA21Z+tXFOzNfTXB9nA6+KNCiVG9Fvq+SdshAG+slwzKvYhdQcqCyDbcnWCfM1hPjyaD9IMdZ5iZFYaH2mrnsRyW+HQ+on4ZchM1J80BM/nUZ/f9N4LDkWIS8nSlpHSfbAn8rB6ae2SZlZOJx9YY9P19fxrabtE7/duImmj+6yUdKw= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5872.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(6029001)(39860400002)(376002)(366004)(136003)(396003)(346002)(451199015)(186003)(26005)(53546011)(38100700002)(38070700005)(82960400001)(9686003)(55016003)(83380400001)(86362001)(122000001)(30864003)(41300700001)(66946007)(52536014)(966005)(316002)(8936002)(66556008)(8676002)(4326008)(66476007)(76116006)(66446008)(5660300002)(478600001)(2906002)(7696005)(64756008)(6506007)(110136005)(71200400001)(54906003)(33656002)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?0YIbPuqDazqrFCbCRpZhzfbSjXgypAjz141s77ZjvyN2w/A6KSRbtayGLm8s?= =?us-ascii?Q?bvCYdGsTADtRAyhzqBTH70kfqlKt68Yk1yDkWBt4N5F/f9qbZDl77xDPRWBL?= =?us-ascii?Q?F9kddFXLnZZtdB405Dk3pZg8r0Ta8e8VItFDPUD05k2xRQ2oOF4jxYIAWYJz?= =?us-ascii?Q?yFcip+LB5SQPkCqqdejbltVb4o92HZFLO2u8R44wYYFnPTZNjCBTcj4PCl1s?= =?us-ascii?Q?Vsr5gwndqkZvvNrhec0bsjSBuBMA4m/jODQpslU/fqctRUUvxbCSsxT2gvG6?= =?us-ascii?Q?xGUJRLeUsezQ0PlHXp0BH534cOlpxFJOEuYscKm8mh0jEiqI16+BGmYqmi3v?= =?us-ascii?Q?RHhkLPsi7vE/hg3uBUa8JfXC45HSEs8sD1llMkjRofvjy0BirgsoooUqJINM?= =?us-ascii?Q?n1epJy4zMaw2jfC5erFonJ50GhQZv0hwCHL4jut0gc2mMADKwaaXDpbTIpmY?= =?us-ascii?Q?PSDQYJhR688JyVCHw9mm3PQH6ICuMzAlqTC/0GlZQ8Q2VVUuZMPE1POFBg3W?= =?us-ascii?Q?K2wJMN1HTswHPLF/RtF3hLj3s1th+zrLcN6PE0n9LCVO4y/3mg7BNumvAj2Q?= =?us-ascii?Q?3OwqRZkkzaZ8RUGzo87LPnZ5b4DQfmMFB85JUkIBT1wIaSS8XjtHv8FT4Yk7?= =?us-ascii?Q?W3izYDrBDIJXzHVAxpACneDgvvAWX2kvPbz3hvkfeeKrcRh8P6gj3J+r345b?= =?us-ascii?Q?YJdkBQ0EiE59drQVLFOtJVLaX9YZVMLNJqipjzBrX1I9ljNlEhBkvI0pfywe?= =?us-ascii?Q?b9bOZTMQzFYTKWK7RNSBE+pywZDphwQMwe4e5Re40W/Ijo7CPXmcnZ+guhMP?= =?us-ascii?Q?8OWo20EK2ON9JuwGFduFu3RskCF0UCF2GiG6L39KwnrUXotYFLUKa4Hcbbce?= =?us-ascii?Q?qADiD2qmUZDDx18w2g7b14rKWBxAlTkQBXcXuvW6cEFhhHGfH7UfDiBY7zgm?= =?us-ascii?Q?KvIRf3O69EGgqxjQ7/G9O9Ac7knxa6FBrDMqhWpHj78jrinzZfeELSTAmXWQ?= =?us-ascii?Q?mUISCJNVnY05frOZtXF1tyQdXcQJ+ApKcFFip5U7PIKYhIRA7n6QWBavHZSC?= =?us-ascii?Q?2roJHH+HX4JgySm6LYhf+2jVFjR2car8IZ938DwLeKVHJeOPp4QsNZruaOR4?= =?us-ascii?Q?mKvBNUhSPlME8Xmuz0s6weNL8pN55GosZLpxqd2SvtDMfjO5A/TvXr6jRek8?= =?us-ascii?Q?B1kl+aV8Q7m+lEMElUVEJyJFBjfR49jilIbKeNeYWPaWnUiYwGkox9YfKBVh?= =?us-ascii?Q?YqqfUnEXS8QqWY0Hfemi/RyrYj/lXRocKm/lREbPEMGGuZfdPPMLVmnxr1hq?= =?us-ascii?Q?YYEWanxBmS2iLEzWeZTsyrZD3QQxxPxfsVsuzF4QfqaU9YCgXeud3dkpPz53?= =?us-ascii?Q?Wut9dZSoklzOi0PlhusAoE7T15T61Uk/onJsIbPGHHiddB2VHvdTSv0yGjk5?= =?us-ascii?Q?W6BYFmQFjYal42Xu1pEsgspHgwo3gHJIKC116NiQcZsnfqa1oNgOZ1J8+YXu?= =?us-ascii?Q?31sFCP5zx3UucpU4hdB70GJmxrKzotX3gIzsTC9QIyI9xE6Nm4WigOE59K8t?= =?us-ascii?Q?megUztRQaKFKVENliZBUHDR/le285+EiNuzEOzmU?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5872.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 29086f71-c4be-4627-7bf1-08daefc32c31 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2023 08:51:16.5136 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: m4GhPxtUdWdreyuzBgpU4w1AUkfxHEf3S0sfy94alElXkczvQi8cRU9mpxZLovmYA3LxsdpKkDS1W02SajTUJA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5339 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for the cleanup. Comments inline. In general, we need ensure CpuDeadLoop() for *any* parsing error in VE hand= ler. Just ASSERT or VmCall(HALT) is not enough, because ASSERT =3D=3D nothi= ng in release, while VmCall(HALT) is not trusted. ASSERT can only be used to handle internal impossible logic, but not input = from VMM. Thank you Yao, Jiewen > -----Original Message----- > From: Xu, Min M > Sent: Thursday, December 29, 2022 4:56 PM > To: devel@edk2.groups.io > Cc: Xu, Min M ; Aktas, Erdem > ; James Bottomley ; Yao, > Jiewen ; Gerd Hoffmann ; > Tom Lendacky ; Ryan Afranji > > Subject: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit >=20 > From: Min M Xu >=20 > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4169 >=20 > The previous TDX MmioExit doesn't handle the Mmio instructions correctly > in some scenarios. This patch refactors the implementation to fix the > issues. >=20 > Cc: Erdem Aktas > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Gerd Hoffmann > Cc: Tom Lendacky > Cc: Ryan Afranji > Reported-by: Ryan Afranji > Signed-off-by: Min Xu > --- > OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498 ++++++++++++++------ > 1 file changed, 347 insertions(+), 151 deletions(-) >=20 > diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c > b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c > index 30d547d5fe55..e0998722af39 100644 > --- a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c > +++ b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c > @@ -13,6 +13,10 @@ > #include > #include > #include > +#include "CcInstruction.h" > + > +#define TDX_MMIO_READ 0 > +#define TDX_MMIO_WRITE 1 >=20 > typedef union { > struct { > @@ -216,14 +220,15 @@ STATIC > VOID > EFIAPI > TdxDecodeInstruction ( > - IN UINT8 *Rip > + IN UINT8 *Rip, > + IN UINT32 Length > ) > { > UINTN i; >=20 > DEBUG ((DEBUG_INFO, "TDX: #TD[EPT] instruction (%p):", Rip)); > - for (i =3D 0; i < 15; i++) { > - DEBUG ((DEBUG_INFO, "%02x:", Rip[i])); > + for (i =3D 0; i < MIN (15, Length); i++) { > + DEBUG ((DEBUG_INFO, "%02x ", Rip[i])); > } >=20 > DEBUG ((DEBUG_INFO, "\n")); > @@ -235,50 +240,324 @@ TdxDecodeInstruction ( > TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \ [Jiewen] Please put CpuDeadLoop() here. > } >=20 > +/** > + * Tdx MMIO access via TdVmcall. > + * > + * @param MmioSize Size of the MMIO access > + * @param ReadOrWrite Read or write operation > + * @param GuestPA Guest physical address > + * @param Val Pointer to the value which is read or written > + > + * @retval EFI_SUCCESS Successfully access the mmio > + * @retval Others Other errors as indicated > + */ > STATIC > -UINT64 * > -EFIAPI > -GetRegFromContext ( > - IN EFI_SYSTEM_CONTEXT_X64 *Regs, > - IN UINTN RegIndex > +UINT64 [Jiewen] According to the return code in the function, I think we need use = EFI_STATUS here. > +TdxMmioReadWrite ( > + IN UINT32 MmioSize, > + IN UINT32 ReadOrWrite, > + IN UINT64 GuestPA, > + IN UINT64 *Val > ) > { > - switch (RegIndex) { > - case 0: return &Regs->Rax; > - break; > - case 1: return &Regs->Rcx; > - break; > - case 2: return &Regs->Rdx; > - break; > - case 3: return &Regs->Rbx; > - break; > - case 4: return &Regs->Rsp; > - break; > - case 5: return &Regs->Rbp; > - break; > - case 6: return &Regs->Rsi; > - break; > - case 7: return &Regs->Rdi; > - break; > - case 8: return &Regs->R8; > - break; > - case 9: return &Regs->R9; > + UINT64 Status; > + > + if ((MmioSize !=3D 1) && (MmioSize !=3D 2) && (MmioSize !=3D 4) && > (MmioSize !=3D 8)) { > + ASSERT (FALSE); > + return EFI_INVALID_PARAMETER; > + } > + > + if (Val =3D=3D NULL) { > + ASSERT (FALSE); > + return EFI_INVALID_PARAMETER; > + } > + > + if (ReadOrWrite =3D=3D TDX_MMIO_READ) { > + Status =3D TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_READ, > GuestPA, 0, Val); > + } else if (ReadOrWrite =3D=3D TDX_MMIO_WRITE) { > + Status =3D TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_WRITE, > GuestPA, *Val, 0); > + } else { > + ASSERT (FALSE); > + Status =3D EFI_INVALID_PARAMETER; > + } > + > + return Status; > +} [Jiewen] I checked error state. I think we need add CpuDeadLoo() after TdVmCall(TDVMCALL_HALT) below: if (Status) { DEBUG (( DEBUG_ERROR, "#VE Error (0x%llx) returned from host, ExitReason is %d, ExitQualifi= cation =3D 0x%x.\n", Status, ReturnData.VeInfo.ExitReason, ReturnData.VeInfo.ExitQualification.Val )); TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0); } In general, please CpuDeadLoop() for all TDVMCALL_HALT in CcExitHandleVe() > + > +typedef struct { > + UINT8 OpCode; > + UINT32 Bytes; > + EFI_PHYSICAL_ADDRESS Address; > + UINT64 Val; > + UINT64 *Register; > + UINT32 ReadOrWrite; > +} MMIO_EXIT_PARSED_INSTRUCTION; > + > +/** > + * Parse the MMIO instructions. > + * > + * @param Regs Pointer to the EFI_SYSTEM_CONTEXT_X64 which > includes the instructions > + * @param InstructionData Pointer to the CC_INSTRUCTION_DATA > + * @param ParsedInstruction Pointer to the parsed instruction data > + * > + * @retval EFI_SUCCESS Successfully parsed the instructions > + * @retval Others Other error as indicated > + */ > +STATIC > +EFI_STATUS > +ParseMmioExitInstructions ( > + IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs, > + IN OUT CC_INSTRUCTION_DATA *InstructionData, > + OUT MMIO_EXIT_PARSED_INSTRUCTION *ParsedInstruction > + ) > +{ > + EFI_STATUS Status; > + UINT8 OpCode; > + UINT8 SignByte; > + UINT32 Bytes; > + EFI_PHYSICAL_ADDRESS Address; > + UINT64 Val; > + UINT64 *Register; > + UINT32 ReadOrWrite; > + > + Address =3D 0; > + Bytes =3D 0; > + Register =3D NULL; > + Status =3D EFI_SUCCESS; > + Val =3D 0; > + > + Status =3D CcInitInstructionData (InstructionData, NULL, Regs); > + if (Status !=3D EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "%a: Initialize InstructionData failed! (%r)\n"= , > __FUNCTION__, Status)); > + ASSERT (FALSE); > + return Status; > + } > + > + OpCode =3D *(InstructionData->OpCodes); > + if (OpCode =3D=3D TWO_BYTE_OPCODE_ESCAPE) { > + OpCode =3D *(InstructionData->OpCodes + 1); > + } > + > + switch (OpCode) { > + // > + // MMIO write (MOV reg/memX, regX) > + // > + case 0x88: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0x89: > + CcDecodeModRm (Regs, InstructionData); > + Bytes =3D ((Bytes !=3D 0) ? Bytes : > + (InstructionData->DataSize =3D=3D Size16Bits) ? 2 : > + (InstructionData->DataSize =3D=3D Size32Bits) ? 4 : > + (InstructionData->DataSize =3D=3D Size64Bits) ? 8 : > + 0); > + > + if (InstructionData->Ext.ModRm.Mod =3D=3D 3) { > + DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode: > 0x%x)\n", __FUNCTION__, OpCode)); > + ASSERT (FALSE); > + return EFI_UNSUPPORTED; > + } > + > + Address =3D InstructionData->Ext.RmData; > + Val =3D InstructionData->Ext.RegData; > + ReadOrWrite =3D TDX_MMIO_WRITE; > + > break; > - case 10: return &Regs->R10; > + > + // > + // MMIO write (MOV moffsetX, aX) > + // > + case 0xA2: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0xA3: > + Bytes =3D ((Bytes !=3D 0) ? Bytes : > + (InstructionData->DataSize =3D=3D Size16Bits) ? 2 : > + (InstructionData->DataSize =3D=3D Size32Bits) ? 4 : > + (InstructionData->DataSize =3D=3D Size64Bits) ? 8 : > + 0); > + > + InstructionData->ImmediateSize =3D (UINTN)(1 << InstructionData- > >AddrSize); > + InstructionData->End +=3D InstructionData->ImmediateSize; > + CopyMem (&Address, InstructionData->Immediate, InstructionData- > >ImmediateSize); > + > + Val =3D Regs->Rax; > + ReadOrWrite =3D TDX_MMIO_WRITE; > break; > - case 11: return &Regs->R11; > + > + // > + // MMIO write (MOV reg/memX, immX) > + // > + case 0xC6: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0xC7: > + CcDecodeModRm (Regs, InstructionData); > + Bytes =3D ((Bytes !=3D 0) ? Bytes : > + (InstructionData->DataSize =3D=3D Size16Bits) ? 2 : > + (InstructionData->DataSize =3D=3D Size32Bits) ? 4 : > + (InstructionData->DataSize =3D=3D Size64Bits) ? 8 : > + 0); > + > + InstructionData->ImmediateSize =3D Bytes; > + InstructionData->End +=3D Bytes; > + > + Val =3D 0; > + CopyMem (&Val, InstructionData->Immediate, InstructionData- > >ImmediateSize); > + > + Address =3D InstructionData->Ext.RmData; > + ReadOrWrite =3D TDX_MMIO_WRITE; > + > break; > - case 12: return &Regs->R12; > + > + // > + // MMIO read (MOV regX, reg/memX) > + // > + case 0x8A: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0x8B: > + CcDecodeModRm (Regs, InstructionData); > + Bytes =3D ((Bytes !=3D 0) ? Bytes : > + (InstructionData->DataSize =3D=3D Size16Bits) ? 2 : > + (InstructionData->DataSize =3D=3D Size32Bits) ? 4 : > + (InstructionData->DataSize =3D=3D Size64Bits) ? 8 : > + 0); > + if (InstructionData->Ext.ModRm.Mod =3D=3D 3) { > + // > + // NPF on two register operands??? > + // > + DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode: > 0x%x)\n", __FUNCTION__, OpCode)); > + ASSERT (FALSE); [Jiewen] I am not sure if it is useful to put ASSERT here. If this is possible case, but we don't want to support, just return UNSUPPO= RTED without ASSERT. It will cause CpuDeadLoop() later anyway. > + return EFI_UNSUPPORTED; > + } > + > + Address =3D InstructionData->Ext.RmData; > + ReadOrWrite =3D TDX_MMIO_READ; > + > + Register =3D CcGetRegisterPointer (Regs, InstructionData->Ext.ModR= m.Reg); > + if (Bytes =3D=3D 4) { > + // > + // Zero-extend for 32-bit operation > + // > + *Register =3D 0; > + } > + > break; > - case 13: return &Regs->R13; > + > + // > + // MMIO read (MOV aX, moffsetX) > + // > + case 0xA0: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0xA1: > + Bytes =3D ((Bytes !=3D 0) ? Bytes : > + (InstructionData->DataSize =3D=3D Size16Bits) ? 2 : > + (InstructionData->DataSize =3D=3D Size32Bits) ? 4 : > + (InstructionData->DataSize =3D=3D Size64Bits) ? 8 : > + 0); > + > + InstructionData->ImmediateSize =3D (UINTN)(1 << InstructionData- > >AddrSize); > + InstructionData->End +=3D InstructionData->ImmediateSize; > + > + Address =3D 0; > + CopyMem ( > + &Address, > + InstructionData->Immediate, > + InstructionData->ImmediateSize > + ); > + > + if (Bytes =3D=3D 4) { > + // > + // Zero-extend for 32-bit operation > + // > + Regs->Rax =3D 0; > + } > + > + Register =3D &Regs->Rax; > + ReadOrWrite =3D TDX_MMIO_READ; > + > break; > - case 14: return &Regs->R14; > + > + // > + // MMIO read w/ zero-extension ((MOVZX regX, reg/memX) > + // > + case 0xB6: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0xB7: > + CcDecodeModRm (Regs, InstructionData); > + Bytes =3D (Bytes !=3D 0) ? Bytes : 2; > + Address =3D InstructionData->Ext.RmData; > + > + Register =3D CcGetRegisterPointer (Regs, InstructionData->Ext.ModR= m.Reg); > + SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0); > + > + ReadOrWrite =3D TDX_MMIO_READ; > + > break; > - case 15: return &Regs->R15; > + > + // > + // MMIO read w/ sign-extension (MOVSX regX, reg/memX) > + // > + case 0xBE: > + Bytes =3D 1; > + // > + // fall through > + // > + case 0xBF: > + CcDecodeModRm (Regs, InstructionData); > + Bytes =3D (Bytes !=3D 0) ? Bytes : 2; > + > + Address =3D InstructionData->Ext.RmData; > + > + if (Bytes =3D=3D 1) { > + UINT8 *Data; > + Data =3D (UINT8 *)&Val; > + SignByte =3D ((*Data & BIT7) !=3D 0) ? 0xFF : 0x00; > + } else { > + UINT16 *Data; > + Data =3D (UINT16 *)&Val; > + SignByte =3D ((*Data & BIT15) !=3D 0) ? 0xFF : 0x00; > + } > + > + Register =3D CcGetRegisterPointer (Regs, InstructionData->Ext.ModR= m.Reg); > + SetMem (Register, (UINTN)(1 << InstructionData->DataSize), SignByt= e); > + > + ReadOrWrite =3D TDX_MMIO_READ; > + > break; > + > + default: > + DEBUG ((DEBUG_ERROR, "%a: Invalid MMIO opcode (%x)\n", > __FUNCTION__, OpCode)); > + Status =3D EFI_UNSUPPORTED; > + ASSERT (FALSE); > + } > + > + if (!EFI_ERROR (Status)) { > + ParsedInstruction->OpCode =3D OpCode; > + ParsedInstruction->Address =3D Address; > + ParsedInstruction->Bytes =3D Bytes; > + ParsedInstruction->Register =3D Register; > + ParsedInstruction->Val =3D Val; > + ParsedInstruction->ReadOrWrite =3D ReadOrWrite; > } >=20 > - return NULL; > + return Status; > } >=20 > /** > @@ -300,25 +579,13 @@ MmioExit ( > IN TDCALL_VEINFO_RETURN_DATA *Veinfo > ) > { > - UINT64 Status; > - UINT32 MmioSize; > - UINT32 RegSize; > - UINT8 OpCode; > - BOOLEAN SeenRex; > - UINT64 *Reg; > - UINT8 *Rip; > - UINT64 Val; > - UINT32 OpSize; > - MODRM ModRm; > - REX Rex; > - TD_RETURN_DATA TdReturnData; > - UINT8 Gpaw; > - UINT64 TdSharedPageMask; > - > - Rip =3D (UINT8 *)Regs->Rip; > - Val =3D 0; > - Rex.Val =3D 0; > - SeenRex =3D FALSE; > + UINT64 Status; > + TD_RETURN_DATA TdReturnData; > + UINT8 Gpaw; > + UINT64 Val; > + UINT64 TdSharedPageMask; > + CC_INSTRUCTION_DATA InstructionData; > + MMIO_EXIT_PARSED_INSTRUCTION ParsedInstruction; >=20 > Status =3D TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData); > if (Status =3D=3D TDX_EXIT_REASON_SUCCESS) { > @@ -335,112 +602,41 @@ MmioExit ( > CpuDeadLoop (); > } >=20 > - // > - // Default to 32bit transfer > - // > - OpSize =3D 4; > + Status =3D ParseMmioExitInstructions (Regs, &InstructionData, > &ParsedInstruction); > + if (Status !=3D EFI_SUCCESS) { > + return Status; > + } >=20 > - do { > - OpCode =3D *Rip++; > - if (OpCode =3D=3D 0x66) { > - OpSize =3D 2; > - } else if ((OpCode =3D=3D 0x64) || (OpCode =3D=3D 0x65) || (OpCode = =3D=3D 0x67)) { > - continue; > - } else if ((OpCode >=3D 0x40) && (OpCode <=3D 0x4f)) { > - SeenRex =3D TRUE; > - Rex.Val =3D OpCode; > - } else { > - break; > + if (Veinfo->GuestPA !=3D (ParsedInstruction.Address | TdSharedPageMask= )) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: Address is not correct! (%d: 0x%llx !=3D 0x%llx)\n", > + __FUNCTION__, > + ParsedInstruction.OpCode, > + Veinfo->GuestPA, > + ParsedInstruction.Address > + )); > + ASSERT (FALSE); [Jiewen] Not sure if ASSERT here is useful. We should DeadLoop() later. > + return EFI_ABORTED; > + } > + > + if (ParsedInstruction.ReadOrWrite =3D=3D TDX_MMIO_WRITE ) { > + Status =3D TdxMmioReadWrite (ParsedInstruction.Bytes, > TDX_MMIO_WRITE, Veinfo->GuestPA, &ParsedInstruction.Val); > + } else { > + Val =3D 0; > + Status =3D TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_READ, > Veinfo->GuestPA, &Val); > + if (Status =3D=3D 0) { > + CopyMem (ParsedInstruction.Register, &Val, ParsedInstruction.Bytes= ); > } > - } while (TRUE); > - > - // > - // We need to have at least 2 more bytes for this instruction > - // > - TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 13); [Jiewen] We need make sure it will DeadLoop() if TDX_DECODER_BUG_ON is ON. > - > - OpCode =3D *Rip++; > - // > - // Two-byte opecode, get next byte > - // > - if (OpCode =3D=3D 0x0F) { > - OpCode =3D *Rip++; > - } > - > - switch (OpCode) { > - case 0x88: > - case 0x8A: > - case 0xB6: > - MmioSize =3D 1; > - break; > - case 0xB7: > - MmioSize =3D 2; > - break; > - default: > - MmioSize =3D Rex.Bits.W ? 8 : OpSize; > - break; > - } > - > - /* Punt on AH/BH/CH/DH unless it shows up. */ > - ModRm.Val =3D *Rip++; > - TDX_DECODER_BUG_ON (MmioSize =3D=3D 1 && ModRm.Bits.Reg > 4 > && !SeenRex && OpCode !=3D 0xB6); > - Reg =3D GetRegFromContext (Regs, ModRm.Bits.Reg | ((int)Rex.Bits.R << = 3)); > - TDX_DECODER_BUG_ON (!Reg); > - > - if (ModRm.Bits.Rm =3D=3D 4) { > - ++Rip; /* SIB byte */ > - } > - > - if ((ModRm.Bits.Mod =3D=3D 2) || ((ModRm.Bits.Mod =3D=3D 0) && > (ModRm.Bits.Rm =3D=3D 5))) { > - Rip +=3D 4; /* DISP32 */ > - } else if (ModRm.Bits.Mod =3D=3D 1) { > - ++Rip; /* DISP8 */ > - } > - > - switch (OpCode) { > - case 0x88: > - case 0x89: > - CopyMem ((void *)&Val, Reg, MmioSize); > - Status =3D TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA, > Val, 0); > - break; > - case 0xC7: > - CopyMem ((void *)&Val, Rip, OpSize); > - Status =3D TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA, > Val, 0); > - Rip +=3D OpSize; > - default: > - // > - // 32-bit write registers are zero extended to the full register > - // Hence 'MOVZX r[32/64], r/m16' is > - // hardcoded to reg size 8, and the straight MOV case has a reg > - // size of 8 in the 32-bit read case. > - // > - switch (OpCode) { > - case 0xB6: > - RegSize =3D Rex.Bits.W ? 8 : OpSize; > - break; > - case 0xB7: > - RegSize =3D 8; > - break; > - default: > - RegSize =3D MmioSize =3D=3D 4 ? 8 : MmioSize; > - break; > - } > - > - Status =3D TdVmCall (TDVMCALL_MMIO, MmioSize, 0, Veinfo->GuestPA, = 0, > &Val); > - if (Status =3D=3D 0) { [Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status) > - ZeroMem (Reg, RegSize); > - CopyMem (Reg, (void *)&Val, MmioSize); > - } > } >=20 > if (Status =3D=3D 0) { [Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status) > - TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 15); > - > // > // We change instruction length to reflect true size so handler can > // bump rip > // > - Veinfo->ExitInstructionLength =3D (UINT32)((UINT64)Rip - Regs->Rip)= ; > + Veinfo->ExitInstructionLength =3D (UINT32)(CcInstructionLength > (&InstructionData)); > + TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo- > >ExitInstructionLength); > } >=20 > return Status; > -- > 2.29.2.windows.2