From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web12.5985.1654742864084955546 for ; Wed, 08 Jun 2022 19:47:45 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=MOtteSSK; spf=pass (domain: intel.com, ip: 192.55.52.43, 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=1654742864; x=1686278864; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=SphUPzYC4ltuMmD3nV+OQFbOsotddXecOJuTjQA+KP0=; b=MOtteSSKM0t+VOBRbjoxdNjuRCIrDzhncaqtRrhqLVIwuoOHRSCai9qx c5+FdKLgoyBk8ItiaoD4EOO/r6B7/M81EQfe0zXCGB9jf+7wvuy7MVQBf QcqOTFYa58hQlMNAHkfP2QSHgEKSRK+BqOkRFWqzP7EmfYzxwJiDvWMx3 bDIgNHmy3jyziObAy5gQusGh2XqyjngACVdFwWZQh1xn5PyIZd4ebooUk lKAr+rjkceliN/QrgS9LGdgcq6isPups89X9rgDBRNAdbCUdJbN/xx5zZ jI6+7ERDtlzAAuh6+xuvy5cqjkVDlVkqzCDXTXylMlQRjcV4o4jp/rLQK Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10372"; a="363451770" X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="363451770" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2022 19:47:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,287,1647327600"; d="scan'208";a="724189682" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by fmsmga001.fm.intel.com with ESMTP; 08 Jun 2022 19:47:42 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX606.amr.corp.intel.com (10.22.229.19) 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 19:47:42 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Wed, 8 Jun 2022 19:47:42 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27 via Frontend Transport; Wed, 8 Jun 2022 19:47:42 -0700 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.41) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.27; Wed, 8 Jun 2022 19:47:41 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=guC22SyKoG9k1UulMHVCKIyPIs9i98BDJ98ven/WFJQly76QdlfWzW1yyrz+S/e49ffmsz9bMSInus5dYXVTjxAbxXWKF3JXfgCGb6CtvacqyoDsNR/djah9y8ZRploXty8T34neaqUaxtHnnRhHJ/hfxVBAfTVTW6T0G5LIUE1MVvf+waF+rqO6E/mR+HHVWng7sAoWnY062V33pcChSgxD6h1DRb3Mjf6CuMGqL0aHnKpa/QYu9ugDCsFWbLo+pXB7hAfeBtTYv/3jnf7C6hLQfJKVPkn805tmXwWUxvk13+6XyrV0VbeuwsZmX91ybPII+KP1qLmvJ8roOv/39A== 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=NddVh8AgxvalaPUt6z5LetIbXuZSSM49Mn7A/V0Jglo=; b=BIWWtkXj0uA84tTOREXX+XEskIZv9KjIBlugfRF3pMvgpRcwT9W29gwbXrEoWISwtBkJ8p4DbEKK9YI9/zvmnCynp/QR4VI+FCLfFx3GqytDAH76BY5nzAn78K+2k2lmTYQyDl34OxnSYWM7tQ7aCsqZ/RQk7iLOUEylI8Ln4nREusqzV1WEkzS4WbLVkUT7oken8bQkEEWb5yTc+rcvYwy6pq53psLqRSHVNmBwBYwsAtGkX/Lb9N2xaKAe/6IRpk6NEqWy31/SX6oJ3kpYCTLIh/p+osJqG6eJehNsPcl6YP65jhtsmxvGaRKSNXYXEDSpDuSI/xrnMlyX6aRBkA== 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 BYAPR11MB3528.namprd11.prod.outlook.com (2603:10b6:a03:87::26) 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 02:47:39 +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 02:47:39 +0000 From: "Wu, Hao A" To: "Czajkowski, Maciej" , "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: AQHYeaNbITTSYmoSMkeMRh0OHsejLq1GQYHg Date: Thu, 9 Jun 2022 02:47:39 +0000 Message-ID: References: <20220606124529.2152-1-maciej.czajkowski@intel.com> <20220606124529.2152-3-maciej.czajkowski@intel.com> In-Reply-To: <20220606124529.2152-3-maciej.czajkowski@intel.com> 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: 4ad06d90-ad0e-41df-da53-08da49c26b28 x-ms-traffictypediagnostic: BYAPR11MB3528: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: AGbzvIAKnkatlP6r2Q5T/Y47Ec42MeJH4H46+ZNRBxlf5B2hfAtDKXEEFh20rXQKylUszT7e4s5yAyYbear09a2OMZ+UEl1uCFgjBb8U056k+AQxnLcUZe24YGEwM/rovZ657/8xslXkB0DfJgM7tx5bhHIQOTd5OZxV9/mt+m5oaz0IqJG/TsRLDkGcAOMW+0T/fjLoL3iHo/wEwy8EUKJSeudN0F3CY8DgEhnVUJksUBNt8KSer6a31CNbdlr6kONFKbplfGdW4+R0cwc329Jrz4rJdaXpaFztTDd8Y2VfHAJPryA70puzcij6NX3XsrS6nLFketyVUEToiUxL6LqbzEnzDP8F8U1CyX+Cfe6A9NXKCKAhzF3EGv+T2rHBDF+HEs17MWaY9bE2IyVuXT/Io3gLfEN/XpuiocZDpL3n1b1Bf73IHvv1swXhDK6/JzKyyDCPoQxr9lEAKinRPDu89JwNBl5vHpS5BbKQHwJwwFTgpRqtp1c14coIPrPONMHeDK0VBGrWK7VcoTTsfFe2mQKPO46qf5Yeks72zduoIUHVHEFLjR22DxZxiNtLFNCHUHZPkB1uZM/535RzOCKvys8dqG5Ay0l3wAhjc5wFTaM6jNvmwomNhjtOK/9guisz6kcb+uEEuhC7ivH0MFxPNQUh1p+GjsbyOtUpqxawgQlejsWXX824zUFbbv8sl4L39ExO6ZPJw0NAb31s0fAurUDKpJHs3PcaAsABGfqNjpEEyq7ZBG4wL4dVLXjo/2tfWdlUE+aiGyTp1ZCp0g0ctRto3VnSmqTxXo9B5Cw= 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)(53546011)(71200400001)(7696005)(8936002)(38070700005)(966005)(76116006)(52536014)(186003)(66556008)(508600001)(5660300002)(64756008)(4326008)(66946007)(66476007)(66446008)(8676002)(110136005)(107886003)(38100700002)(30864003)(2906002)(6506007)(33656002)(82960400001)(26005)(122000001)(55016003)(19627235002)(316002)(83380400001)(86362001)(9686003)(579004)(559001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?CAc27C2Cl7oquzLHH135Xhb1y+oSj1D0tSQeP00qqOC2gErqA0a4RNCxLrwl?= =?us-ascii?Q?7VIpWl9VzFmoQUL2Gb2fCwQm1pALHx4aH8Gb+WCCbzXv750aJK5JgFzUMQ1a?= =?us-ascii?Q?zEF5Kd3FyFo59Ux+3LgmE7ECV9LevTp/rvGrumAls76E2jktkGc1LGW1BZq1?= =?us-ascii?Q?kMCg9fFwwlVWjKQZJdzKmHa2ZdDAd9J7QwoBeRpwLPbXruPJmGCy/o+npCz7?= =?us-ascii?Q?1qv2abfvvcNxw913mqNybRRGruMFMTpwQEwpeQfoRF1+/4MIboeiOf0m90cU?= =?us-ascii?Q?1iWBAUkwp2wobobWPeoZhhq0Jftql82lvyliDLOU6Dr4zb5kt9lt5gQnRUvJ?= =?us-ascii?Q?mXd3hIoqyh8I4Swez4TxOE3W7bUQ/HapZlwdTdPEcHAC+4YXMIWpXEFc9hnW?= =?us-ascii?Q?Jn5WJXltwtTN+Qonbj7EttlHQeISapvEww9hHkEfzQ0lzd33oYCjLNh1OcU1?= =?us-ascii?Q?c3t1vDhu8ZUKvjl7h+kv9O3vBlxOOEOLF6NwOyULmTDNgGsXwMAewAh3Hdyz?= =?us-ascii?Q?tFl07Etvlhq5UeaDZLiOR6Pa4C0C8+RrwB8naP1fR0a4QJaKkl3vZTaUT2T/?= =?us-ascii?Q?BZWyk2r9zY6wqsEqFL3wNiTBskkNzHKnaMqFM7maa+0e/6CmmauyNFDbkgvQ?= =?us-ascii?Q?SENig9d5qCC/n7gYbdD6ilctZHcYvd2inqxsI1UEvCpRouWkRCk1j6SC+V44?= =?us-ascii?Q?nn7AtiH7S1LWKnez5Ihma9Cm/LI17HlDY57v80v+CpuR+gBi+yDDPd9357el?= =?us-ascii?Q?6ldWrEOvQQn7qnpxdSp456KTcIF4DWPHIIDBC4nHVnbmNOGPrN5fcBFXN5Vh?= =?us-ascii?Q?VBz7jb2mgbwVbkyhU5GkeuPuCwyW15lv+QDobPldw/cZ29hfucrqLhdBdZPF?= =?us-ascii?Q?oDx2SEMMy48Ld9koDjfTH+DIYSdVFada7ytYtA3m8VcgQhmrxPB+NfK45Bwk?= =?us-ascii?Q?7gy+wAsHFRxuHUw4gOAhLlBlWgq1Wrictob3fmG98Ujbi2r2ZoqxdY+b7Pma?= =?us-ascii?Q?XW3rAZSbA5XXYfZxtF+/6h58O/rrnFmtgtiRLDaCF+SDDYj+LmX/r/OVf/lK?= =?us-ascii?Q?IPGcHIKO4r8UI13zoEza/rsj8q8rjyKR6Fq3l8rByA/9qR9NsvJDp2p+zxOu?= =?us-ascii?Q?xVvZ8uNKaY8Ncln+I2nW+2vOYksG42hoJPm0ex4I9owHIxW0oP/KwTHxxnTP?= =?us-ascii?Q?FpQsRzuo5TV6p26OJGjKmfvnIz20Na3jCnAbDblO5AzqrCHzu2/tB1kIYMGa?= =?us-ascii?Q?+RuINa2qbmDi4HDjeplS7lJyjK2MmP/tRmecATTsXpOtb263n3sdex64NkY3?= =?us-ascii?Q?ZNB7QyqusP47b4nLIqybQ+QUk9Tk/M+PRelMy/jCfxn8JRF4SxHC5IfvIN8Y?= =?us-ascii?Q?JzDjBAF9+SpMefdfVJVCbd4lXmoNaRpHOhIPdwhUXd19heJrWE8nd1lUw+ra?= =?us-ascii?Q?b8IgUr1MO4wVmqLQJDEzghGJNAYipr2l8wrOj6oqyCFHeyMaJRNR4GUgqDYa?= =?us-ascii?Q?jF4CAh+6sKdqA/+ITtkIkydD2jWBzmsQFDZwMzBtE5N6/rXz6slqw4ETr1EL?= =?us-ascii?Q?x2knN51fQ/nHQKwFQ8sDqins3bm74XQUWTgnWAFZUkhg1O1YWoU1ZUPjT33r?= =?us-ascii?Q?ag0xuVqyv0Jz+FE1aM3AWUKd8O27GlpD4QNA1NfhqPgOZVJFFOPNqHE/rmt1?= =?us-ascii?Q?MLbnASxJw4IZRFfaSwOV5MxjqV+ahxfzLHPM6yIvYSvQvRQRCVu6UD3qExas?= =?us-ascii?Q?cfreVdQ4XA=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: 4ad06d90-ad0e-41df-da53-08da49c26b28 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jun 2022 02:47:39.6512 (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: Ubo3K7dsRIOXzZcKQNtfT5KBgMk6VlDlIS+NVMDVse4q2XU7SGYSwmY8HW7UGQqK5miMLwV1d5wua56V1ylJDg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3528 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 Couple of general level comments/questions: 1) The implementation of functions AtaAhciPciDevicePpiInstallationCallback(= ) & AtaAhciInitPrivateDataFromPciDevice() has many duplications. Is it poss= ible 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 t= o ensure the origin gEdkiiPeiAtaAhciHostControllerPpiGuid path is not broke= n. 5) Could you help to create a GitHub Pull Request to trigger the CI tests f= or this series? More inline comments below: > -----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 >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3907 >=20 > 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. >=20 > 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(-) >=20 > 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 @@ > **/ >=20 >=20 >=20 > #include "AhciPei.h" >=20 > +#include >=20 > +#include >=20 > +#include >=20 > + >=20 > +/** >=20 > + Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation. >=20 > + >=20 > + @param[in] PeiServices Pointer to PEI Services Table. >=20 > + @param[in] NotifyDescriptor Pointer to the descriptor for the Notif= ication >=20 > + event that caused this function to exec= ute. >=20 > + @param[in] Ppi Pointer to the PPI data associated with= this function. >=20 > + >=20 > + @retval EFI_SUCCESS The function completes successfully >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI >=20 > +AtaAhciHostControllerPpiInstallationCallback ( >=20 > + IN EFI_PEI_SERVICES **PeiServices, >=20 > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, >=20 > + IN VOID *Ppi >=20 > + ); >=20 > + >=20 > +/** >=20 > + Callback for EDKII_PCI_DEVICE_PPI installation. >=20 > + >=20 > + @param[in] PeiServices Pointer to PEI Services Table. >=20 > + @param[in] NotifyDescriptor Pointer to the descriptor for the Notif= ication >=20 > + event that caused this function to exec= ute. >=20 > + @param[in] Ppi Pointer to the PPI data associated with= this function. >=20 > + >=20 > + @retval EFI_SUCCESS The function completes successfully >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI >=20 > +AtaAhciPciDevicePpiInstallationCallback ( >=20 > + IN EFI_PEI_SERVICES **PeiServices, >=20 > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, >=20 > + IN VOID *Ppi >=20 > + ); Could you help to put the above 2 function declarations in AhciPei.h to kee= p consistency? >=20 >=20 >=20 > EFI_PEI_PPI_DESCRIPTOR mAhciAtaPassThruPpiListTemplate =3D { >=20 > (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), >=20 > @@ -40,6 +81,18 @@ EFI_PEI_NOTIFY_DESCRIPTOR > mAhciEndOfPeiNotifyListTemplate =3D { > AhciPeimEndOfPei >=20 > }; >=20 >=20 >=20 > +EFI_PEI_NOTIFY_DESCRIPTOR mAtaAhciHostControllerNotify =3D { >=20 > + (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), >=20 > + &gEdkiiPeiAtaAhciHostControllerPpiGuid, >=20 > + AtaAhciHostControllerPpiInstallationCallback >=20 > +}; >=20 > + >=20 > +EFI_PEI_NOTIFY_DESCRIPTOR mPciDevicePpiNotify =3D { >=20 > + (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), >=20 > + &gEdkiiPeiPciDevicePpiGuid, >=20 > + AtaAhciPciDevicePpiInstallationCallback >=20 > +}; >=20 > + >=20 > /** >=20 > Free the DMA resources allocated by an ATA AHCI controller. >=20 >=20 >=20 > @@ -111,33 +164,28 @@ AhciPeimEndOfPei ( > } >=20 >=20 >=20 > /** >=20 > - Entry point of the PEIM. >=20 > + Initialize and install PrivateData PPIs. >=20 >=20 >=20 > - @param[in] FileHandle Handle of the file being invoked. >=20 > - @param[in] PeiServices Describes the list of possible PEI Services. >=20 > - >=20 > - @retval EFI_SUCCESS PPI successfully installed. >=20 > + @param[in] Private A pointer to the > PEI_AHCI_CONTROLLER_PRIVATE_DATA data >=20 > + structure. >=20 >=20 >=20 > + @retval EFI_SUCCESS AHCI controller initialized and PPIs installed >=20 > + @retval others Failed to initialize AHCI controller >=20 > **/ >=20 > EFI_STATUS >=20 > -EFIAPI >=20 > -AtaAhciPeimEntry ( >=20 > - IN EFI_PEI_FILE_HANDLE FileHandle, >=20 > - IN CONST EFI_PEI_SERVICES **PeiServices >=20 > +AtaAhciInitPrivateData ( >=20 > + IN UINTN MmioBase, >=20 > + IN EFI_DEVICE_PATH_PROTOCOL *DevicePath, >=20 > + IN UINTN DevicePathLength >=20 > ) >=20 > { >=20 > - EFI_STATUS Status; >=20 > - EFI_BOOT_MODE BootMode; >=20 > - EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; >=20 > - UINT8 Controller; >=20 > - UINTN MmioBase; >=20 > - UINTN DevicePathLength; >=20 > - EFI_DEVICE_PATH_PROTOCOL *DevicePath; >=20 > - UINT32 PortBitMap; >=20 > - PEI_AHCI_CONTROLLER_PRIVATE_DATA *Private; >=20 > - UINT8 NumberOfPorts; >=20 > + EFI_STATUS Status; >=20 > + UINT32 PortBitMap; >=20 > + UINT8 NumberOfPorts; >=20 > + PEI_AHCI_CONTROLLER_PRIVATE_DATA *Private; >=20 > + EFI_BOOT_MODE BootMode; >=20 >=20 >=20 > - DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__)); >=20 > + DEBUG ((DEBUG_INFO, "Initializing private data for ATA\n")); >=20 >=20 >=20 > // >=20 > // Get the current boot mode. >=20 > @@ -149,19 +197,149 @@ AtaAhciPeimEntry ( > } >=20 >=20 >=20 > // >=20 > - // Locate the ATA AHCI host controller PPI. >=20 > - // >=20 > - Status =3D PeiServicesLocatePpi ( >=20 > - &gEdkiiPeiAtaAhciHostControllerPpiGuid, >=20 > - 0, >=20 > - NULL, >=20 > - (VOID **)&AhciHcPpi >=20 > - ); >=20 > + // Check validity of the device path of the ATA AHCI controller. >=20 > + // >=20 > + Status =3D AhciIsHcDevicePathValid (DevicePath, DevicePathLength); >=20 > + if (EFI_ERROR (Status)) { >=20 > + DEBUG (( >=20 > + DEBUG_ERROR, >=20 > + "%a: The device path is invalid.\n", >=20 > + __FUNCTION__ >=20 > + )); >=20 > + return Status; >=20 > + } >=20 > + >=20 > + // >=20 > + // For S3 resume performance consideration, not all ports on an ATA AH= CI >=20 > + // controller will be enumerated/initialized. The driver consumes the >=20 > + // content within S3StorageDeviceInitList LockBox to get the ports tha= t >=20 > + // will be enumerated/initialized during S3 resume. >=20 > + // >=20 > + if (BootMode =3D=3D BOOT_ON_S3_RESUME) { >=20 > + NumberOfPorts =3D AhciS3GetEumeratePorts (DevicePath, DevicePathLeng= th, > &PortBitMap); >=20 > + if (NumberOfPorts =3D=3D 0) { >=20 > + return EFI_SUCCESS; >=20 > + } >=20 > + } else { >=20 > + PortBitMap =3D MAX_UINT32; >=20 > + } >=20 > + >=20 > + // >=20 > + // Memory allocation for controller private data. >=20 > + // >=20 > + Private =3D AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA= )); >=20 > + if (Private =3D=3D NULL) { >=20 > + DEBUG (( >=20 > + DEBUG_ERROR, >=20 > + "%a: Fail to allocate private data.\n", >=20 > + __FUNCTION__ >=20 > + )); >=20 > + return EFI_OUT_OF_RESOURCES; >=20 > + } >=20 > + >=20 > + // >=20 > + // Initialize controller private data. >=20 > + // >=20 > + Private->Signature =3D > AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE; >=20 > + Private->MmioBase =3D MmioBase; >=20 > + Private->DevicePathLength =3D DevicePathLength; >=20 > + Private->DevicePath =3D DevicePath; >=20 > + Private->PortBitMap =3D PortBitMap; >=20 > + InitializeListHead (&Private->DeviceList); >=20 > + >=20 > + Status =3D AhciModeInitialization (Private); >=20 > if (EFI_ERROR (Status)) { >=20 > - DEBUG ((DEBUG_ERROR, "%a: Failed to locate AtaAhciHostControllerPpi.= \n", > __FUNCTION__)); >=20 > - return EFI_UNSUPPORTED; >=20 > + return Status; >=20 > + } >=20 > + >=20 > + Private->AtaPassThruMode.Attributes =3D > EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | >=20 > + EFI_ATA_PASS_THRU_ATTRIBUTES_LOG= ICAL; >=20 > + Private->AtaPassThruMode.IoAlign =3D sizeof (UINTN); >=20 > + Private->AtaPassThruPpi.Revision =3D > EDKII_PEI_ATA_PASS_THRU_PPI_REVISION; >=20 > + Private->AtaPassThruPpi.Mode =3D &Private->AtaPassThruMode; >=20 > + Private->AtaPassThruPpi.PassThru =3D AhciAtaPassThruPassThru; >=20 > + Private->AtaPassThruPpi.GetNextPort =3D AhciAtaPassThruGetNextPort; >=20 > + Private->AtaPassThruPpi.GetNextDevice =3D AhciAtaPassThruGetNextDevice= ; >=20 > + Private->AtaPassThruPpi.GetDevicePath =3D AhciAtaPassThruGetDevicePath= ; >=20 > + CopyMem ( >=20 > + &Private->AtaPassThruPpiList, >=20 > + &mAhciAtaPassThruPpiListTemplate, >=20 > + sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > + ); >=20 > + Private->AtaPassThruPpiList.Ppi =3D &Private->AtaPassThruPpi; >=20 > + PeiServicesInstallPpi (&Private->AtaPassThruPpiList); >=20 > + >=20 > + Private->BlkIoPpi.GetNumberOfBlockDevices =3D AhciBlockIoGetDeviceNo; >=20 > + Private->BlkIoPpi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMediaInfo; >=20 > + Private->BlkIoPpi.ReadBlocks =3D AhciBlockIoReadBlocks; >=20 > + CopyMem ( >=20 > + &Private->BlkIoPpiList, >=20 > + &mAhciBlkIoPpiListTemplate, >=20 > + sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > + ); >=20 > + Private->BlkIoPpiList.Ppi =3D &Private->BlkIoPpi; >=20 > + PeiServicesInstallPpi (&Private->BlkIoPpiList); >=20 > + >=20 > + Private->BlkIo2Ppi.Revision =3D > EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION; >=20 > + Private->BlkIo2Ppi.GetNumberOfBlockDevices =3D AhciBlockIoGetDeviceNo2= ; >=20 > + Private->BlkIo2Ppi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMediaInfo= 2; >=20 > + Private->BlkIo2Ppi.ReadBlocks =3D AhciBlockIoReadBlocks2; >=20 > + CopyMem ( >=20 > + &Private->BlkIo2PpiList, >=20 > + &mAhciBlkIo2PpiListTemplate, >=20 > + sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > + ); >=20 > + Private->BlkIo2PpiList.Ppi =3D &Private->BlkIo2Ppi; >=20 > + PeiServicesInstallPpi (&Private->BlkIo2PpiList); >=20 > + >=20 > + if (Private->TrustComputingDevices !=3D 0) { >=20 > + DEBUG (( >=20 > + DEBUG_INFO, >=20 > + "%a: Security Security Command PPI will be produced.\n", >=20 > + __FUNCTION__ >=20 > + )); >=20 > + Private->StorageSecurityPpi.Revision =3D > EDKII_STORAGE_SECURITY_PPI_REVISION; >=20 > + Private->StorageSecurityPpi.GetNumberofDevices =3D > AhciStorageSecurityGetDeviceNo; >=20 > + Private->StorageSecurityPpi.GetDevicePath =3D > AhciStorageSecurityGetDevicePath; >=20 > + Private->StorageSecurityPpi.ReceiveData =3D > AhciStorageSecurityReceiveData; >=20 > + Private->StorageSecurityPpi.SendData =3D AhciStorageSecuri= tySendData; >=20 > + CopyMem ( >=20 > + &Private->StorageSecurityPpiList, >=20 > + &mAhciStorageSecurityPpiListTemplate, >=20 > + sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > + ); >=20 > + Private->StorageSecurityPpiList.Ppi =3D &Private->StorageSecurityPpi= ; >=20 > + PeiServicesInstallPpi (&Private->StorageSecurityPpiList); >=20 > } >=20 >=20 >=20 > + CopyMem ( >=20 > + &Private->EndOfPeiNotifyList, >=20 > + &mAhciEndOfPeiNotifyListTemplate, >=20 > + sizeof (EFI_PEI_NOTIFY_DESCRIPTOR) >=20 > + ); >=20 > + PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList); >=20 > + >=20 > + return EFI_SUCCESS; >=20 > +} >=20 > + >=20 > +/** >=20 > + Initialize AHCI controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI > instance. >=20 > + >=20 > + @param[in] AhciHcPpi Pointer to the AHCI Host Controller PPI instance= . >=20 > + >=20 > + @retval EFI_SUCCESS PPI successfully installed. >=20 > +**/ >=20 > +EFI_STATUS >=20 > +AtaAhciInitPrivateDataFromHostControllerPpi ( >=20 > + IN EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi >=20 > + ) >=20 > +{ >=20 > + UINT8 Controller; >=20 > + UINTN MmioBase; >=20 > + UINTN DevicePathLength; >=20 > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; >=20 > + EFI_STATUS Status; >=20 > + >=20 > Controller =3D 0; >=20 > MmioBase =3D 0; >=20 > while (TRUE) { >=20 > @@ -193,65 +371,7 @@ AtaAhciPeimEntry ( > return Status; >=20 > } >=20 >=20 >=20 > - // >=20 > - // Check validity of the device path of the ATA AHCI controller. >=20 > - // >=20 > - Status =3D AhciIsHcDevicePathValid (DevicePath, DevicePathLength); >=20 > - if (EFI_ERROR (Status)) { >=20 > - DEBUG (( >=20 > - DEBUG_ERROR, >=20 > - "%a: The device path is invalid for Controller %d.\n", >=20 > - __FUNCTION__, >=20 > - Controller >=20 > - )); >=20 > - Controller++; >=20 > - continue; >=20 > - } >=20 > - >=20 > - // >=20 > - // For S3 resume performance consideration, not all ports on an ATA = AHCI >=20 > - // controller will be enumerated/initialized. The driver consumes th= e >=20 > - // content within S3StorageDeviceInitList LockBox to get the ports t= hat >=20 > - // will be enumerated/initialized during S3 resume. >=20 > - // >=20 > - if (BootMode =3D=3D BOOT_ON_S3_RESUME) { >=20 > - NumberOfPorts =3D AhciS3GetEumeratePorts (DevicePath, DevicePathLe= ngth, > &PortBitMap); >=20 > - if (NumberOfPorts =3D=3D 0) { >=20 > - // >=20 > - // No ports need to be enumerated for this controller. >=20 > - // >=20 > - Controller++; >=20 > - continue; >=20 > - } >=20 > - } else { >=20 > - PortBitMap =3D MAX_UINT32; >=20 > - } >=20 > - >=20 > - // >=20 > - // Memory allocation for controller private data. >=20 > - // >=20 > - Private =3D AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DA= TA)); >=20 > - if (Private =3D=3D NULL) { >=20 > - DEBUG (( >=20 > - DEBUG_ERROR, >=20 > - "%a: Fail to allocate private data for Controller %d.\n", >=20 > - __FUNCTION__, >=20 > - Controller >=20 > - )); >=20 > - return EFI_OUT_OF_RESOURCES; >=20 > - } >=20 > - >=20 > - // >=20 > - // Initialize controller private data. >=20 > - // >=20 > - Private->Signature =3D > AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE; >=20 > - Private->MmioBase =3D MmioBase; >=20 > - Private->DevicePathLength =3D DevicePathLength; >=20 > - Private->DevicePath =3D DevicePath; >=20 > - Private->PortBitMap =3D PortBitMap; >=20 > - InitializeListHead (&Private->DeviceList); >=20 > - >=20 > - Status =3D AhciModeInitialization (Private); >=20 > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath, DevicePathL= ength); >=20 > if (EFI_ERROR (Status)) { >=20 > DEBUG (( >=20 > DEBUG_ERROR, >=20 > @@ -260,86 +380,261 @@ AtaAhciPeimEntry ( > Controller, >=20 > Status >=20 > )); >=20 > - Controller++; >=20 > - continue; >=20 > - } >=20 > - >=20 > - Private->AtaPassThruMode.Attributes =3D > EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | >=20 > - EFI_ATA_PASS_THRU_ATTRIBUTES_L= OGICAL; >=20 > - Private->AtaPassThruMode.IoAlign =3D sizeof (UINTN); >=20 > - Private->AtaPassThruPpi.Revision =3D > EDKII_PEI_ATA_PASS_THRU_PPI_REVISION; >=20 > - Private->AtaPassThruPpi.Mode =3D &Private->AtaPassThruMode; >=20 > - Private->AtaPassThruPpi.PassThru =3D AhciAtaPassThruPassThru; >=20 > - Private->AtaPassThruPpi.GetNextPort =3D AhciAtaPassThruGetNextPort= ; >=20 > - Private->AtaPassThruPpi.GetNextDevice =3D AhciAtaPassThruGetNextDevi= ce; >=20 > - Private->AtaPassThruPpi.GetDevicePath =3D AhciAtaPassThruGetDevicePa= th; >=20 > - CopyMem ( >=20 > - &Private->AtaPassThruPpiList, >=20 > - &mAhciAtaPassThruPpiListTemplate, >=20 > - sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > - ); >=20 > - Private->AtaPassThruPpiList.Ppi =3D &Private->AtaPassThruPpi; >=20 > - PeiServicesInstallPpi (&Private->AtaPassThruPpiList); >=20 > - >=20 > - Private->BlkIoPpi.GetNumberOfBlockDevices =3D AhciBlockIoGetDeviceNo= ; >=20 > - Private->BlkIoPpi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMediaInf= o; >=20 > - Private->BlkIoPpi.ReadBlocks =3D AhciBlockIoReadBlocks; >=20 > - CopyMem ( >=20 > - &Private->BlkIoPpiList, >=20 > - &mAhciBlkIoPpiListTemplate, >=20 > - sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > - ); >=20 > - Private->BlkIoPpiList.Ppi =3D &Private->BlkIoPpi; >=20 > - PeiServicesInstallPpi (&Private->BlkIoPpiList); >=20 > - >=20 > - Private->BlkIo2Ppi.Revision =3D > EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION; >=20 > - Private->BlkIo2Ppi.GetNumberOfBlockDevices =3D AhciBlockIoGetDeviceN= o2; >=20 > - Private->BlkIo2Ppi.GetBlockDeviceMediaInfo =3D AhciBlockIoGetMediaIn= fo2; >=20 > - Private->BlkIo2Ppi.ReadBlocks =3D AhciBlockIoReadBlocks= 2; >=20 > - CopyMem ( >=20 > - &Private->BlkIo2PpiList, >=20 > - &mAhciBlkIo2PpiListTemplate, >=20 > - sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > - ); >=20 > - Private->BlkIo2PpiList.Ppi =3D &Private->BlkIo2Ppi; >=20 > - PeiServicesInstallPpi (&Private->BlkIo2PpiList); >=20 > - >=20 > - if (Private->TrustComputingDevices !=3D 0) { >=20 > + } else { >=20 > DEBUG (( >=20 > DEBUG_INFO, >=20 > - "%a: Security Security Command PPI will be produced for Controll= er %d.\n", >=20 > + "%a: Controller %d has been successfully initialized.\n", >=20 > __FUNCTION__, >=20 > Controller >=20 > )); >=20 > - Private->StorageSecurityPpi.Revision =3D > EDKII_STORAGE_SECURITY_PPI_REVISION; >=20 > - Private->StorageSecurityPpi.GetNumberofDevices =3D > AhciStorageSecurityGetDeviceNo; >=20 > - Private->StorageSecurityPpi.GetDevicePath =3D > AhciStorageSecurityGetDevicePath; >=20 > - Private->StorageSecurityPpi.ReceiveData =3D > AhciStorageSecurityReceiveData; >=20 > - Private->StorageSecurityPpi.SendData =3D AhciStorageSecu= ritySendData; >=20 > - CopyMem ( >=20 > - &Private->StorageSecurityPpiList, >=20 > - &mAhciStorageSecurityPpiListTemplate, >=20 > - sizeof (EFI_PEI_PPI_DESCRIPTOR) >=20 > - ); >=20 > - Private->StorageSecurityPpiList.Ppi =3D &Private->StorageSecurityP= pi; >=20 > - PeiServicesInstallPpi (&Private->StorageSecurityPpiList); >=20 > } >=20 >=20 >=20 > - CopyMem ( >=20 > - &Private->EndOfPeiNotifyList, >=20 > - &mAhciEndOfPeiNotifyListTemplate, >=20 > - sizeof (EFI_PEI_NOTIFY_DESCRIPTOR) >=20 > - ); >=20 > - PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList); >=20 > - >=20 > - DEBUG (( >=20 > - DEBUG_INFO, >=20 > - "%a: Controller %d has been successfully initialized.\n", >=20 > - __FUNCTION__, >=20 > - Controller >=20 > - )); >=20 > Controller++; >=20 > } >=20 >=20 >=20 > return EFI_SUCCESS; >=20 > } >=20 > + >=20 > +/** >=20 > + Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation. >=20 > + >=20 > + @param[in] PeiServices Pointer to PEI Services Table. >=20 > + @param[in] NotifyDescriptor Pointer to the descriptor for the Notif= ication >=20 > + event that caused this function to exec= ute. >=20 > + @param[in] Ppi Pointer to the PPI data associated with= this function. >=20 > + >=20 > + @retval EFI_SUCCESS The function completes successfully Please help to add the descriptions for function returning error status. >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI >=20 > +AtaAhciHostControllerPpiInstallationCallback ( >=20 > + IN EFI_PEI_SERVICES **PeiServices, >=20 > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, >=20 > + IN VOID *Ppi >=20 > + ) >=20 > +{ >=20 > + EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; >=20 > + >=20 > + if (Ppi =3D=3D NULL) { >=20 > + return EFI_INVALID_PARAMETER; >=20 > + } >=20 > + >=20 > + AhciHcPpi =3D (EDKII_ATA_AHCI_HOST_CONTROLLER_PPI*) Ppi; >=20 > + >=20 > + return AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi); >=20 > +} >=20 > + >=20 > +/** >=20 > + Callback for EDKII_PCI_DEVICE_PPI installation. >=20 > + >=20 > + @param[in] PeiServices Pointer to PEI Services Table. >=20 > + @param[in] NotifyDescriptor Pointer to the descriptor for the Notif= ication >=20 > + event that caused this function to exec= ute. >=20 > + @param[in] Ppi Pointer to the PPI data associated with= this function. >=20 > + >=20 > + @retval EFI_SUCCESS The function completes successfully Please help to add the descriptions for function returning error status. >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI >=20 > +AtaAhciPciDevicePpiInstallationCallback ( >=20 > + IN EFI_PEI_SERVICES **PeiServices, >=20 > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor, >=20 > + IN VOID *Ppi >=20 > + ) >=20 > +{ >=20 > + EDKII_PCI_DEVICE_PPI *PciDevice; >=20 > + PCI_TYPE00 PciData; >=20 > + UINTN MmioBase; >=20 > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; >=20 > + UINTN DevicePathLength; >=20 > + EFI_STATUS Status; >=20 > + >=20 > + PciDevice =3D (EDKII_PCI_DEVICE_PPI*) Ppi; >=20 > + >=20 > + // >=20 > + // Now further check the PCI header: Base Class (offset 0x0B) and >=20 > + // Sub Class (offset 0x0A). This controller should be an SATA controll= er >=20 > + // >=20 > + Status =3D PciDevice->PciIo.Pci.Read ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoWidthUint8, >=20 > + PCI_CLASSCODE_OFFSET, >=20 > + sizeof (PciData.Hdr.ClassCode), >=20 > + PciData.Hdr.ClassCode >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + return EFI_UNSUPPORTED; >=20 > + } >=20 > + >=20 > + if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) { >=20 > + return EFI_UNSUPPORTED; >=20 > + } >=20 > + >=20 > + Status =3D PciDevice->PciIo.Attributes ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoAttributeOperationSet, >=20 > + EFI_PCI_DEVICE_ENABLE, >=20 > + NULL >=20 > + ); Do you think we need to align with AtaAtapiPassThru DXE counterpart when en= abling the controller? MdeModulePkg\Bus\Ata\AtaAtapiPassThru\AtaAtapiPassThru.c - AtaAtapiPassThru= Start(): 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 > + if (EFI_ERROR (Status)) { >=20 > + return EFI_UNSUPPORTED; >=20 > + } >=20 > + >=20 > + Status =3D PciDevice->PciIo.Pci.Read ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoWidthUint32, >=20 > + 0x24, >=20 > + sizeof (UINTN), >=20 > + &MmioBase >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + return EFI_UNSUPPORTED; >=20 > + } >=20 > + >=20 > + DevicePathLength =3D GetDevicePathSize (PciDevice->DevicePath); >=20 > + DevicePath =3D PciDevice->DevicePath; >=20 > + >=20 > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath, DevicePathLen= gth); >=20 > + if (EFI_ERROR (Status)) { >=20 > + DEBUG (( >=20 > + DEBUG_INFO, >=20 > + "%a: Failed to init controller, with Status - %r\n", >=20 > + __FUNCTION__, >=20 > + Status >=20 > + )); >=20 > + } >=20 > + >=20 > + return EFI_SUCCESS; >=20 > +} >=20 > + >=20 Missing function description comments for AtaAhciInitPrivateDataFromPciDevi= ce. > +EFI_STATUS >=20 > +AtaAhciInitPrivateDataFromPciDevice ( >=20 > + VOID >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status; >=20 > + UINTN ControllerIndex; >=20 > + EDKII_PCI_DEVICE_PPI *PciDevice; >=20 > + PCI_TYPE00 PciData; >=20 > + UINTN MmioBase; >=20 > + EFI_DEVICE_PATH_PROTOCOL *DevicePath; >=20 > + UINTN DevicePathLength; >=20 > + >=20 > + Status =3D EFI_SUCCESS; >=20 > + for (ControllerIndex =3D 0; Status !=3D EFI_NOT_FOUND; ControllerIndex= ++ ) { >=20 > + Status =3D PeiServicesLocatePpi ( >=20 > + &gEdkiiPeiPciDevicePpiGuid, >=20 > + ControllerIndex, >=20 > + NULL, >=20 > + (VOID**) &PciDevice >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + continue; >=20 > + } >=20 > + >=20 > + // >=20 > + // Now further check the PCI header: Base Class (offset 0x0B) and >=20 > + // Sub Class (offset 0x0A). This controller should be an SATA contro= ller >=20 > + // >=20 > + Status =3D PciDevice->PciIo.Pci.Read ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoWidthUint8, >=20 > + PCI_CLASSCODE_OFFSET, >=20 > + sizeof (PciData.Hdr.ClassCode), >=20 > + PciData.Hdr.ClassCode >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + continue; >=20 > + } >=20 > + >=20 > + if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) { >=20 > + continue; >=20 > + } >=20 > + >=20 > + Status =3D PciDevice->PciIo.Attributes ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoAttributeOperationSet, >=20 > + EFI_PCI_DEVICE_ENABLE, >=20 > + NULL >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + continue; >=20 > + } >=20 > + >=20 > + Status =3D PciDevice->PciIo.Pci.Read ( >=20 > + &PciDevice->PciIo, >=20 > + EfiPciIoWidthUint32, >=20 > + 0x24, >=20 > + sizeof (UINTN), >=20 > + &MmioBase >=20 > + ); >=20 > + if (EFI_ERROR (Status)) { >=20 > + continue; >=20 > + } >=20 > + >=20 > + DevicePathLength =3D GetDevicePathSize (PciDevice->DevicePath); >=20 > + DevicePath =3D PciDevice->DevicePath; >=20 > + >=20 > + Status =3D AtaAhciInitPrivateData (MmioBase, DevicePath, DevicePathL= ength); >=20 > + if (EFI_ERROR (Status)) { >=20 > + DEBUG (( >=20 > + DEBUG_INFO, >=20 > + "%a: Failed to init controller, with Status - %r\n", >=20 > + __FUNCTION__, >=20 > + Status >=20 > + )); >=20 > + } >=20 > + // >=20 > + // Override the status to continue the for loop >=20 > + // >=20 > + Status =3D EFI_SUCCESS; >=20 > + } >=20 > + >=20 > + return EFI_SUCCESS; >=20 > +} >=20 > + >=20 > +/** >=20 > + Entry point of the PEIM. >=20 > + >=20 > + @param[in] FileHandle Handle of the file being invoked. >=20 > + @param[in] PeiServices Describes the list of possible PEI Services. >=20 > + >=20 > + @retval EFI_SUCCESS PPI successfully installed. >=20 > + >=20 > +**/ >=20 > +EFI_STATUS >=20 > +EFIAPI >=20 > +AtaAhciPeimEntry ( >=20 > + IN EFI_PEI_FILE_HANDLE FileHandle, >=20 > + IN CONST EFI_PEI_SERVICES **PeiServices >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status; >=20 > + EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *AhciHcPpi; >=20 > + >=20 > + DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__)); >=20 > + >=20 > + PeiServicesNotifyPpi (&mAtaAhciHostControllerNotify); >=20 > + >=20 > + PeiServicesNotifyPpi (&mPciDevicePpiNotify); >=20 > + >=20 > + Status =3D AtaAhciInitPrivateDataFromPciDevice (); >=20 > + >=20 > + // >=20 > + // Locate the ATA AHCI host controller PPI. >=20 > + // >=20 > + Status =3D PeiServicesLocatePpi ( >=20 > + &gEdkiiPeiAtaAhciHostControllerPpiGuid, >=20 > + 0, >=20 > + NULL, >=20 > + (VOID **)&AhciHcPpi >=20 > + ); >=20 > + if (!EFI_ERROR (Status)) { >=20 > + DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi to initiali= ze > private data.\n", __FUNCTION__)); Suggest to use DEBUG_INFO here. Best Regards, Hao Wu >=20 > + Status =3D AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi); >=20 > + } >=20 > + >=20 > + return Status; >=20 > +} >=20 > 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 { > } >=20 > }; >=20 >=20 >=20 > -/** >=20 > - Returns the 16-bit Length field of a device path node. >=20 > - >=20 > - Returns the 16-bit Length field of the device path node specified by N= ode. >=20 > - Node is not required to be aligned on a 16-bit boundary, so it is reco= mmended >=20 > - that a function such as ReadUnaligned16() be used to extract the conte= nts of >=20 > - the Length field. >=20 > - >=20 > - If Node is NULL, then ASSERT(). >=20 > - >=20 > - @param Node A pointer to a device path node data structure. >=20 > - >=20 > - @return The 16-bit Length field of the device path node specified by N= ode. >=20 > - >=20 > -**/ >=20 > -UINTN >=20 > -DevicePathNodeLength ( >=20 > - IN CONST VOID *Node >=20 > - ) >=20 > -{ >=20 > - ASSERT (Node !=3D NULL); >=20 > - return ReadUnaligned16 ((UINT16 *)&((EFI_DEVICE_PATH_PROTOCOL > *)(Node))->Length[0]); >=20 > -} >=20 > - >=20 > -/** >=20 > - Returns a pointer to the next node in a device path. >=20 > - >=20 > - If Node is NULL, then ASSERT(). >=20 > - >=20 > - @param Node A pointer to a device path node data structure. >=20 > - >=20 > - @return a pointer to the device path node that follows the device path= node >=20 > - specified by Node. >=20 > - >=20 > -**/ >=20 > -EFI_DEVICE_PATH_PROTOCOL * >=20 > -NextDevicePathNode ( >=20 > - IN CONST VOID *Node >=20 > - ) >=20 > -{ >=20 > - ASSERT (Node !=3D NULL); >=20 > - return (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)(Node) + > DevicePathNodeLength (Node)); >=20 > -} >=20 > - >=20 > /** >=20 > Get the size of the current device path instance. >=20 >=20 >=20 > 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 >=20 > LockBoxLib >=20 > PeimEntryPoint >=20 > + DevicePathLib >=20 >=20 >=20 > [Ppis] >=20 > gEdkiiPeiAtaAhciHostControllerPpiGuid ## CONSUMES >=20 > gEdkiiIoMmuPpiGuid ## CONSUMES >=20 > gEfiEndOfPeiSignalPpiGuid ## CONSUMES >=20 > + gEdkiiPeiPciDevicePpiGuid ## CONSUMES >=20 > gEdkiiPeiAtaPassThruPpiGuid ## SOMETIMES_PRODUCES >=20 > gEfiPeiVirtualBlockIoPpiGuid ## SOMETIMES_PRODUCES >=20 > gEfiPeiVirtualBlockIo2PpiGuid ## SOMETIMES_PRODUCES >=20 > @@ -65,8 +67,7 @@ [Guids] >=20 >=20 > [Depex] >=20 > gEfiPeiMemoryDiscoveredPpiGuid AND >=20 > - gEfiPeiMasterBootModePpiGuid AND >=20 > - gEdkiiPeiAtaAhciHostControllerPpiGuid >=20 > + gEfiPeiMasterBootModePpiGuid >=20 >=20 >=20 > [UserExtensions.TianoCore."ExtraFiles"] >=20 > AhciPeiExtra.uni >=20 > -- > 2.27.0.windows.1