From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web11.6253.1654744103644410096 for ; Wed, 08 Jun 2022 20:08:23 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=HorzEP+A; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: hao.a.wu@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1654744103; x=1686280103; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=rOcYruotHkKmblLOWyvzthNKlUAl9Z5oerATJ7+ooHc=; b=HorzEP+A1qwzcS6OOqapruqezKup5nYQTvTyQmHIOK0dKpjELqhp/dn+ I2fnlD08TlkfZ0YDb/+rjn4VSUDqK+ajmY1679yZnSCQV0qWHtkqx3pg0 5f8EQyaFmyvybTp2Ji9v/IREqEy5QuzfSGR0SDwzk8EhAiToNymk9X52j pRJIMLeY1Q2e/z6zGtPc4mGtAx4wtuPYqEUXYP0+49alkYsMeDcxfnRCz kKwlTne4MSXSl38XPYrJLEWEerx/RxycPukpJquTfMGOXFxMTKL2xbZko 10SqhjTP4YrhXhl1H0ZUjP1z+ZIpOPS4kX7K2nYgPBzyvIXugRVIGSGVl w==; X-IronPort-AV: E=McAfee;i="6400,9594,10372"; a="260262903" X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="260262903" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2022 20:08:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="566191988" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by orsmga002.jf.intel.com with ESMTP; 08 Jun 2022 20:08:22 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Wed, 8 Jun 2022 20:08:21 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27 via Frontend Transport; Wed, 8 Jun 2022 20:08:21 -0700 Received: from NAM04-BN8-obe.outbound.protection.outlook.com (104.47.74.43) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.27; Wed, 8 Jun 2022 20:08:21 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TzSva0mfUU5HMEE2ynJiKvwkDpO5zvLPFtVWIPfQH1yAPJI9dvS3HEpVT+pnsdX/grtaJfI64NAHLZybLwNZp9neXx0k6HRyP7yN3KUn6/yie0UiVvr+imSSKMBkb4HNAEaNhw3oqCgoqy5EdsbS28RcrhthsZz5ZGePHTCsCquXpXdnQhFWmF2q3NNCbLh6dLPpOd3/ed6ZzIB+edkL8qRyq4IQXkE2xQfH3oL3GRcYar7mkxPlrUmbpacFEBU+Y0cr8loYG9AwvISWF1nAiV4QUtEZXJARCaT03SMoySSsxgYdiTqMQE/aP3C0ecckofVBhaSfxOonoNM1FWIt6Q== 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=EJHPWhuinJpnA+RnY4MbIsHsR0GvrKR5yIrcqFd0660=; b=kn/ohue3PDFpVAlTxk9QiBKkmzy1dKjNOet7M5LMdVGzipWUbZPiT+TixAfjx/54Ozf5GgtRrzm5BfZy/vAzjoz3kWj8xwg7jiii01EK1RePMgzmgIhEpl6uRhKbPJOvrooduPNATbCytDqBHXwfVRqgsXgBhcB9xNy9tPkB+Nc14s2IK4z/edbkyOcDuzM7rN6IuTeIwIrMLcNlEago5seNXQufPukj0QgmuXydQkxjzmGbVaG5vwDHriUSNSc2lXbTuxZYRG4mrfKn3dr3Aoqo6jyNY8GtHDlH9bc2vb24ZOjB1pZNOXUpRFShCBctEE9g7iqqsb/34yaxVi2/KQ== 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 DM6PR11MB4025.namprd11.prod.outlook.com (2603:10b6:5:197::31) by BN6PR1101MB2257.namprd11.prod.outlook.com (2603:10b6:405:5a::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5314.13; Thu, 9 Jun 2022 03:08:18 +0000 Received: from DM6PR11MB4025.namprd11.prod.outlook.com ([fe80::c473:f30f:6b1f:c5ec]) by DM6PR11MB4025.namprd11.prod.outlook.com ([fe80::c473:f30f:6b1f:c5ec%5]) with mapi id 15.20.5314.019; Thu, 9 Jun 2022 03:08:18 +0000 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Wu, Hao A" , "Czajkowski, Maciej" 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: AQHYeaNbITTSYmoSMkeMRh0OHsejLq1GQYHggAAlyXA= Date: Thu, 9 Jun 2022 03:08:17 +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, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 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: 2e98265d-6c77-4bae-c71e-08da49c54d40 x-ms-traffictypediagnostic: BN6PR1101MB2257: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: q/PjWzsnQYGu5/u7QV11Htx3JG8ls6Kom30t6Cpu4YNXqAzvoW+aBouVVX+aP7IIzy3FVfBPWFVuFUQQuo05Qpwl3M1ZD7cdrTWnCnTxwaa5FL51AUi8BRJ+wu/1RclT1jBaG2gLLSYDkgsEYF654kLBnKU/WwuatkComdwG0ArbzGACadpouV8P3PqUMAn4Z8V/2sENxa8MT6+r2YPxToySQkaEpMiQfAxakAJNWCi0R6N5SLoKhH+auM2PyWUNS0VeorCZ6jfCXTqU3KvC41nV1KnjjA3BVaUrEalv/M4Bb/0fo2dqaP3Hm+jRA0LjfhvGW4KFSi2on1sbrNn1DDLrxprIQhOFCiM+ucG3sBuBS6HOHC255mIf1zpVGS4ZEdo93IjkVdPezNMWQOhWuX2uNoJ8MV9Yx6VNU9VVHmux9wxFhlo82eJaeHu7RJR+C4nXlq0u4oylQdYYLSIi3+yQN5fg9ivSF3KXq9oMggWnEQ0yQhONlaQSCvP24xddbzRJKMIuyWHKhFZ+l7PGrAnKlwlZ/Lok9d+ue/RVqlheaMoXN2LhuhlLPeodFxQ0Btx7sU1OoVWdGvH3SByOuLZ/FKfU110SxTRhUdzj66NxU69JeEroeBflkRTZZklFdJ78LilTyS0YKaOcoBbc9fOAjTKrR6iyOvEfVqu2k0LV7/KULTVZ0A+jI2zGL/sHrD8h7W9Wxn6lNuZV78n4Uw1uW1ucbi1HhemDaIq6P9tz35+QC33knx9EQZTjw7hOSXcCFsyK765YTUeGay3H6iBS3wOLQIq0AwTQX/0bdUQ4xGL66V0M858gdKS6UXA/90+79S+dwtmqsamtkeo9Fw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM6PR11MB4025.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230001)(366004)(55016003)(82960400001)(52536014)(966005)(2906002)(508600001)(8936002)(30864003)(71200400001)(33656002)(26005)(5660300002)(2940100002)(9686003)(53546011)(186003)(6506007)(7696005)(122000001)(86362001)(83380400001)(38070700005)(107886003)(4326008)(38100700002)(110136005)(6636002)(66556008)(66476007)(66446008)(64756008)(66946007)(76116006)(19627235002)(316002)(8676002)(559001)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?5FoVutipvc1daXIEdIILrGo3BTelwGdnfstTMOQ501LOUr95kx8vIsCisul+?= =?us-ascii?Q?N7FkIBLtzSFPEwA68/lPMPCcHQWQgvPKigufJGHql9rHLP8LIEhlwJj9if6d?= =?us-ascii?Q?BRsFq+4kDfnEtKCrzMZNJE94u0ja2fTJzCJKND3XJUu6PKis0wsKv5Vok+TF?= =?us-ascii?Q?BDpcWruUo6I2/UsL6zpn6odFyIKAtUbrO28+K0bDi2xigA/Pd/VO//PZaCl/?= =?us-ascii?Q?Y6r8Xd1P0bLC82H+1Y4rqXTqdYnI8owL+T0uZMxMJCAWHC8l5ji9jfIz+psG?= =?us-ascii?Q?e8B1gBMota96NqfgDifr6xZmJ6dxtrlkfFY5d7/jrzST36+YCd8fZFfynfwn?= =?us-ascii?Q?GmpIulHnabSQpp1l/znAd+QlBd13ljHL5ZN3lb8klNtxOfS+Uw8vQBi2dFS/?= =?us-ascii?Q?LcdCnFLrldta8LJimXStNqU8W71qrLdVw3x9tDtcVhBqlNPhgs+4qdQOG+Ns?= =?us-ascii?Q?nazq8b+xaLNhY+oUwjbMh3O2VH7DgwyCJ32IqEzxTxDRaXUvO2RStN2zZf4N?= =?us-ascii?Q?5PsI0kn9fLe7b0iNuOaFj1M2V/zK6KsU4YmWo0d++xAP0v9ExvoHhTjFrKqn?= =?us-ascii?Q?XizcpXPz8Oe4HrEaB3qm/m422czH6Rfy9xFSL4+7ZsQLfQrXR+QAUFQQxROm?= =?us-ascii?Q?j+AXVkDElqlywHug3qbaf7g0n7WZda3r2XRjkd7EqmxOZAQsstun+qqPfF0+?= =?us-ascii?Q?avozZfHSNTN9+boAMM5Z1Wv3rtV1l5txI2EiV9XVztfxAwjbEgJ6seDC0l5h?= =?us-ascii?Q?BHDsMe+441H5OS4WqkLzIHBc3CN0WmzuvwZhIo+qEMJKRqiW7UaCH01Mu0q7?= =?us-ascii?Q?AIb3SvGoQMc6vmoxIdBCnWyMYjkXGnTsaVi6ZbDl1ocqD4O+Ua6urHow+hfg?= =?us-ascii?Q?duJzJpti9Mo13RbPNfqaMYnfue9iCYdANKR4v+pH3sIzZ2eUAx7NqyHPsmYk?= =?us-ascii?Q?u60Jk8spJ92vrK2bdfg5+AszG4nkIcfFuod9YR182xYID7xvmx7aTExg3tTM?= =?us-ascii?Q?40zRY2q6F2NoB8ZUzQyEDAmRWqxp1Z9nNaVZsVcZpSKOxGC2m3n4LETTf6Y7?= =?us-ascii?Q?MfK3EyjWzYTuOmUTZNhz3FKkvMy2QRG7wDMMLR87VNmUPFo+HRAyMprhVLvX?= =?us-ascii?Q?yYq9vzUNvz2+vff1Sm/erYVzg3TxtCRDK3r/ptBPVQyIZCgGHDmD0hj9En2w?= =?us-ascii?Q?TiitnkJ5SPRoeKeNOCCLj7vcmvQBAHOLBjikZWPEs8V0J/Z2ZnNJGznvEI9l?= =?us-ascii?Q?pXakbSaCE2aVcQK9hrdsICdgoIquVsM/7ekGhpXZfuVHOaHKBvDSbRCTcUsO?= =?us-ascii?Q?ZKepDZMuUXR4xfjj/omg8hE6Qv6CE8nmAqlNHW17NHlloij5d8Y58B0V5P5G?= =?us-ascii?Q?qe4r1gVps0noVIeqO2FUfOHIABivIH/nBOwh3dbqnI2KQikFS1GN1wF1FAg1?= =?us-ascii?Q?b8Im2gLIHVlyZYA80POZmjyrIR2yFhCL1IndX642eU8laTVpIlZ+2gOy8gRe?= =?us-ascii?Q?GILlhccns5Ekp4Mqx0iEaYXTjc942HEXB8J/NEhlYoIHjNVmBNBNOqdfAIOz?= =?us-ascii?Q?GuRG+RVUuFAogmHsUDDLe2M6OEdORfDV9eVtZGv72S3iyWAWpOSxuhTb4LLm?= =?us-ascii?Q?uLk+zgxkXGy5tQcUIb1uv4EtS2hZ4uLyW/fsztVEjPxHYEh7a4ItKpyhJNat?= =?us-ascii?Q?9uOEmnz6fxjBm7VAk46ueikKc2AiFpaXx7Qr4Am5P4uvFwBpZKBx/JkfGkvO?= =?us-ascii?Q?en5GEgH+ZA=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4025.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2e98265d-6c77-4bae-c71e-08da49c54d40 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jun 2022 03:08:17.9510 (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: 13HJBMF9J6jo9xpQuHI3amJhOfO9mhqqJXyBujBFVftrs1g+ZP7Tigwd7HaZAIqh8rJrZFrfkmAEnlxtKwzBSA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1101MB2257 Return-Path: hao.a.wu@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 A > Sent: Thursday, June 9, 2022 10:48 AM > To: Czajkowski, Maciej ; devel@edk2.groups.i= o > Cc: Ni, Ray > Subject: Re: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use > PCI_DEVICE_PPI to manage AHCI device >=20 > Couple of general level comments/questions: > 1) The implementation of functions AtaAhciPciDevicePpiInstallationCallbac= k() & > AtaAhciInitPrivateDataFromPciDevice() has many duplications. Is it possib= le to > abstract a separate function to reduce duplicated codes? > 2) What DevicePathLib instance should be used for the PEI case? As far 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 > 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 like= to > ensure the origin gEdkiiPeiAtaAhciHostControllerPpiGuid path is not broke= n. > 5) Could you help to create a GitHub Pull Request to trigger the CI 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 > > 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 > > EDKII_PCI_DEVICE_PPI and EDKII_PEI_ATA_AHCI_HOST_CONTROLLER_PPI to > > 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 to k= eep > consistency? >=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 > > 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 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 ATA > > + 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) { > > > > + return EFI_SUCCESS; > > > > + } > > > > + } 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.\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 > > + AhciAtaPassThruGetNextDevice; > > > > + Private->AtaPassThruPpi.GetDevicePath =3D > > + AhciAtaPassThruGetDevicePath; > > > > + CopyMem ( > > > > + &Private->AtaPassThruPpiList, > > > > + &mAhciAtaPassThruPpiListTemplate, > > > > + sizeof (EFI_PEI_PPI_DESCRIPTOR) > > > > + ); > > > > + Private->AtaPassThruPpiList.Ppi =3D &Private->AtaPassThruPpi; > > > > + PeiServicesInstallPpi (&Private->AtaPassThruPpiList); > > > > + > > > > + Private->BlkIoPpi.GetNumberOfBlockDevices =3D AhciBlockIoGetDeviceNo= ; > > > > + Private->BlkIoPpi.GetBlockDeviceMediaInfo =3D > > + 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 > > + AhciBlockIoGetDeviceNo2; > > > > + Private->BlkIo2Ppi.GetBlockDeviceMediaInfo =3D > > + 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 > > + &Private->StorageSecurityPpi; > > > > + PeiServicesInstallPpi (&Private->StorageSecurityPpiList); > > > > } > > > > > > > > + CopyMem ( > > > > + &Private->EndOfPeiNotifyList, > > > > + &mAhciEndOfPeiNotifyListTemplate, > > > > + sizeof (EFI_PEI_NOTIFY_DESCRIPTOR) > > > > + ); > > > > + PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList); > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +/** > > > > + Initialize AHCI controller from 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, > > + 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. >=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. >=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 > > + 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 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 > ); > } >=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, > > + 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 > AtaAhciInitPrivateDataFromPciDevice. >=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) and > > > > + // Sub Class (offset 0x0A). This controller should be an SATA > > + 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, > > + 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 > > + initialize > > private data.\n", __FUNCTION__)); >=20 >=20 > Suggest to use DEBUG_INFO here. >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > + Status =3D AtaAhciInitPrivateDataFromHostControllerPpi (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 > > 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 > > recommended > > > > - that a function such as ReadUnaligned16() be used to extract the > > 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 > > *)(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 > > 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) + > > 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