From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web10.8947.1681203254285369822 for ; Tue, 11 Apr 2023 01:54:14 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=ifnCmY29; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: ray.ni@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1681203254; x=1712739254; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=eb3RIZFFknsTiXN/IrOeSs96J3mMvebrmG/N2dga5Og=; b=ifnCmY29tPpTM40QMYUN2+9gBNj+VyP8fo4wY3GxSTmSG7OL7p/0Wh5c rE0wU8QtYwXsllZE9jaNm2W3pTq1IoHn+2fW9LS0AyGWIB05is4L/i1Cn knvaZ4jlvMnR9MzYXFFCvFd0opD+0sqtSbrNN7quUwKw5tA9KP9tiRs1s ownPw0q65ZPf/Cn18F7oYCDjGxHV4v4M7YlTRSJU5rFc0YM1zRBW5r/Gd KHhscYB/nWlpbV65hLUmjzotPMRZWeIADZ9/zuqR59Yym46DSEorxfCo/ 5Mpe+eesu151uoYCWt0O9larAw1DL7eAQ6Ax/2vyIv0OzWRBpTIhSMiZr w==; X-IronPort-AV: E=McAfee;i="6600,9927,10676"; a="341050508" X-IronPort-AV: E=Sophos;i="5.98,336,1673942400"; d="scan'208";a="341050508" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Apr 2023 01:54:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10676"; a="638737207" X-IronPort-AV: E=Sophos;i="5.98,336,1673942400"; d="scan'208";a="638737207" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga003.jf.intel.com with ESMTP; 11 Apr 2023 01:54:13 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 11 Apr 2023 01:54:13 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) 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.23; Tue, 11 Apr 2023 01:54:12 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Tue, 11 Apr 2023 01:54:12 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.48) 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.23; Tue, 11 Apr 2023 01:54:12 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kG5einJL4J2W4Zt0E7WYYU64q/3YtUlmevzDL6kMlfmi9bRrRYbBGEJlKXszOLrAfgfdoP95njXLq7hk3wPq9dxD4+kynCGB36uSSPZMZNtAlwAFSFEtDtvbQJWMI64VKEpHGVpWdTCNaBN/9vhwRZT+wyfAH72GvPSxhfAvzjy19/ohPRpc7J/LqRu+Y5gi6mZHLSaGdOOhNkUAu7bwi6No2yuypizr0ov8nWan5vA9/i5wgbUNSjAaRWnUAnZW5GFMGZpTC4HUPm2MnsOV098uBwuXBeB5VkT/wVJNMYuE8jgMirNaORbxIp5J6vjDkWle9yEcNZFgjxOg927ZTQ== 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=wGrs3le9qiPshS7hyVHGKeve6c9XPkyzG3Wyph3FEyI=; b=mEW1DG/35FOlKmg3xnvPr4TSQmjx9CqWVBj0ihDVTSppXS0M8jlXDROI5CprSQ5F4R8+OH9atJ3CpCwdiWOenlBWieLByou2CYz7BP4wO+XrUzYG1zJz9RNG+kGbpTm3lSkOWWfnSanON8llsZfqiaKW/uziVhOanJgixusR8GG7tcFpxH/MdMGmacK3JO+j1fSLfUt7m61jzJNPzva2KDpvqpOqIMeqq8MJ4Wo4grXGTlZFAU9G3Vq6XcZIkmubwmG0I34f2tnaHqpHOjHnBy//FwQqTP+BVFNCyg5ooySe8sAJU++WJqlrzqatGPLtri9S4PgAfEFXzigAbQv35w== 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 MN6PR11MB8244.namprd11.prod.outlook.com (2603:10b6:208:470::14) by MN2PR11MB4647.namprd11.prod.outlook.com (2603:10b6:208:262::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6277.33; Tue, 11 Apr 2023 08:54:10 +0000 Received: from MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::ae07:e96a:4a24:8a69]) by MN6PR11MB8244.namprd11.prod.outlook.com ([fe80::ae07:e96a:4a24:8a69%7]) with mapi id 15.20.6277.034; Tue, 11 Apr 2023 08:54:10 +0000 From: "Ni, Ray" To: "devel@edk2.groups.io" , "abdattar@amd.com" CC: Paul Grimes , Garrett Kirkendall , Abner Chang , "Dong, Eric" , "Kumar, Rahul R" , "Gerd Hoffmann" Subject: Re: [edk2-devel] [PATCH v8 7/9] UefiCpuPkg: Implements SmmSmramSaveStateLib for Intel Thread-Topic: [edk2-devel] [PATCH v8 7/9] UefiCpuPkg: Implements SmmSmramSaveStateLib for Intel Thread-Index: AQHZa50pit+a9t1NiUiTc0WIZJLxVq8lyoAA Date: Tue, 11 Apr 2023 08:54:10 +0000 Message-ID: References: <67da7e065711b9ad299c21031ba3b3eae8bb9231.1681121324.git.abdattar@amd.com> In-Reply-To: <67da7e065711b9ad299c21031ba3b3eae8bb9231.1681121324.git.abdattar@amd.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: MN6PR11MB8244:EE_|MN2PR11MB4647:EE_ x-ms-office365-filtering-correlation-id: 97269113-9738-493f-bf9f-08db3a6a514e x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: RbepNuhw1u/tvl7IptnIq8fh4sfpj6a0u5u28RbGXy1cA3EZPXFCXb9CdtYngIMpdAsyDvCBFgojKMWw82PZkA1+k82uHG+cCsttMujqGeMwUHt+wV4h1bEzgx60O71gm45pD3Fb2HAxyz3vT/4S7b/B1E7lTsjct3yWPrLYZZn2tafbiZRYUHOjrrvuHhf4FeMZ15xOy99zw1oefT1ARU5SxYGLxJiFoi+1c8cRqTv38CYSrzqer0UILiE4neWqkR13G7pdQI+G/X8RkApT4Z0alYkdrnFwQxtnmqcmqB8svIG0gpG6Kz2pG6pD+/zXDWoNjF6rus6NxI0lKNqFHkNC0U6imfgGx5kO+eU60sbGUAllUCeU+8bigKrd04y+KHqV3MsPTylWjildDu1jVvFLMiEbnZjHycPYADR3iUBpq5rX/zrucWEJ1UyMEbT/mS3pFpCb5/YYuvy70Yg7Noew0pqVu6Rd8dkCd4oGSOQ76PrIvKiUABr/QchYStrmnpJkQJ3a9JmgR9gNRT+TQhLeQU+CPeb7f9acnbha9fDyU9S4BZCJ8rOhc6PKuUZdgB1a0akmahMFUevY7uT2ZTLTr9vI95G6VvJaRU5DLQtSDFFolQbyP5FxbEFwZKGWxPBizVtxFDdqp5/01UudoA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MN6PR11MB8244.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(396003)(39860400002)(376002)(136003)(366004)(346002)(451199021)(33656002)(86362001)(66946007)(110136005)(76116006)(71200400001)(41300700001)(7696005)(316002)(478600001)(66556008)(4326008)(66476007)(54906003)(64756008)(66446008)(55016003)(52536014)(8936002)(8676002)(2906002)(38070700005)(5660300002)(38100700002)(186003)(83380400001)(82960400001)(122000001)(26005)(6506007)(9686003)(213903007);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?CqPAwtT6vQuFjduBrBHx86S/WRlLdfC845E4cV+H2YZgt9mvhnEgoGzReKF8?= =?us-ascii?Q?nFS/fq9KAA+ME2idHUhWT3FPj7xjLGteaCryxoDGd31TVJI9FjTZh8mlC5/S?= =?us-ascii?Q?eFJiH71pJyyKc8w0kFhTwHRwen2CROF5tlNbdmc2bXNustCHC6FkC8EyiEfb?= =?us-ascii?Q?69OiBhCr7tXTpZstHdxXMR4y/bd8IdtBhj2MWyNpPrtvGbeZvpmYB3u+gbfL?= =?us-ascii?Q?+cBIhyy43SoNqHXxg94j5NwGRMEhBdgmh8PrjQDlL7QQQcAxG5otPappV9xh?= =?us-ascii?Q?IGpVpCMw9QkSVPhfLxrA4PNnI6VuoqH6TcfeQPdUcPAwrL/GNuWvfEiZYdy7?= =?us-ascii?Q?lrzeVSM1GfPOvpct6zjRmZqVzUencfKoE7rzqq2/FE8ZI2WTuv8bq6810xFo?= =?us-ascii?Q?/sYA67/TbsWBvA0DTiEXHpP+SMdQEDrtC1/zQjQgjliNJrBMiOA8FFBmZyox?= =?us-ascii?Q?DxVJdaaIPOm/b566A7blHxLAbpMomYc7qKWq/0AqmwNvo/2MUAAbeDwk36aN?= =?us-ascii?Q?TtgF9Gsuf57LtW/HJSRf0ceZDfgChcaNsDxDof402YBAesItwMka5X+9PrGA?= =?us-ascii?Q?19Lywl2wEH8hYvB9xBH/aECgnyD/QwE9TJgRjE/rbVq+Cn9vX8YCffdbqBtL?= =?us-ascii?Q?ISJ1PxJKRNORBxkTdh9stRj6iHUL7+GBV+i0Y+NVSc/Qn69eWzC+NZNST8yR?= =?us-ascii?Q?1NsAIcNG1uek02CD/zDwaj1Y0lAaxIDXQ7IdSoxvqi4Mu6eSkeX3fFMQ5u+F?= =?us-ascii?Q?+hy0lmyfHYdYnEVRSMpKgbPykaB7B7Gl1orTsgSJthqsF9yYE8FuDzZ3auKP?= =?us-ascii?Q?14rAy46cNrzhUUZGAs+vLTXGtR3O0fl80CpW82VGSkCF3XFrs5TemMTeSigq?= =?us-ascii?Q?0KLl9jhn+dCy+hJv14z4gplfOgJwV8Ey/XPmep/8PvOIv0ahj5VUZ4UQuNbT?= =?us-ascii?Q?1Xu+PvajZYy6pfY2B570KGG3NymMcYC7Kpur1W2oAfm1XQDNCXj6eUB/AQYO?= =?us-ascii?Q?s680Kb/Lvqn5ut6sBbGlIxTu5Vfv6kaTy0emhcdMtZsipN4LUk+NEP2BNLhD?= =?us-ascii?Q?/6iWoI+WzhH6HF2dQpmEDrm+lEGCCL04DPGw4oRQnVoLDTmF6KqOUnQt670Z?= =?us-ascii?Q?jvd3SIbWGF6rq7kyfHmxuAilLnNgIBoxk6CZm6jIAIlzFBNIiEmQBbGDGZlC?= =?us-ascii?Q?JLeK6+/Y8dPqUxrJvT+YOt48Xe/Ljun1sGnNxpaiN1vbyMI1j4LzNGphpl4z?= =?us-ascii?Q?befgYR9zIFN3aoU8vFLBT46UZR99Zn+Oysph7NFXMzI+9pQLdS2Y3uB6ufUA?= =?us-ascii?Q?oldPTEHGWgmwFL4SZD/rDvpbQxW+jRV8EQTOaX5MUuWUbNMyN0kJP9XgZf9+?= =?us-ascii?Q?UhwXTA30YOxgDvLtdxXzz+A41Uwi9k1EdJwT62+d/2oJS/aYMqTVZqGE43lw?= =?us-ascii?Q?rGzAHYOrWErsYRnb4jk5bSdMgkCF3cL34vk4xzeJMd2408Q9v+hSP5pEnTff?= =?us-ascii?Q?hFwJFF+DbjIdrp5vKgZS/zVh+ksna3hqiNvS8k42jtQOd/2siliBtnCyMQJl?= =?us-ascii?Q?XumQZQMQksFwTzyIj/I=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8244.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 97269113-9738-493f-bf9f-08db3a6a514e X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Apr 2023 08:54:10.7765 (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: uC0j74GlRrHzLehy12y7rHNsF55GNF5W+UCx5Y+xYMkUDEeT4XLjO4fbzKtB3cYaZPDbvpn4GkuizJasZQYkbQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4647 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > + > SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel > SmmSmramSaveStateLib.inf 1. Can you rename it to "IntelMmSaveStateLib"? > + INF_VERSION =3D 1.29 > + BASE_NAME =3D IntelSmmSmramSaveStateLib > + FILE_GUID =3D 37E8137B-9F74-4250-8951-7A970A3C39C= 0 > + MODULE_TYPE =3D DXE_SMM_DRIVER > + VERSION_STRING =3D 1.0 > + LIBRARY_CLASS =3D SmmSmramSaveStateLib 2. Can you check back the comments I left for AMD instance and apply simila= r changes here? > **/ > EFI_STATUS > -EFIAPI 3. Why remove "EFIAPI"? > +/** > + Read an SMM Save State register on the target processor. If this func= tion > + returns EFI_UNSUPPORTED, then the caller is responsible for reading th= e > + SMM Save Sate register. > + > + @param[in] CpuIndex The index of the CPU to read the SMM Save State. > The > + value must be between 0 and the NumberOfCpus fie= ld in > + the System Management System Table (SMST). > + @param[in] Register The SMM Save State register to read. > + @param[in] Width The number of bytes to read from the CPU save > state. > + @param[out] Buffer Upon return, this holds the CPU register value r= ead > + from the save state. > + > + @retval EFI_SUCCESS The register was read from Save State. > + @retval EFI_INVALID_PARAMTER Buffer is NULL. > + @retval EFI_UNSUPPORTED This function does not support reading > Register. > + @retval EFI_NOT_FOUND If desired Register not found. > +**/ > +EFI_STATUS > +EFIAPI > +SmramSaveStateReadRegister ( > + IN UINTN CpuIndex, > + IN EFI_SMM_SAVE_STATE_REGISTER Register, > + IN UINTN Width, > + OUT VOID *Buffer > + ) > +{ > + UINT32 SmmRevId; > + SMRAM_SAVE_STATE_IOMISC IoMisc; > + EFI_SMM_SAVE_STATE_IO_INFO *IoInfo; > + > + // > + // Check for special EFI_SMM_SAVE_STATE_REGISTER_LMA > + // > + if (Register =3D=3D EFI_SMM_SAVE_STATE_REGISTER_LMA) { > + // > + // Only byte access is supported for this register > + // > + if (Width !=3D 1) { > + return EFI_INVALID_PARAMETER; > + } > + > + *(UINT8 *)Buffer =3D SmramSaveStateGetRegisterLma (); 4. It's Intel specific flow. I am curious how AMD flow handles the LMA read= . > + > + return EFI_SUCCESS; > + } > + > + // > + // Check for special EFI_SMM_SAVE_STATE_REGISTER_IO > + // > + if (Register =3D=3D EFI_SMM_SAVE_STATE_REGISTER_IO) { 5. Similar question here: how AMD flow handles the IO read? > + > + // > + // Convert Register to a register lookup table index > + // > + return SmramSaveStateReadRegisterByIndex (CpuIndex, > SmramSaveStateGetRegisterIndex (Register), Width, Buffer); 6. Can you double check here? The mSmmSmramCpuWidthOffset[] of Intel/AMD ve= rsion don't put the GdtBase in the same location. SMM_SAVE_STATE_REGISTER_MAX_INDEX is 2 for AMD but should be 4 for Intel. How about define a "CONST UINTN gSmmSaveStateRegisterMaxIndex" with differe= nt values in Intel/AmdSmramSaveState.c? > EFI_STATUS > -EFIAPI 7. Why remove "EFIAPI"? > +UINT8 > +IntelSmramSaveStateGetRegisterLma ( 8. Can you implement the SmramSaveStateGetRegisterLma() in Intel/Amd C file= s?