From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web12.5748.1655126379722325066 for ; Mon, 13 Jun 2022 06:19:40 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=iHxUd4Xb; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: maciej.czajkowski@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1655126379; x=1686662379; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version:content-transfer-encoding; bh=mbRISc894iHMQ4pYPF6Xpf6O1CnJYYd98zAQQ27XiGg=; b=iHxUd4Xbh8MwqarWQGF/I8z3+FUVMq1EQ3yjYbmtkOz7bPi9BhB4BIqs P5eQkgRk9XhVssTW3K0q9/Iucj4Afv26OvG7eUcqGBAtcA6wanjJWJS41 3zSMJMPkke/5KBKGFEmRIOVbiKcBHhkJ3GPSZRXRJIkLyLwXJH0KxIGmQ 8uJpARW6goc6WAd+giVl+v4Jp6kONaqOXV2Zlo5RqA+zL4rKh6X8Nh0jt IS3TF6R0aQyhqe2luxQ4dvrNq1cx2R9kXAP8Nrl2S8s34tdyuRdsX29KY fT/4DyYSc80vZ8Be6r2JZwC79EsHKgR+rCg2H2H+DqdjD0TOCVon8KCch g==; X-IronPort-AV: E=McAfee;i="6400,9594,10376"; a="279000840" X-IronPort-AV: E=Sophos;i="5.91,297,1647327600"; d="scan'208";a="279000840" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jun 2022 06:19:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,297,1647327600"; d="scan'208";a="639733228" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by fmsmga008.fm.intel.com with ESMTP; 13 Jun 2022 06:19:38 -0700 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Mon, 13 Jun 2022 06:19:37 -0700 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Mon, 13 Jun 2022 06:19:37 -0700 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27 via Frontend Transport; Mon, 13 Jun 2022 06:19:37 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.106) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.27; Mon, 13 Jun 2022 06:19:37 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RK34dPhouZFdsYPT5lb6MILznT6JrhYZJqHpNyWOcBp1rxj/JmFbBCFgpDMZi3H953nHGtSNYE7y3e0rbiGV1xuFXmYyk5bZzKVYa61aB1/zq0U4FtRfUHRyBko4hO2p5avSkJ+rC4hJ3SIeP5P7gqEPUuTqfzq898m1B6fEE49IfINFRA3lBjuV3FhSG7ClDcWGF8SWndi9MH58BAdV2l+zgG+0YTY8SSYBCJ5XHnbzhdxoW2JwbPyHoXn3clvIof5pvqlDJdjWzMqdzGxfbQpEJOlv904Di5ZjMbOZslosONCBFQR0r+D7EXtErHhRHs/WUv4vN8/KjXyDCZbizg== 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=ffELTCkNBUdUQ8xcS2Pg2R9nYIAco/1YlDGJ3ffdfp0=; b=Mb8xI1P+BePthBbPvJUeMn2DUE1K9ZvGWmVuxH3QfEjfZTnJePvJ4mCZOl3RKhy0kyGqv8Q2pHnr6KyqHfrOo5pXP9ybepuBsHGRRsIqLbfdPwBxke838wDEAuAV+q+Qivospa6iM5tSZrUXiMNHbK+1rS3s9QFohuTdIEfkkyf1UTMDKJ/+kqsx032YqwJvl7Hlge7el9bxcX3+APK6+wyzUy/WRJJQUXMM79QPB4sgpVik1OcqCgJZRieE7WA1k2hQsgQwq8wNJeCSbed5KFRbOwmRsjErq0Pscm5RRpM4L27qLj1qD5JE7XlbWyuTtHddLGJ7Z4/JoSavhAiS9w== 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 BYAPR11MB3047.namprd11.prod.outlook.com (2603:10b6:a03:8b::32) by BN6PR11MB4162.namprd11.prod.outlook.com (2603:10b6:405:84::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5332.15; Mon, 13 Jun 2022 13:19:34 +0000 Received: from BYAPR11MB3047.namprd11.prod.outlook.com ([fe80::989a:5ac5:3970:6d5e]) by BYAPR11MB3047.namprd11.prod.outlook.com ([fe80::989a:5ac5:3970:6d5e%6]) with mapi id 15.20.5332.021; Mon, 13 Jun 2022 13:19:34 +0000 From: "Maciej Czajkowski" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Ni, Ray" Subject: Re: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Thread-Topic: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Thread-Index: AQHYe6tIidkZVxMVTk++UAWaMYDc6a1GZP2AgAbu0TA= Date: Mon, 13 Jun 2022 13:19:34 +0000 Message-ID: References: <20220606124529.2152-1-maciej.czajkowski@intel.com> <20220606124529.2152-3-maciej.czajkowski@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-office365-filtering-correlation-id: d613ddce-60dc-4b19-fba8-08da4d3f5bd7 x-ms-traffictypediagnostic: BN6PR11MB4162:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: IRKNZxbbL1L3JGkWyVAunjG4EOhxF+YW3wfMhMsKJttpfxx761gayE6oMEESkBLxmfgZ3gqk9GorpR2Jpyi9S9QIZvt9W1hlX+y2WFjQ5pcMCE/r2GLvIKSc5zbbTat/s5gm2IRbe/O3oHeeEvtcx0YZ8yl75cDX7mFhGCHMRwxxteWETdmoAXzWwWOkyzZpJWoghY/GhhPyt4dNgg7OTMCHGrZVE6eC1U6qxkypscKXVDC8TggjMMO+EpfGGwHRDLjF3VQPIgu95Lvq1+qZWOubv8bF8G4W/qAx/9RjaHkH+qxKAp4iDjTwCELep1rsz20Q+nZBHinEMM+qU+ue+0MLnzXIg1DibDv47qG7fLYXAtQW0hW1yJLNsXQyd87mhynF4P6wnmSI0Bx3ebf7/XXnJf2WnIoZ7i98Pn86bdg8rZMLMxqwYTT2364tmKLjq+VTpEKTVe4QkQtrgJtN+W1MLajQjhmVxaf7D45M1F9eKOsQbrK3vrmldQBkolBX0dQ1Qhxjt5jDLITvSkJ7iOlwGYprLAHxeHfhAb5dvDjbZwG7v1l/fyhDSfdDRIpQAT5j0KjQ9D7rXAuEDxLWaRkCTw3zdsZbJdIljkKllsp2iwBsnaor+mdZOV6yZa5RdcYo1f51WmiJpq9d0AmuSeEcKCteyP38YHK1EHt5IQvK97R/wJ6PD/QGq+hJCQG52/ziCnoO/JnNisuNxO472xhmV57X2S2eXmHUa4EbAnRoJ2bGl5aZ4ORAvsF4N2AlD+Zo4NAFySsSKuP/mPmWyYnyDoMZirzvw9XclOXt6Vr7OeNhIpWRtnSGIGJ7zw9BajvbGFVzDll3eYX5eAO6KQ== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR11MB3047.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(366004)(966005)(122000001)(71200400001)(38070700005)(82960400001)(76116006)(508600001)(8676002)(66946007)(2906002)(86362001)(8936002)(110136005)(66446008)(4326008)(316002)(19627235002)(9686003)(107886003)(186003)(26005)(66476007)(66556008)(64756008)(6506007)(53546011)(7696005)(38100700002)(83380400001)(5660300002)(33656002)(55016003)(52536014)(30864003)(579004)(559001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?de/oPY+Eafcl12i5vnIFMHJBr9PeZB5mAhxw+b7VeiC9QjYf6tQQXZ0Curud?= =?us-ascii?Q?kphMU9eE7deH+ql6cs5Bx6mI3zFNjIOZXx3tKWsWZu+zfxefq5U2CQDDQbwH?= =?us-ascii?Q?G9HHHzJQUAKo1QGiqm5xpUy35/ToWnai8nKsb/f+TFv9kA8e64LkJMF3IBwl?= =?us-ascii?Q?JKuyoTaCWTfQYNN+7WUNlQD+ZQQgTMDpu44iaX6dw9Cz78JDFD/dMKT/nZfP?= =?us-ascii?Q?UYfzH2zPO2W6+CevQ+Qboa2UdpWG3rRkXkxfSnY6LPBAu+J6OEGbhVJ7sHD6?= =?us-ascii?Q?qyHJ0YRpMvlya2DuP3tfbm1XrS1gYUQ5f0zFptukOPBw1qe6uZ3kKxVOLRSC?= =?us-ascii?Q?uO1AUMu1Y73xMbpyEL6O7xFcS1rq0Lz/X2SUkUWGASGg59mPnqDiblXAzc5e?= =?us-ascii?Q?WggAJQ/g/CEf+I+KZpgqlac+9Yzk+Ia/ZKgk1Jh0gqCzxCEm+Ut5z0lOCnW/?= =?us-ascii?Q?bu73WamcCFuEZtZAyXVzxH5sDAp4RqlgXIBfEw8/Y+CH0koO6CVa60Ysq8tN?= =?us-ascii?Q?XB5ThLHctoejq+lHrEur45WL/SIPCJHIk6VvJaD7I+/YOenv2Wcv+8ycEUkJ?= =?us-ascii?Q?rBoYpLF6YDG06OCGqAHTIS5VTkiboG3qzA7a9wan5gkoKux2IA4CBAnKd8ue?= =?us-ascii?Q?rt+PHiJZESffGWfelpfGZ4Xs/vkwcWWBk07qJAjA2ZXyVNUzrzKvaqFjQ8GK?= =?us-ascii?Q?6jH9RkSgWtL8CYnosnDO216Ktrtka1WzuHnF8gD5yc2eIQDz4m1Ay7xJI+oh?= =?us-ascii?Q?FvmE9I6AtHnSXI9Z2r1Qc0LRUmz2GhDhtonATw867C51jFeBEfvoXpzH17QB?= =?us-ascii?Q?ZyJsi4iHIeyFjondUO/l9DzJInxeUpjrst87o/qWOL2dwYkh5LJ+sbOF5xJm?= =?us-ascii?Q?1nLR/27Ce+n8k+xtd/apiD3y7HvwkycrbFU6EXqq7bm8cuiBhC1Cbk/3w8B1?= =?us-ascii?Q?ykzzfnG8NqvKEyNwIgZDDC8HHFvc6n/lxnS4kDx0evGJzctJuxylKGo4CcSQ?= =?us-ascii?Q?elj+9GmIHWytHvLJJgz7uKDKERK2DvlB6WUJPF6ucmWZxhFcVYgMLQecLyXq?= =?us-ascii?Q?2eGWz4VVW9xzCWJEqPUlmVaRhrZf0PmMXT1e0rjw6KTgttf0Eu8s8UY8MuEw?= =?us-ascii?Q?2IHpUZWU/kDiSQZjp3CwFo4qoIGeVxpZFCMvRTe/TosrxMDz8JmvaPzY/ETY?= =?us-ascii?Q?n4CL/zKmQQgGYQX4/Y36sklXW0Dj8P2d+bimD6pxjOkbFtvwFqTGJOnhjfhj?= =?us-ascii?Q?vh6AJVyj9AzlF3vzR/APiiitMYJJZJt+84YSQswTtvirZi6nMle1B7LLaTYG?= =?us-ascii?Q?yP3SSh9ZR7ZV6Sb70VZbP/dtvzkwBxXSl0N+5+blthJ1OLmHf0XAtvJsEKKd?= =?us-ascii?Q?RN0spX6/MAhe7Geo1KJ+bvhrkeVEqR3l9Ilnggu5TYPVkXbH/zakRgSju8dF?= =?us-ascii?Q?6eXDnTCHizohNGXwuHbH2kDxyxGXywEPqAuDoA5NSNCwrR0ckXKYIHjslxP/?= =?us-ascii?Q?sCFFGwv3ud3MEI2O1aVuq1Ia2mPfvI9HfH/U86L/DjMKIhTyd3TDQ2MZRZLZ?= =?us-ascii?Q?rgTFqumZZuUOpnnsijVu7pBhFNcEhlwH9fASR+D+fsYuRpgPAQexpo7Ay4Me?= =?us-ascii?Q?DY7HO6u8k/8xlomx/3yOCVr3Ea5rMI/9juO4COC4p9DjktYHVFEragwdzbv3?= =?us-ascii?Q?4n42LcOzN8xxDjSXY4X9C+8hq77F6WuN87uW6w0jgiPpY72pFiZm6XitgpAQ?= =?us-ascii?Q?MX+MqE6djVOgcjOpjcjiT2mGXqIR2LE=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3047.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: d613ddce-60dc-4b19-fba8-08da4d3f5bd7 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jun 2022 13:19:34.5349 (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: 6Xk5fk+fZHt6HyGmSDGjtxPoyWV40C0+rQnpzBse7O1Z4IAZiY0gxhQ0BI9MC2FYXqG3gPwuNO4Kta8WYoveC1twQjjjTj+TQV5jDazFOIA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB4162 Return-Path: maciej.czajkowski@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hello, 1. Yes, I will try to fix that in the v2 patch. 2. We have a review opened to add such instance - https://edk2.groups.io/g/= devel/message/89970 3. For now it will be implemented in the silicon code, so you are right - w= e should keep them. Also, it would require a larger library refactor to con= sume such code from PCI_DEVICE_PPI if we are going to still support both PC= I_DEVICE_PPI and AHCI_HOST_CONTROLLER_PPI. However, what are you thoughts a= bout future of the library? If we can get rid of AHCI_HOST_CONTROLLER_PPI, = I think that it is possible to remove the IOMMU code. 4. It has been run in the simulation environment, and a BlockIo read has be= en performed in PEI phase - and it was performed successfully. 5. Sure, will do that for v2 patch. -----Original Message----- From: Wu, Hao A =20 Sent: czwartek, 9 czerwca 2022 05:08 To: devel@edk2.groups.io; Wu, Hao A ; Czajkowski, Macie= j Cc: Ni, Ray Subject: RE: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use PCI_DEVIC= E_PPI to manage AHCI device For "3) Could you help to check if the DMA memory related codes in MdeModul= ePkg\Bus\Ata\AhciPei\DmaMem.c can be covered by the 'PciIo' service in EDKI= I_PCI_DEVICE_PPI?" After a second thought, my take is that there will be no PciBusPei implemen= tation added in edk2. So there will be no enforcement for producers of EDKII_PCI_DEVICE_PPI to ad= d IOMMU support like in PciBusDxe. If my above understanding is correct, then I think we might still need to k= eep those IOMMU support codes in AhciPei PEIM. Best Regards, Hao Wu > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Wu, Hao=20 > A > Sent: Thursday, June 9, 2022 10:48 AM > To: Czajkowski, Maciej ;=20 > devel@edk2.groups.io > Cc: Ni, Ray > Subject: Re: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use=20 > PCI_DEVICE_PPI to manage AHCI device >=20 > Couple of general level comments/questions: > 1) The implementation of functions=20 > AtaAhciPciDevicePpiInstallationCallback() & > AtaAhciInitPrivateDataFromPciDevice() has many duplications. Is it=20 > possible to abstract a separate function to reduce duplicated codes? > 2) What DevicePathLib instance should be used for the PEI case? As far=20 > as I know, current DevicePathLib instances in edk2 do not support PEIM. > 3) Could you help to check if the DMA memory related codes in=20 > MdeModulePkg\Bus\Ata\AhciPei\DmaMem.c can be covered by the 'PciIo' > service in EDKII_PCI_DEVICE_PPI? > 4) May I know what kind of tests are performed for this patch? Would=20 > like to ensure the origin gEdkiiPeiAtaAhciHostControllerPpiGuid path is n= ot broken. > 5) Could you help to create a GitHub Pull Request to trigger the CI=20 > tests for this series? >=20 > More inline comments below: >=20 >=20 > > -----Original Message----- > > From: Czajkowski, Maciej > > Sent: Monday, June 6, 2022 8:45 PM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A ; Ni, Ray > > Subject: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use=20 > > PCI_DEVICE_PPI to manage AHCI device > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3907 > > > > This change modifies AhciPei library to allow usage both=20 > > EDKII_PCI_DEVICE_PPI and EDKII_PEI_ATA_AHCI_HOST_CONTROLLER_PPI to=20 > > manage ATA HDD working under AHCI mode. > > > > Cc: Hao A Wu > > Cc: Ray Ni > > Signed-off-by: Maciej Czajkowski > > --- > > MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c | 615 +++++++++++++++----- > > MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c | 44 -- > > MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf | 5 +- > > 3 files changed, 458 insertions(+), 206 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c > > b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c > > index 208b7e9a3606..31bb3c0760ab 100644 > > --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c > > +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c > > @@ -9,6 +9,47 @@ > > **/ > > > > > > > > #include "AhciPei.h" > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +/** > > > > + Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation. > > > > + > > > > + @param[in] PeiServices Pointer to PEI Services Table. > > > > + @param[in] NotifyDescriptor Pointer to the descriptor for the Not= ification > > > > + event that caused this function to ex= ecute. > > > > + @param[in] Ppi Pointer to the PPI data associated wi= th this > function. > > > > + > > > > + @retval EFI_SUCCESS The function completes successfully > > > > + > > > > +**/ > > > > +EFI_STATUS > > > > +EFIAPI > > > > +AtaAhciHostControllerPpiInstallationCallback ( > > > > + IN EFI_PEI_SERVICES **PeiServices, > > > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, > > > > + IN VOID *Ppi > > > > + ); > > > > + > > > > +/** > > > > + Callback for EDKII_PCI_DEVICE_PPI installation. > > > > + > > > > + @param[in] PeiServices Pointer to PEI Services Table. > > > > + @param[in] NotifyDescriptor Pointer to the descriptor for the Not= ification > > > > + event that caused this function to ex= ecute. > > > > + @param[in] Ppi Pointer to the PPI data associated wi= th this > function. > > > > + > > > > + @retval EFI_SUCCESS The function completes successfully > > > > + > > > > +**/ > > > > +EFI_STATUS > > > > +EFIAPI > > > > +AtaAhciPciDevicePpiInstallationCallback ( > > > > + IN EFI_PEI_SERVICES **PeiServices, > > > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, > > > > + IN VOID *Ppi > > > > + ); >=20 >=20 > Could you help to put the above 2 function declarations in AhciPei.h=20 > to keep consistency? Sure, will fix that in v2 patch. >=20 >=20 > > > > > > > > EFI_PEI_PPI_DESCRIPTOR mAhciAtaPassThruPpiListTemplate =3D { > > > > (EFI_PEI_PPI_DESCRIPTOR_PPI | > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > > > > @@ -40,6 +81,18 @@ EFI_PEI_NOTIFY_DESCRIPTOR=20 > > mAhciEndOfPeiNotifyListTemplate =3D { > > AhciPeimEndOfPei > > > > }; > > > > > > > > +EFI_PEI_NOTIFY_DESCRIPTOR mAtaAhciHostControllerNotify =3D { > > > > + (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > > > > + &gEdkiiPeiAtaAhciHostControllerPpiGuid, > > > > + AtaAhciHostControllerPpiInstallationCallback > > > > +}; > > > > + > > > > +EFI_PEI_NOTIFY_DESCRIPTOR mPciDevicePpiNotify =3D { > > > > + (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > > > > + &gEdkiiPeiPciDevicePpiGuid, > > > > + AtaAhciPciDevicePpiInstallationCallback > > > > +}; > > > > + > > > > /** > > > > Free the DMA resources allocated by an ATA AHCI controller. > > > > > > > > @@ -111,33 +164,28 @@ AhciPeimEndOfPei ( } > > > > > > > > /** > > > > - Entry point of the PEIM. > > > > + Initialize and install PrivateData PPIs. > > > > > > > > - @param[in] FileHandle Handle of the file being invoked. > > > > - @param[in] PeiServices Describes the list of possible PEI Service= s. > > > > - > > > > - @retval EFI_SUCCESS PPI successfully installed. > > > > + @param[in] Private A pointer to the > > PEI_AHCI_CONTROLLER_PRIVATE_DATA data > > > > + structure. > > > > > > > > + @retval EFI_SUCCESS AHCI controller initialized and PPIs=20 > > + installed > > > > + @retval others Failed to initialize AHCI controller > > > > **/ > > > > EFI_STATUS > > > > -EFIAPI > > > > -AtaAhciPeimEntry ( > > > > - IN EFI_PEI_FILE_HANDLE FileHandle, > > > > - IN CONST EFI_PEI_SERVICES **PeiServices > > > > +AtaAhciInitPrivateData ( > > > > + IN UINTN MmioBase, > > > > + IN EFI_DEVICE_PATH_PROTOCOL *DevicePath, > > > > + IN UINTN DevicePathLength > > > > ) > > > > { > > > > - EFI_STATUS Status; > > > > - EFI_BOOT_MODE BootMode; > > > > - EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; > > > > - UINT8 Controller; > > > > - UINTN MmioBase; > > > > - UINTN DevicePathLength; > > > > - EFI_DEVICE_PATH_PROTOCOL *DevicePath; > > > > - UINT32 PortBitMap; > > > > - PEI_AHCI_CONTROLLER_PRIVATE_DATA *Private; > > > > - UINT8 NumberOfPorts; > > > > + EFI_STATUS Status; > > > > + UINT32 PortBitMap; > > > > + UINT8 NumberOfPorts; > > > > + PEI_AHCI_CONTROLLER_PRIVATE_DATA *Private; > > > > + EFI_BOOT_MODE BootMode; > > > > > > > > - DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__)); > > > > + DEBUG ((DEBUG_INFO, "Initializing private data for ATA\n")); > > > > > > > > // > > > > // Get the current boot mode. > > > > @@ -149,19 +197,149 @@ AtaAhciPeimEntry ( > > } > > > > > > > > // > > > > - // Locate the ATA AHCI host controller PPI. > > > > - // > > > > - Status =3D PeiServicesLocatePpi ( > > > > - &gEdkiiPeiAtaAhciHostControllerPpiGuid, > > > > - 0, > > > > - NULL, > > > > - (VOID **)&AhciHcPpi > > > > - ); > > > > + // Check validity of the device path of the ATA AHCI controller. > > > > + // > > > > + Status =3D AhciIsHcDevicePathValid (DevicePath, DevicePathLength); > > > > + if (EFI_ERROR (Status)) { > > > > + DEBUG (( > > > > + DEBUG_ERROR, > > > > + "%a: The device path is invalid.\n", > > > > + __FUNCTION__ > > > > + )); > > > > + return Status; > > > > + } > > > > + > > > > + // > > > > + // For S3 resume performance consideration, not all ports on an=20 > > + ATA AHCI > > > > + // controller will be enumerated/initialized. The driver consumes=20 > > + the > > > > + // content within S3StorageDeviceInitList LockBox to get the=20 > > + ports that > > > > + // will be enumerated/initialized during S3 resume. > > > > + // > > > > + if (BootMode =3D=3D BOOT_ON_S3_RESUME) { > > > > + NumberOfPorts =3D AhciS3GetEumeratePorts (DevicePath,=20 > > + DevicePathLength, > > &PortBitMap); > > > > + if (NumberOfPorts =3D=3D 0) { > > > > + return EFI_SUCCESS; > > > > + } > > > > + } else { > > > > + PortBitMap =3D MAX_UINT32; > > > > + } > > > > + > > > > + // > > > > + // Memory allocation for controller private data. > > > > + // > > > > + Private =3D AllocateZeroPool (sizeof=20 > > + (PEI_AHCI_CONTROLLER_PRIVATE_DATA)); > > > > + if (Private =3D=3D NULL) { > > > > + DEBUG (( > > > > + DEBUG_ERROR, > > > > + "%a: Fail to allocate private data.\n", > > > > + __FUNCTION__ > > > > + )); > > > > + return EFI_OUT_OF_RESOURCES; > > > > + } > > > > + > > > > + // > > > > + // Initialize controller private data. > > > > + // > > > > + Private->Signature =3D > > AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE; > > > > + Private->MmioBase =3D MmioBase; > > > > + Private->DevicePathLength =3D DevicePathLength; > > > > + Private->DevicePath =3D DevicePath; > > > > + Private->PortBitMap =3D PortBitMap; > > > > + InitializeListHead (&Private->DeviceList); > > > > + > > > > + Status =3D AhciModeInitialization (Private); > > > > if (EFI_ERROR (Status)) { > > > > - DEBUG ((DEBUG_ERROR, "%a: Failed to locate > AtaAhciHostControllerPpi.\n", > > __FUNCTION__)); > > > > - return EFI_UNSUPPORTED; > > > > + return Status; > > > > + } > > > > + > > > > + Private->AtaPassThruMode.Attributes =3D > > EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | > > > > + > > + EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL; > > > > + Private->AtaPassThruMode.IoAlign =3D sizeof (UINTN); > > > > + Private->AtaPassThruPpi.Revision =3D > > EDKII_PEI_ATA_PASS_THRU_PPI_REVISION; > > > > + Private->AtaPassThruPpi.Mode =3D &Private->AtaPassThruMode; > > > > + Private->AtaPassThruPpi.PassThru =3D AhciAtaPassThruPassThru; > > > > + Private->AtaPassThruPpi.GetNextPort =3D AhciAtaPassThruGetNextPort= ; > > > > + Private->AtaPassThruPpi.GetNextDevice =3D=20 > > + AhciAtaPassThruGetNextDevice; > > > > + Private->AtaPassThruPpi.GetDevicePath =3D=20 > > + AhciAtaPassThruGetDevicePath; > > > > + CopyMem ( > > > > + &Private->AtaPassThruPpiList, > > > > + &mAhciAtaPassThruPpiListTemplate, > > > > + sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > + ); > > > > + Private->AtaPassThruPpiList.Ppi =3D &Private->AtaPassThruPpi; > > > > + PeiServicesInstallPpi (&Private->AtaPassThruPpiList); > > > > + > > > > + Private->BlkIoPpi.GetNumberOfBlockDevices =3D=20 > > + AhciBlockIoGetDeviceNo; > > > > + Private->BlkIoPpi.GetBlockDeviceMediaInfo =3D=20 > > + AhciBlockIoGetMediaInfo; > > > > + Private->BlkIoPpi.ReadBlocks =3D AhciBlockIoReadBlocks; > > > > + CopyMem ( > > > > + &Private->BlkIoPpiList, > > > > + &mAhciBlkIoPpiListTemplate, > > > > + sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > + ); > > > > + Private->BlkIoPpiList.Ppi =3D &Private->BlkIoPpi; > > > > + PeiServicesInstallPpi (&Private->BlkIoPpiList); > > > > + > > > > + Private->BlkIo2Ppi.Revision =3D > > EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION; > > > > + Private->BlkIo2Ppi.GetNumberOfBlockDevices =3D=20 > > + AhciBlockIoGetDeviceNo2; > > > > + Private->BlkIo2Ppi.GetBlockDeviceMediaInfo =3D=20 > > + AhciBlockIoGetMediaInfo2; > > > > + Private->BlkIo2Ppi.ReadBlocks =3D AhciBlockIoReadBlocks= 2; > > > > + CopyMem ( > > > > + &Private->BlkIo2PpiList, > > > > + &mAhciBlkIo2PpiListTemplate, > > > > + sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > + ); > > > > + Private->BlkIo2PpiList.Ppi =3D &Private->BlkIo2Ppi; > > > > + PeiServicesInstallPpi (&Private->BlkIo2PpiList); > > > > + > > > > + if (Private->TrustComputingDevices !=3D 0) { > > > > + DEBUG (( > > > > + DEBUG_INFO, > > > > + "%a: Security Security Command PPI will be produced.\n", > > > > + __FUNCTION__ > > > > + )); > > > > + Private->StorageSecurityPpi.Revision =3D > > EDKII_STORAGE_SECURITY_PPI_REVISION; > > > > + Private->StorageSecurityPpi.GetNumberofDevices =3D > > AhciStorageSecurityGetDeviceNo; > > > > + Private->StorageSecurityPpi.GetDevicePath =3D > > AhciStorageSecurityGetDevicePath; > > > > + Private->StorageSecurityPpi.ReceiveData =3D > > AhciStorageSecurityReceiveData; > > > > + Private->StorageSecurityPpi.SendData =3D > AhciStorageSecuritySendData; > > > > + CopyMem ( > > > > + &Private->StorageSecurityPpiList, > > > > + &mAhciStorageSecurityPpiListTemplate, > > > > + sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > + ); > > > > + Private->StorageSecurityPpiList.Ppi =3D=20 > > + &Private->StorageSecurityPpi; > > > > + PeiServicesInstallPpi (&Private->StorageSecurityPpiList); > > > > } > > > > > > > > + CopyMem ( > > > > + &Private->EndOfPeiNotifyList, > > > > + &mAhciEndOfPeiNotifyListTemplate, > > > > + sizeof (EFI_PEI_NOTIFY_DESCRIPTOR) > > > > + ); > > > > + PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList); > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +/** > > > > + Initialize AHCI controller from=20 > > + EDKII_ATA_AHCI_HOST_CONTROLLER_PPI > > instance. > > > > + > > > > + @param[in] AhciHcPpi Pointer to the AHCI Host Controller PPI instan= ce. > > > > + > > > > + @retval EFI_SUCCESS PPI successfully installed. > > > > +**/ > > > > +EFI_STATUS > > > > +AtaAhciInitPrivateDataFromHostControllerPpi ( > > > > + IN EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi > > > > + ) > > > > +{ > > > > + UINT8 Controller; > > > > + UINTN MmioBase; > > > > + UINTN DevicePathLength; > > > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; > > > > + EFI_STATUS Status; > > > > + > > > > Controller =3D 0; > > > > MmioBase =3D 0; > > > > while (TRUE) { > > > > @@ -193,65 +371,7 @@ AtaAhciPeimEntry ( > > return Status; > > > > } > > > > > > > > - // > > > > - // Check validity of the device path of the ATA AHCI controller. > > > > - // > > > > - Status =3D AhciIsHcDevicePathValid (DevicePath, DevicePathLength); > > > > - if (EFI_ERROR (Status)) { > > > > - DEBUG (( > > > > - DEBUG_ERROR, > > > > - "%a: The device path is invalid for Controller %d.\n", > > > > - __FUNCTION__, > > > > - Controller > > > > - )); > > > > - Controller++; > > > > - continue; > > > > - } > > > > - > > > > - // > > > > - // For S3 resume performance consideration, not all ports on an AT= A AHCI > > > > - // controller will be enumerated/initialized. The driver consumes = the > > > > - // content within S3StorageDeviceInitList LockBox to get the ports= that > > > > - // will be enumerated/initialized during S3 resume. > > > > - // > > > > - if (BootMode =3D=3D BOOT_ON_S3_RESUME) { > > > > - NumberOfPorts =3D AhciS3GetEumeratePorts (DevicePath, > DevicePathLength, > > &PortBitMap); > > > > - if (NumberOfPorts =3D=3D 0) { > > > > - // > > > > - // No ports need to be enumerated for this controller. > > > > - // > > > > - Controller++; > > > > - continue; > > > > - } > > > > - } else { > > > > - PortBitMap =3D MAX_UINT32; > > > > - } > > > > - > > > > - // > > > > - // Memory allocation for controller private data. > > > > - // > > > > - Private =3D AllocateZeroPool (sizeof > (PEI_AHCI_CONTROLLER_PRIVATE_DATA)); > > > > - if (Private =3D=3D NULL) { > > > > - DEBUG (( > > > > - DEBUG_ERROR, > > > > - "%a: Fail to allocate private data for Controller %d.\n", > > > > - __FUNCTION__, > > > > - Controller > > > > - )); > > > > - return EFI_OUT_OF_RESOURCES; > > > > - } > > > > - > > > > - // > > > > - // Initialize controller private data. > > > > - // > > > > - Private->Signature =3D > > AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE; > > > > - Private->MmioBase =3D MmioBase; > > > > - Private->DevicePathLength =3D DevicePathLength; > > > > - Private->DevicePath =3D DevicePath; > > > > - Private->PortBitMap =3D PortBitMap; > > > > - InitializeListHead (&Private->DeviceList); > > > > - > > > > - Status =3D AhciModeInitialization (Private); > > > > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath,=20 > > + DevicePathLength); > > > > if (EFI_ERROR (Status)) { > > > > DEBUG (( > > > > DEBUG_ERROR, > > > > @@ -260,86 +380,261 @@ AtaAhciPeimEntry ( > > Controller, > > > > Status > > > > )); > > > > - Controller++; > > > > - continue; > > > > - } > > > > - > > > > - Private->AtaPassThruMode.Attributes =3D > > EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | > > > > - EFI_ATA_PASS_THRU_ATTRIBUTES= _LOGICAL; > > > > - Private->AtaPassThruMode.IoAlign =3D sizeof (UINTN); > > > > - Private->AtaPassThruPpi.Revision =3D > > EDKII_PEI_ATA_PASS_THRU_PPI_REVISION; > > > > - Private->AtaPassThruPpi.Mode =3D &Private->AtaPassThruMod= e; > > > > - Private->AtaPassThruPpi.PassThru =3D AhciAtaPassThruPassThru; > > > > - Private->AtaPassThruPpi.GetNextPort =3D AhciAtaPassThruGetNextPo= rt; > > > > - Private->AtaPassThruPpi.GetNextDevice =3D AhciAtaPassThruGetNextDe= vice; > > > > - Private->AtaPassThruPpi.GetDevicePath =3D AhciAtaPassThruGetDevice= Path; > > > > - CopyMem ( > > > > - &Private->AtaPassThruPpiList, > > > > - &mAhciAtaPassThruPpiListTemplate, > > > > - sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > - ); > > > > - Private->AtaPassThruPpiList.Ppi =3D &Private->AtaPassThruPpi; > > > > - PeiServicesInstallPpi (&Private->AtaPassThruPpiList); > > > > - > > > > - Private->BlkIoPpi.GetNumberOfBlockDevices =3D AhciBlockIoGetDevice= No; > > > > - Private->BlkIoPpi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMediaI= nfo; > > > > - Private->BlkIoPpi.ReadBlocks =3D AhciBlockIoReadBlock= s; > > > > - CopyMem ( > > > > - &Private->BlkIoPpiList, > > > > - &mAhciBlkIoPpiListTemplate, > > > > - sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > - ); > > > > - Private->BlkIoPpiList.Ppi =3D &Private->BlkIoPpi; > > > > - PeiServicesInstallPpi (&Private->BlkIoPpiList); > > > > - > > > > - Private->BlkIo2Ppi.Revision =3D > > EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION; > > > > - Private->BlkIo2Ppi.GetNumberOfBlockDevices =3D AhciBlockIoGetDevic= eNo2; > > > > - Private->BlkIo2Ppi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMedia= Info2; > > > > - Private->BlkIo2Ppi.ReadBlocks =3D AhciBlockIoReadBloc= ks2; > > > > - CopyMem ( > > > > - &Private->BlkIo2PpiList, > > > > - &mAhciBlkIo2PpiListTemplate, > > > > - sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > - ); > > > > - Private->BlkIo2PpiList.Ppi =3D &Private->BlkIo2Ppi; > > > > - PeiServicesInstallPpi (&Private->BlkIo2PpiList); > > > > - > > > > - if (Private->TrustComputingDevices !=3D 0) { > > > > + } else { > > > > DEBUG (( > > > > DEBUG_INFO, > > > > - "%a: Security Security Command PPI will be produced for > Controller %d.\n", > > > > + "%a: Controller %d has been successfully initialized.\n", > > > > __FUNCTION__, > > > > Controller > > > > )); > > > > - Private->StorageSecurityPpi.Revision =3D > > EDKII_STORAGE_SECURITY_PPI_REVISION; > > > > - Private->StorageSecurityPpi.GetNumberofDevices =3D > > AhciStorageSecurityGetDeviceNo; > > > > - Private->StorageSecurityPpi.GetDevicePath =3D > > AhciStorageSecurityGetDevicePath; > > > > - Private->StorageSecurityPpi.ReceiveData =3D > > AhciStorageSecurityReceiveData; > > > > - Private->StorageSecurityPpi.SendData =3D > AhciStorageSecuritySendData; > > > > - CopyMem ( > > > > - &Private->StorageSecurityPpiList, > > > > - &mAhciStorageSecurityPpiListTemplate, > > > > - sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > - ); > > > > - Private->StorageSecurityPpiList.Ppi =3D &Private->StorageSecurit= yPpi; > > > > - PeiServicesInstallPpi (&Private->StorageSecurityPpiList); > > > > } > > > > > > > > - CopyMem ( > > > > - &Private->EndOfPeiNotifyList, > > > > - &mAhciEndOfPeiNotifyListTemplate, > > > > - sizeof (EFI_PEI_NOTIFY_DESCRIPTOR) > > > > - ); > > > > - PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList); > > > > - > > > > - DEBUG (( > > > > - DEBUG_INFO, > > > > - "%a: Controller %d has been successfully initialized.\n", > > > > - __FUNCTION__, > > > > - Controller > > > > - )); > > > > Controller++; > > > > } > > > > > > > > return EFI_SUCCESS; > > > > } > > > > + > > > > +/** > > > > + Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation. > > > > + > > > > + @param[in] PeiServices Pointer to PEI Services Table. > > > > + @param[in] NotifyDescriptor Pointer to the descriptor for the Not= ification > > > > + event that caused this function to ex= ecute. > > > > + @param[in] Ppi Pointer to the PPI data associated wi= th this > function. > > > > + > > > > + @retval EFI_SUCCESS The function completes successfully >=20 >=20 > Please help to add the descriptions for function returning error status. Sure, will fix that in v2 patch. >=20 >=20 > > > > + > > > > +**/ > > > > +EFI_STATUS > > > > +EFIAPI > > > > +AtaAhciHostControllerPpiInstallationCallback ( > > > > + IN EFI_PEI_SERVICES **PeiServices, > > > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, > > > > + IN VOID *Ppi > > > > + ) > > > > +{ > > > > + EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; > > > > + > > > > + if (Ppi =3D=3D NULL) { > > > > + return EFI_INVALID_PARAMETER; > > > > + } > > > > + > > > > + AhciHcPpi =3D (EDKII_ATA_AHCI_HOST_CONTROLLER_PPI*) Ppi; > > > > + > > > > + return AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi); > > > > +} > > > > + > > > > +/** > > > > + Callback for EDKII_PCI_DEVICE_PPI installation. > > > > + > > > > + @param[in] PeiServices Pointer to PEI Services Table. > > > > + @param[in] NotifyDescriptor Pointer to the descriptor for the Not= ification > > > > + event that caused this function to ex= ecute. > > > > + @param[in] Ppi Pointer to the PPI data associated wi= th this > function. > > > > + > > > > + @retval EFI_SUCCESS The function completes successfully >=20 >=20 > Please help to add the descriptions for function returning error status. Sure, will fix that in v2 patch. >=20 >=20 > > > > + > > > > +**/ > > > > +EFI_STATUS > > > > +EFIAPI > > > > +AtaAhciPciDevicePpiInstallationCallback ( > > > > + IN EFI_PEI_SERVICES **PeiServices, > > > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, > > > > + IN VOID *Ppi > > > > + ) > > > > +{ > > > > + EDKII_PCI_DEVICE_PPI *PciDevice; > > > > + PCI_TYPE00 PciData; > > > > + UINTN MmioBase; > > > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; > > > > + UINTN DevicePathLength; > > > > + EFI_STATUS Status; > > > > + > > > > + PciDevice =3D (EDKII_PCI_DEVICE_PPI*) Ppi; > > > > + > > > > + // > > > > + // Now further check the PCI header: Base Class (offset 0x0B) and > > > > + // Sub Class (offset 0x0A). This controller should be an SATA=20 > > + controller > > > > + // > > > > + Status =3D PciDevice->PciIo.Pci.Read ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoWidthUint8, > > > > + PCI_CLASSCODE_OFFSET, > > > > + sizeof (PciData.Hdr.ClassCode), > > > > + PciData.Hdr.ClassCode > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + Status =3D PciDevice->PciIo.Attributes ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoAttributeOperationSet, > > > > + EFI_PCI_DEVICE_ENABLE, > > > > + NULL > > > > + ); >=20 >=20 > Do you think we need to align with AtaAtapiPassThru DXE counterpart=20 > when enabling the controller? > MdeModulePkg\Bus\Ata\AtaAtapiPassThru\AtaAtapiPassThru.c - > AtaAtapiPassThruStart(): > Status =3D PciIo->Attributes ( > PciIo, > EfiPciIoAttributeOperationSupported, > 0, > &EnabledPciAttributes > ); > if (!EFI_ERROR (Status)) { > EnabledPciAttributes &=3D (UINT64)EFI_PCI_DEVICE_ENABLE; > Status =3D PciIo->Attributes ( > PciIo, > EfiPciIoAttributeOperationEnable, > EnabledPciAttributes, > NULL > ); > } Yes, I think it is a good idea. Will be changed in v2 patch. >=20 >=20 > > > > + if (EFI_ERROR (Status)) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + Status =3D PciDevice->PciIo.Pci.Read ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoWidthUint32, > > > > + 0x24, > > > > + sizeof (UINTN), > > > > + &MmioBase > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + return EFI_UNSUPPORTED; > > > > + } > > > > + > > > > + DevicePathLength =3D GetDevicePathSize (PciDevice->DevicePath); > > > > + DevicePath =3D PciDevice->DevicePath; > > > > + > > > > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath,=20 > > + DevicePathLength); > > > > + if (EFI_ERROR (Status)) { > > > > + DEBUG (( > > > > + DEBUG_INFO, > > > > + "%a: Failed to init controller, with Status - %r\n", > > > > + __FUNCTION__, > > > > + Status > > > > + )); > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > >=20 >=20 > Missing function description comments for=20 > AtaAhciInitPrivateDataFromPciDevice. I will fix that in v2 patch. >=20 >=20 > > +EFI_STATUS > > > > +AtaAhciInitPrivateDataFromPciDevice ( > > > > + VOID > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + UINTN ControllerIndex; > > > > + EDKII_PCI_DEVICE_PPI *PciDevice; > > > > + PCI_TYPE00 PciData; > > > > + UINTN MmioBase; > > > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; > > > > + UINTN DevicePathLength; > > > > + > > > > + Status =3D EFI_SUCCESS; > > > > + for (ControllerIndex =3D 0; Status !=3D EFI_NOT_FOUND; > > + ControllerIndex++ ) { > > > > + Status =3D PeiServicesLocatePpi ( > > > > + &gEdkiiPeiPciDevicePpiGuid, > > > > + ControllerIndex, > > > > + NULL, > > > > + (VOID**) &PciDevice > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + continue; > > > > + } > > > > + > > > > + // > > > > + // Now further check the PCI header: Base Class (offset 0x0B)=20 > > + and > > > > + // Sub Class (offset 0x0A). This controller should be an SATA=20 > > + controller > > > > + // > > > > + Status =3D PciDevice->PciIo.Pci.Read ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoWidthUint8, > > > > + PCI_CLASSCODE_OFFSET, > > > > + sizeof (PciData.Hdr.ClassCode), > > > > + PciData.Hdr.ClassCode > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + continue; > > > > + } > > > > + > > > > + if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) { > > > > + continue; > > > > + } > > > > + > > > > + Status =3D PciDevice->PciIo.Attributes ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoAttributeOperationSet, > > > > + EFI_PCI_DEVICE_ENABLE, > > > > + NULL > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + continue; > > > > + } > > > > + > > > > + Status =3D PciDevice->PciIo.Pci.Read ( > > > > + &PciDevice->PciIo, > > > > + EfiPciIoWidthUint32, > > > > + 0x24, > > > > + sizeof (UINTN), > > > > + &MmioBase > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > + continue; > > > > + } > > > > + > > > > + DevicePathLength =3D GetDevicePathSize (PciDevice->DevicePath); > > > > + DevicePath =3D PciDevice->DevicePath; > > > > + > > > > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath,=20 > > + DevicePathLength); > > > > + if (EFI_ERROR (Status)) { > > > > + DEBUG (( > > > > + DEBUG_INFO, > > > > + "%a: Failed to init controller, with Status - %r\n", > > > > + __FUNCTION__, > > > > + Status > > > > + )); > > > > + } > > > > + // > > > > + // Override the status to continue the for loop > > > > + // > > > > + Status =3D EFI_SUCCESS; > > > > + } > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +/** > > > > + Entry point of the PEIM. > > > > + > > > > + @param[in] FileHandle Handle of the file being invoked. > > > > + @param[in] PeiServices Describes the list of possible PEI Service= s. > > > > + > > > > + @retval EFI_SUCCESS PPI successfully installed. > > > > + > > > > +**/ > > > > +EFI_STATUS > > > > +EFIAPI > > > > +AtaAhciPeimEntry ( > > > > + IN EFI_PEI_FILE_HANDLE FileHandle, > > > > + IN CONST EFI_PEI_SERVICES **PeiServices > > > > + ) > > > > +{ > > > > + EFI_STATUS Status; > > > > + EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; > > > > + > > > > + DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__)); > > > > + > > > > + PeiServicesNotifyPpi (&mAtaAhciHostControllerNotify); > > > > + > > > > + PeiServicesNotifyPpi (&mPciDevicePpiNotify); > > > > + > > > > + Status =3D AtaAhciInitPrivateDataFromPciDevice (); > > > > + > > > > + // > > > > + // Locate the ATA AHCI host controller PPI. > > > > + // > > > > + Status =3D PeiServicesLocatePpi ( > > > > + &gEdkiiPeiAtaAhciHostControllerPpiGuid, > > > > + 0, > > > > + NULL, > > > > + (VOID **)&AhciHcPpi > > > > + ); > > > > + if (!EFI_ERROR (Status)) { > > > > + DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi to=20 > > + initialize > > private data.\n", __FUNCTION__)); >=20 >=20 > Suggest to use DEBUG_INFO here. Ok, will fix that in v2 patch. >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > + Status =3D AtaAhciInitPrivateDataFromHostControllerPpi=20 > > + (AhciHcPpi); > > > > + } > > > > + > > > > + return Status; > > > > +} > > > > diff --git a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c > > b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c > > index 81f8743d40d8..cf0955929dde 100644 > > --- a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c > > +++ b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c > > @@ -38,50 +38,6 @@ EFI_DEVICE_PATH_PROTOCOL=20 > > mAhciEndDevicePathNodeTemplate =3D { > > } > > > > }; > > > > > > > > -/** > > > > - Returns the 16-bit Length field of a device path node. > > > > - > > > > - Returns the 16-bit Length field of the device path node specified by= Node. > > > > - Node is not required to be aligned on a 16-bit boundary, so it is=20 > > recommended > > > > - that a function such as ReadUnaligned16() be used to extract the=20 > > contents of > > > > - the Length field. > > > > - > > > > - If Node is NULL, then ASSERT(). > > > > - > > > > - @param Node A pointer to a device path node data structure. > > > > - > > > > - @return The 16-bit Length field of the device path node specified by= Node. > > > > - > > > > -**/ > > > > -UINTN > > > > -DevicePathNodeLength ( > > > > - IN CONST VOID *Node > > > > - ) > > > > -{ > > > > - ASSERT (Node !=3D NULL); > > > > - return ReadUnaligned16 ((UINT16 *)&((EFI_DEVICE_PATH_PROTOCOL=20 > > *)(Node))->Length[0]); > > > > -} > > > > - > > > > -/** > > > > - Returns a pointer to the next node in a device path. > > > > - > > > > - If Node is NULL, then ASSERT(). > > > > - > > > > - @param Node A pointer to a device path node data structure. > > > > - > > > > - @return a pointer to the device path node that follows the device=20 > > path node > > > > - specified by Node. > > > > - > > > > -**/ > > > > -EFI_DEVICE_PATH_PROTOCOL * > > > > -NextDevicePathNode ( > > > > - IN CONST VOID *Node > > > > - ) > > > > -{ > > > > - ASSERT (Node !=3D NULL); > > > > - return (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)(Node) +=20 > > DevicePathNodeLength (Node)); > > > > -} > > > > - > > > > /** > > > > Get the size of the current device path instance. > > > > > > > > diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf > > b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf > > index 912ff7a8ba4f..788660b33299 100644 > > --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf > > +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf > > @@ -50,11 +50,13 @@ [LibraryClasses] > > TimerLib > > > > LockBoxLib > > > > PeimEntryPoint > > > > + DevicePathLib > > > > > > > > [Ppis] > > > > gEdkiiPeiAtaAhciHostControllerPpiGuid ## CONSUMES > > > > gEdkiiIoMmuPpiGuid ## CONSUMES > > > > gEfiEndOfPeiSignalPpiGuid ## CONSUMES > > > > + gEdkiiPeiPciDevicePpiGuid ## CONSUMES > > > > gEdkiiPeiAtaPassThruPpiGuid ## SOMETIMES_PRODUCES > > > > gEfiPeiVirtualBlockIoPpiGuid ## SOMETIMES_PRODUCES > > > > gEfiPeiVirtualBlockIo2PpiGuid ## SOMETIMES_PRODUCES > > > > @@ -65,8 +67,7 @@ [Guids] > > > > > > [Depex] > > > > gEfiPeiMemoryDiscoveredPpiGuid AND > > > > - gEfiPeiMasterBootModePpiGuid AND > > > > - gEdkiiPeiAtaAhciHostControllerPpiGuid > > > > + gEfiPeiMasterBootModePpiGuid > > > > > > > > [UserExtensions.TianoCore."ExtraFiles"] > > > > AhciPeiExtra.uni > > > > -- > > 2.27.0.windows.1 >=20 >=20 >=20 >=20 >=20 --------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydz= ial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-31= 6 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata= i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wi= adomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiek= olwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). If you are not the intended recipient= , please contact the sender and delete all copies; any review or distributi= on by others is strictly prohibited.