From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web10.60590.1673227140746601856 for ; Sun, 08 Jan 2023 17:19:01 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=XyPNqRFv; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: min.m.xu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1673227140; x=1704763140; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=0UZzF6A6XnUnTk/i4RtpGCjZkQDhclhDjlwZru6vpbc=; b=XyPNqRFv4l1OvdJogysV/LS//xzysWeUmbk/29bPFwgTlY/pMm2d87M+ Cvm97n3bXXsRkNiCoUoCCGlqushO/1f/9+v4vZoy+rkLweYitktbqNLWV haeBgKfCEY0PuNAK99fq5mLElxkn8Xfj253o8eQsjQzvnqQfMS95sCvhP hz5nHotZd23ZpGZP/pQQ0WhLtOmZY4rFOMMQX1ABLilc2SGNs5V4xzoR7 OmGMHAZzLaT97ObuAd/sTI07HeRETn+ylMncF3FzW2CgKFUr2SmbaL2JT +bEMeFMWp8ZUgh6/MJOvvF1xVFZK8oDVjO/BuX9fXPx4ZlBcMVtGGa443 w==; X-IronPort-AV: E=McAfee;i="6500,9779,10584"; a="385072766" X-IronPort-AV: E=Sophos;i="5.96,311,1665471600"; d="scan'208";a="385072766" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jan 2023 17:18:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10584"; a="725003923" X-IronPort-AV: E=Sophos;i="5.96,311,1665471600"; d="scan'208";a="725003923" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga004.fm.intel.com with ESMTP; 08 Jan 2023 17:18:59 -0800 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Sun, 8 Jan 2023 17:18:58 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Sun, 8 Jan 2023 17:18:58 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Sun, 8 Jan 2023 17:18:58 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.41) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Sun, 8 Jan 2023 17:18:58 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZYAeS1ggRl8bHx5Flrk3eG3SWwz6AH/TtGPA0tzkE8jA7P6YytuA6b5HYE9YYiFOVcd2AwN6Cpj4WAPnoXie4R6v3iE2o6sid1S9pRVXt2Z0XGYI2a5XxBvm/GzanjZjhrdpyG7+U1RPkzUa5wtybEZ2XIhdjZ+Y43zcOmQ/bGVjCH9WguVMKUngGry5k2QrYS/vzJ+QLedeEb25msngyr7WXJdDufh/tg5v0Mfi4jBawkqu3Cd0pviYMVj0kRSwnA8J6gl6dDFrntfH14pK75ECpPRNu4HpbWvqSYgacutU/03Ba7vsIEc8RJUyBWpzRiPBwc7vQPMKvNUq0dwm2A== 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=UZGbpdfHqShxGcpmubs3ZZatZ08esVoEHjbgkkM00Lg=; b=l+XZ8BxssXZCTNXLjoGADsYdxKUMw5gcPCc5n5seeMJIjjYDBFXB3kPkvvG3Z4tBBgVmEex+ufo0kuHjbjqJ9RaazUJ2JeUwc0h4DYdtDJea5AtEd0Ssx1AKdYjqrKWfNsHQxjfC/s6ci4vmEz9rJVfN01bqqUPtfO7ebGLhHbRXaLhfViHXO8jU7QP7pvBncf/YfMkZWvxYGYPeVxvobwHlbAVJgWn9thMVBNvG1g/0D5p7Qda61zHRb6+mTp76g9H0Kj4dyKuZ2BoYSaBp79u9PuVFYFs41DmyUtRmO4uyYAqzRycFR3qiHVsKoXk2Dv9E/hUXr0Yy+RjG82Q2xA== 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 CO1PR11MB5058.namprd11.prod.outlook.com (2603:10b6:303:99::17) by DM6PR11MB4674.namprd11.prod.outlook.com (2603:10b6:5:2a0::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5986.18; Mon, 9 Jan 2023 01:18:56 +0000 Received: from CO1PR11MB5058.namprd11.prod.outlook.com ([fe80::6768:ff92:56c4:9ed6]) by CO1PR11MB5058.namprd11.prod.outlook.com ([fe80::6768:ff92:56c4:9ed6%3]) with mapi id 15.20.5986.018; Mon, 9 Jan 2023 01:18:56 +0000 From: "Min Xu" To: "Yao, Jiewen" , "devel@edk2.groups.io" CC: "Aktas, Erdem" , James Bottomley , Gerd Hoffmann , Tom Lendacky , "Afranji, Ryan" Subject: Re: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Thread-Topic: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Thread-Index: AQHZG2O52swcJGfLN0iVfMybc9k7Wq6RIUsAgAQ4grA= Date: Mon, 9 Jan 2023 01:18:56 +0000 Message-ID: References: <20221229085548.476-1-min.m.xu@intel.com> <20221229085548.476-3-min.m.xu@intel.com> In-Reply-To: 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: CO1PR11MB5058:EE_|DM6PR11MB4674:EE_ x-ms-office365-filtering-correlation-id: 539ec9f9-5caf-4b39-d646-08daf1df7a87 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: EPe1bLiVcqqYGTbZHg3ph7b7ic2KGKKedXt5h5v5LVKUiqIrtjG21DAfM69mcxJku3anDYZ2spfznNSD6yArS7IAKEotG4q7Dhi1QNkAqpk90fvuyG5rW9VepGRQMQLcyEi9bxG1jAv6xoOaUjoex/gUf0m7qHlMhokK77n+B+7Jha3aDk6d5MBqgbdClgb4WUuDohgWbFUiep/MjBr5cGI0z5y8ST33pw6cCrTsftXQJyB9HAVO8qEipfrSCaevmFyGJzwjkFidEoIi6eyQpRK/QG58AVmlzcJlP9SNEBfnNOWp39TMlQeT8a2rr2Wa7OCARoNw2DTQC2jYNJ4E84XCe0Tauo+lJsEMghg37oEFmq5MSGRnx/eDbedqHq8kuLawAVa8czs2ZLbRG36Y+eV1MBWvIvxn/no+BLlKjuEJ3JSzHf0QpUVcCKGcRDT027xEKT2MpG/bH67CxNKCbxOqRCCn/okDXa1bq1O4y0LcW8KmNOTiPwNy3tsthptWjrMCCHJAcCHKZze5PuqfyeoGe6tLrw+9LzJPyG6lFqGqA/CXJVsINuUirnEtABlWYtIWG+S/zbhBkfv42C+aEZk19NiEoHHzXVM823kXxitERVessAbM76/6uD5dxENtQm8r1h1wFrFxJgV30G8/pReOUOBVUEjqWe/vl+eqm8TAzCkfGFrIGw9ZaKKWuby9BOM57aJTI5kdky4Fcp112DhjAi+TT1vIGCDguwy5EX0= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO1PR11MB5058.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(6029001)(136003)(376002)(396003)(346002)(366004)(39860400002)(451199015)(4326008)(8676002)(66946007)(76116006)(66556008)(66476007)(64756008)(316002)(66446008)(7696005)(110136005)(54906003)(38070700005)(30864003)(2906002)(5660300002)(71200400001)(8936002)(52536014)(41300700001)(86362001)(83380400001)(53546011)(6506007)(966005)(478600001)(33656002)(82960400001)(122000001)(55016003)(9686003)(186003)(38100700002)(26005)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Tr71GhuCl2X0cccjsxGu8MSnTOYkpMlDA0InHXbYd0GnpBBletyYOGirmQF7?= =?us-ascii?Q?kg6ilvZX4/L5G2l1wsx23B8JN2eizoCvHBM+vcdLqaWixuu2Wj/olzz20ky9?= =?us-ascii?Q?T+lj5oqDmQZChPju0Rxo2ybKMr00cNlVsZ4R9I/xh9yiKDdR8Agv4Carn+Kr?= =?us-ascii?Q?OHAjMsXKDK8KOrelimwcCUp36tAWZrMRdcXFYiz7aeUs2953S7u8fnxQoMdF?= =?us-ascii?Q?lZ/omnjxfoONOzzIjjyVRNbsUc0fxF1gwHUEgYHQiX2yywPC9ADNAlgTMrkC?= =?us-ascii?Q?CFV2Yo7xmXpcRzgIoH0rzaicRIwRcit9SQppWPBSG6auoQFu5AsklJLlPqSu?= =?us-ascii?Q?V3DgokeHM83TtG9rilo4PoIjPgdix7pLmrHUw1EB3udwU5+ni2deCZYTVfAc?= =?us-ascii?Q?U0iSN19brRzxuB1Cn3K/P6h4h2LybIltT2w83JEviaUJOw8AJiXIhc7bphdE?= =?us-ascii?Q?TxmguWgdD7KHI7AkoLaL/XLjNz37eJkni7DzJVc3VvFZY4LNud1uaWUAZPLf?= =?us-ascii?Q?mv7tRlXnjJkhFdbz1wYiONDl5njbaq+Zh2aFU7U063pTbg5PRPc8BvornQY4?= =?us-ascii?Q?N8z3AalQHRB43T3/dj6XzJELZW6Gom4KKYa/YFOWq/yV1QdkSPABuocBWf5D?= =?us-ascii?Q?0TAJ3X05+Ro+IR4MCLu8ZYW1G5JAttp6K9r+6+wWzc10ZdF7DLK+V0dLbJzt?= =?us-ascii?Q?YOBBV8CHCAIwsix1/DJW0HPVrcg4lofaSLs3ZfKHNrQwSt/d1l5om2Y61xor?= =?us-ascii?Q?77sxvnu9uLa361oLaYXv1jRBcWbybTyJacfl/0v8bGDerLtfqyUXfWCtl+SB?= =?us-ascii?Q?2JWCrssJ1aJdpsC/Et6DDDhhmsRuE5kdaQYZ1pXHU4VpDjcLJyoanlbZvLhC?= =?us-ascii?Q?dbTKR3l2erb8nTW9Z515g1t41VOmc3/7RuV3zZPuvRIMfBhV8JfQUrA8vb+E?= =?us-ascii?Q?EU047uBZr9vYhvwIEBaHBntXDlfTvUASd2yKwi8udKm8g2PBSqcj0OwGZaQ2?= =?us-ascii?Q?W9yrTdrYIAKKFT0I9DizQIP3pEs/pV8Qr4sJKqDq3mHPecZFaKxTC4z0R4ao?= =?us-ascii?Q?lZHeceus8S0Xx1zMSCHaPEeaHRsjZt4zkPeZBlyPybjwlkbccrvOS2Ag3kIS?= =?us-ascii?Q?xVTC6ntQGK2P4WyXJ+9YA8Q65AgIdX4dnXFdVe4zOmUiO68tkC1bOwX+81BF?= =?us-ascii?Q?2juyTuLZeri53PfZmeKOLZ7LL1cCKuhJ5YTgmQmvAKn+zzgj4/IXodHhO4d5?= =?us-ascii?Q?IObZl8vjtSVc7r/K1yNAPfZ2140ktz7GGenJ6VqrRi5MERJfTKrafNk/E0SN?= =?us-ascii?Q?k0QJVrVYiFucFrG7C3amDog3vi9/PKQdTGwlyzSAEgRTuZPT4mD5aQVw7rFz?= =?us-ascii?Q?0sqxnplZxIdG4g3QJlhlvCpaA3I48FNT/ll9sWM3gUZBW6FtxaZvV2OMZ1Kh?= =?us-ascii?Q?VkHx3zBI4P2MxV0ksgHXtmQ39vkhD2gTJJQObebNF5EEETFl2igOGGTimFnp?= =?us-ascii?Q?Hq9po6lGWgYi0JlRLCYWFDyM8XB9F/d//2ipCmuolKEyKtJovqDS3enxupZB?= =?us-ascii?Q?x8CKgeRAcPVCr+bm/nXEzAWf1WuXIr7zkX2M/KSX?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5058.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 539ec9f9-5caf-4b39-d646-08daf1df7a87 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jan 2023 01:18:56.1601 (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: 6RHV1qRCBvp7T1L7uHwgso0X5teZGvAUqtCIMDyY5FuWO7CAphev14X+ZsBVYxfmpEmPnnMVQCvouSscYrz0eg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4674 Return-Path: min.m.xu@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 comments. It will be updated in the next version. > -----Original Message----- > From: Yao, Jiewen > Sent: Friday, January 6, 2023 4:51 PM > 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 >=20 > Thanks for the cleanup. > Comments inline. >=20 > In general, we need ensure CpuDeadLoop() for *any* parsing error in VE > handler. Just ASSERT or VmCall(HALT) is not enough, because ASSERT =3D=3D > nothing in release, while VmCall(HALT) is not trusted. >=20 > ASSERT can only be used to handle internal impossible logic, but not inpu= t > from VMM. >=20 > Thank you > Yao, Jiewen >=20 > > -----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 > > > > From: Min M Xu > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4169 > > > > The previous TDX MmioExit doesn't handle the Mmio instructions > > correctly in some scenarios. This patch refactors the implementation > > to fix the issues. > > > > 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(-) > > > > 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 > > > > typedef union { > > struct { > > @@ -216,14 +220,15 @@ STATIC > > VOID > > EFIAPI > > TdxDecodeInstruction ( > > - IN UINT8 *Rip > > + IN UINT8 *Rip, > > + IN UINT32 Length > > ) > > { > > UINTN i; > > > > 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])); > > } > > > > DEBUG ((DEBUG_INFO, "\n")); > > @@ -235,50 +240,324 @@ TdxDecodeInstruction ( > > TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \ >=20 > [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 us= e > EFI_STATUS here. >=20 >=20 > > +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; > > +} >=20 > [Jiewen] I checked error state. > I think we need add CpuDeadLoo() after TdVmCall(TDVMCALL_HALT) below: >=20 > if (Status) { > DEBUG (( > DEBUG_ERROR, > "#VE Error (0x%llx) returned from host, ExitReason is %d, ExitQuali= fication > =3D 0x%x.\n", > Status, > ReturnData.VeInfo.ExitReason, > ReturnData.VeInfo.ExitQualification.Val > )); >=20 > TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0); > } >=20 > In general, please CpuDeadLoop() for all TDVMCALL_HALT in CcExitHandleVe(= ) >=20 >=20 > > + > > +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 whic= h > > 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->ImmediateSiz= e; > > + 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); >=20 > [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 > UNSUPPORTED without ASSERT. > It will cause CpuDeadLoop() later anyway. >=20 >=20 > > + return EFI_UNSUPPORTED; > > + } > > + > > + Address =3D InstructionData->Ext.RmData; > > + ReadOrWrite =3D TDX_MMIO_READ; > > + > > + Register =3D CcGetRegisterPointer (Regs, InstructionData- > >Ext.ModRm.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->ImmediateSiz= e; > > + > > + 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.ModRm.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.ModRm.Reg); > > + SetMem (Register, (UINTN)(1 << InstructionData->DataSize), > > + SignByte); > > + > > + 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; > > } > > > > - return NULL; > > + return Status; > > } > > > > /** > > @@ -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; > > > > Status =3D TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData); > > if (Status =3D=3D TDX_EXIT_REASON_SUCCESS) { @@ -335,112 +602,41 @@ > > MmioExit ( > > CpuDeadLoop (); > > } > > > > - // > > - // Default to 32bit transfer > > - // > > - OpSize =3D 4; > > + Status =3D ParseMmioExitInstructions (Regs, &InstructionData, > > &ParsedInstruction); > > + if (Status !=3D EFI_SUCCESS) { > > + return Status; > > + } > > > > - 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 | TdSharedPageMa= sk)) > { > > + 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. >=20 > > + 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. >=20 > > - > > - 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) >=20 >=20 > > - ZeroMem (Reg, RegSize); > > - CopyMem (Reg, (void *)&Val, MmioSize); > > - } > > } > > > > if (Status =3D=3D 0) { > [Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status) >=20 >=20 > > - TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 15); > > - > > // > > // We change instruction length to reflect true size so handler ca= n > > // bump rip > > // > > - Veinfo->ExitInstructionLength =3D (UINT32)((UINT64)Rip - Regs->Ri= p); > > + Veinfo->ExitInstructionLength =3D (UINT32)(CcInstructionLength > > (&InstructionData)); > > + TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo- > > >ExitInstructionLength); > > } > > > > return Status; > > -- > > 2.29.2.windows.2