From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (NAM04-MW2-obe.outbound.protection.outlook.com [40.107.101.127]) by mx.groups.io with SMTP id smtpd.web08.7928.1633347382346116924 for ; Mon, 04 Oct 2021 04:36:22 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@os.amperecomputing.com header.s=selector2 header.b=dC11fhEq; spf=pass (domain: os.amperecomputing.com, ip: 40.107.101.127, mailfrom: nhi@os.amperecomputing.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ISTYjWF279QnBnh/lkHdzBnCy5NP5xmsPMPBYYL0XxN3zV1nQ1zmCtncZQW8DCvgjD9VQreVBA9H3/5rfrZG34+Wvt8ZgoYlNMK10DvJofCP9Ili7p6JyF3ulTz/l455EdUJdpKCYLOqbB9CrPemoenJrNG12t5hwiFqMq9ZZiabasIEpww30Gt7JK+5+3mkmrHNZdB9ZJTXQeaHnUmwkOKBn82tZbnb/JAWNODxMwR6OvVfQTmTK37+yXx7xrCGsr5wIjW9HgP5J1W/w7xmDm10MmLTT6q3t4sKH9iRVJS1BbGI4/qfuni48SbDvW8omWVxxHYK6ZiECOrArfZd+g== 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=EiG9IN5mDhFwBVOfxvfkh7LnxqkOsvcnAy/voFOVjx8=; b=SfLJocKVoJ54WoK0QmjLhKJvChCWnlbKiTdQiIMwII3d4PWmUfzRCC9MId303vqff819W4uwlQbcJBMwq7T3BNCN0QWi/Wv612DAkGQIWFM6Sj5yqN9sOI7rP7tSJxHHjcEOatuyyJCMDG4pKg5ImRaHQm5tIf662NEZ2dLt1EbtznYLFnjzbcBZtsQgRtuipXnkuLh4mAKdkzNfNe2cjeGU2Bdw9ee/+C8O1u4GUxe8ZTs62TkDr0VuQ93alPFQedcQDayjKCKtEFJk++diqipXzNWUU+nNyAdEu6cjYtTtugRwi45L8Pcegqb2wY4VlX6u4xu4UEDk3FncnXaMog== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=os.amperecomputing.com; dmarc=pass action=none header.from=os.amperecomputing.com; dkim=pass header.d=os.amperecomputing.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=os.amperecomputing.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=EiG9IN5mDhFwBVOfxvfkh7LnxqkOsvcnAy/voFOVjx8=; b=dC11fhEq+A93SeOH9qiwwNDSf4D86gOCCcfCCEHdvt+UoBbgD10zD7NJrii4HWhkwKoZgAgrMfljy3uW8DHw9bYjWp2dFqeDSVyBqX1UC2iL8HHnaBAa4Cr7sBPDC5owm7Hbw8z/P4ACW6gB4AU1QCewbWZ19/R9ftcOoKCXJ74= Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=os.amperecomputing.com; Received: from PH0PR01MB7287.prod.exchangelabs.com (2603:10b6:510:10a::21) by PH0PR01MB6391.prod.exchangelabs.com (2603:10b6:510:18::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.17; Mon, 4 Oct 2021 11:36:18 +0000 Received: from PH0PR01MB7287.prod.exchangelabs.com ([fe80::c5cb:7b53:cd09:f0ad]) by PH0PR01MB7287.prod.exchangelabs.com ([fe80::c5cb:7b53:cd09:f0ad%7]) with mapi id 15.20.4566.022; Mon, 4 Oct 2021 11:36:17 +0000 Subject: Re: [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance To: Leif Lindholm Cc: devel@edk2.groups.io, patches@amperecomputing.com, vunguyen@os.amperecomputing.com, Thang Nguyen , Chuong Tran , Phong Vo , Michael D Kinney , Ard Biesheuvel , Nate DeSimone References: <20210915155527.8176-1-nhi@os.amperecomputing.com> <20210915155527.8176-13-nhi@os.amperecomputing.com> <20210928103435.6k7hncti3j2tnicf@leviathan> From: "Nhi Pham" Message-ID: Date: Mon, 4 Oct 2021 18:36:06 +0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 In-Reply-To: <20210928103435.6k7hncti3j2tnicf@leviathan> X-ClientProxiedBy: SG2PR02CA0108.apcprd02.prod.outlook.com (2603:1096:4:92::24) To PH0PR01MB7287.prod.exchangelabs.com (2603:10b6:510:10a::21) Return-Path: nhi@os.amperecomputing.com MIME-Version: 1.0 Received: from [IPv6:2405:4802:91b7:33b0:99c8:6251:eba7:9c35] (2405:4802:91b7:33b0:99c8:6251:eba7:9c35) by SG2PR02CA0108.apcprd02.prod.outlook.com (2603:1096:4:92::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.21 via Frontend Transport; Mon, 4 Oct 2021 11:36:13 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 832c20f3-0afa-4427-3461-08d9872b2dc7 X-MS-TrafficTypeDiagnostic: PH0PR01MB6391: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:4714; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: RotemGPchZuOj9cgPFjMUEVCKLUladikwu0/lqPpJOAAR6/MbZd6+EtMLufBi5IBQxoIZBYDQqrVynTsSeGsBTvHy8FqSxNZlI4pa+t02qiy7n1KD0WHJ3gzXeTV1htspYHXHXXGyFX2udBPFdjbPsg/AIqZbDqIEFbR4Sy6A9+Dc/6IXbG3N8nE785utyhSXSnSU5IK7N2HHUA9v97AEbm6OzW5ahytcHl75kqrovh1wY3UKkq70KMELWa5fBAgvFOfbfXE8cp8ao5oyqHzd6+Zow+7MK6GLZ+cuHEk4fn+pTAuYZNMvX8tppVV6ey6FPFhEuG86pXVF/1JmUFPpsHUYyQM3E9dvRZWhKb49GAyRsu7Ge/FbhO8Zp0YIprsNoVlVLhyZLG34DbaUpWerdm5D1CPyHvKHWwWdI03ShuabmtbL1qocE6FLzqhrsvjDr/BS0OWcyct6UDdGZ5ainoPVrpERUKqrI+Se0KzdhSoBr4+NKS70rUWebN09Htr9fvKtFPPv9T3lXuF/FjUXq2NoT8y/F1kijDba76n5EjvA5dj2nTtdkXKulMGFvwpG4vRWH60pfQRm7qhluLcUOtb2KRieNYpE3W3zEqKJaOTWTv+mTNmRhdccqm/qWwr6+Um54t4qGdyM+Zx7ZUoISAzzw/v4YWqqx4v29CXQb7pDZRIqRXedRDjRcIJS0llWs4rqJuMCLhMtT0nkrPVNBWAiGXxDA+ZYmnErCmfQfg= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH0PR01MB7287.prod.exchangelabs.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(316002)(8936002)(38100700002)(54906003)(66476007)(66556008)(5660300002)(53546011)(8676002)(52116002)(66946007)(6486002)(83380400001)(19627235002)(6666004)(31686004)(186003)(2616005)(6916009)(31696002)(4326008)(2906002)(86362001)(508600001)(45980500001)(43740500002);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?KzJqemRyaXYrOWpFQU01MjY2R3hUOEpGK0VPcU9zeUdFNG81dnBzZm5PSWpY?= =?utf-8?B?TTk3TFBnbDhuOEJxOGllUWFhNG5VV1BVY0xHRlRmK2JrVFh0U3diRTRvSkhv?= =?utf-8?B?YkhlYWs3eVR2SkJXc3RJbHc4K2VMRmpleE5jWUFUc2lVWlE5d29hcG51M2pq?= =?utf-8?B?VHJTckpxd2p3bmgzOW1xTS80QlRpSW1pZVZTZGpnWDFHNElWZmtKaTFmYWRv?= =?utf-8?B?bDV3dDJhbC9NT0lqVE9YRW5IcWFkaG1sb0htN01kSFErNGpRT2FHS0tNSkFs?= =?utf-8?B?Q1JwYkhFazFtYldoMjNlTUtPd3dwTVRNd0RZZHR1Vm1udDlFZjdJWnpGSEts?= =?utf-8?B?R0JrOU9RY3FmWURvbjdQTytBekFPd2VaWjdOSktUVmZQMGxwRkY2aWx4SnZV?= =?utf-8?B?VEdBMmtEazhWd0lpWElUdzRtalloUUN0UlZoRXVoWGxiWThZVDl4VW8zbEYz?= =?utf-8?B?bHB4WjMwNklud2h2dndkUkhZNjIvYjk2NTBuaEdGV281TlVBZ3ZGMVVwc2tk?= =?utf-8?B?czYrN0pmR21abFdKY1VRZ2pwVEVwWExSaEovOHJmRWRlL2tXSGt0RWdaMzlk?= =?utf-8?B?bExmNFlMUEdrNXl3Mm5RdS9jeGR5SWtCTVVqZVQrTnI1S1NRKzdkaWtERHNY?= =?utf-8?B?YXFzNzFYRGJpdGU4TVVnMkg2ek5xVkJTbXJTLzJkS3hCN3ZzNWxWN1RMRG9M?= =?utf-8?B?Zml3d2tsaXVMd1BqT2xsdzhtcWZSbUVFc1p3SWpUcUlVdGhaQUpQZnVFWmcv?= =?utf-8?B?Uy9ycU9pYi9tSWk4bGYvZE1Vb2FPaGEzYXd5TUplVVQ3Ym5uSFhYK2VJR2V1?= =?utf-8?B?N2xXdDk5SnFGVWU3aTlHQTFTLzNEdVZiWURZV2dUTEdvMmc4dUprZ242OTN5?= =?utf-8?B?alkxZllWUUVBL2dqdFNlZDRrcnhIL01lNmk0ZEVRbmRRcjZuS3hmOTlIOTlI?= =?utf-8?B?V3JzZWRmcDFqSW1HdndTQUo5eDNFU0RsSFFxa1ZKZU1WOE9SRG1xRHRiakNR?= =?utf-8?B?eWErVFM5YUVMeGUrNVdzT1ZnSi9iM0xMa1kzTGFUT05uQVpvSzYxbG9kSUNn?= =?utf-8?B?Y3JZS2hDb1BaVktDVXFYejhEQ29SOEhEU3FXTmVibERoSGVodXVnYXFyWEhC?= =?utf-8?B?dDJCT1Bhdjc5SUU3OXFjU1ZvbDUzL05aanhMVWlTaWxyeEpQMExYTkMxWGpo?= =?utf-8?B?dTlDU2dWaU1iaklpMmFWUk0xa2RwbUxEN3o4WjE3MWdpUi9uNWdJTUIyVmdh?= =?utf-8?B?ZENpb2FJVUdycmNISFlSbnNOWTlXV0FMSWlpQlVuTXltaS9qcXVqZmtUOUlF?= =?utf-8?B?Y0pGcTJKdXJGbFVTUWovNlh6M0RYN1o5MnVmWFM0a0N6S1owcXFTQW5EcEtL?= =?utf-8?B?U1dkMG53LzBRMXppUE81eStJdFFLTE03dkZwQ2VwV0kxdWhzaE5mK1RMQ0pZ?= =?utf-8?B?cEtCNitQOVdmNzQwVjl6L2dXOGRlNXplTjJjb0JRdVB0eUsrclhrUWdkK3pG?= =?utf-8?B?QTFXcFZHS3pDdEcyTHJvYlFvanYrZHpZUGdOYmhpV2pPb3JZcEs4RVg5M1hk?= =?utf-8?B?YU8reEJBWlh0dzRBblQ3T1BYdFhXY1I4d2pRcGNsL3c4b01lNDJzdEZCUUVX?= =?utf-8?B?cElIOStCVHloZzIvSDN3OTZJZStKTy9WMVhxRWdrbkhjZVNBOUhJdTdpa0dy?= =?utf-8?B?eVQzdlgvSSt5M3RMRVVoWDFKVlBOYU1UcGpvK2N6Yy82R3Y3RWxXVjk1dEkr?= =?utf-8?B?R0FqeGpNZXpvUmZOTGNtVXU0QW1ZY08rOWZlR01vMUJsOENrcU5nazVocmYx?= =?utf-8?B?SnpYMUl6ZFhkT2NCVXpCY3EzbHY2bW55TmVPaXBlc2FIcUQzZktGVGxod3E1?= =?utf-8?Q?OdJacP/RGhiO0?= X-OriginatorOrg: os.amperecomputing.com X-MS-Exchange-CrossTenant-Network-Message-Id: 832c20f3-0afa-4427-3461-08d9872b2dc7 X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB7287.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Oct 2021 11:36:17.3259 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: aEcUgcMe+rgZcwlo7IqTgLVJNDNXyD5BTQ2C9ZMYT2nkMZoekuq9UtB9aahFVUEriEtxWa74a4NvnrnageiPPq/k7Ot79kzXIsUQehjlBPU= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR01MB6391 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Hi Leif, Thanks a lot for your review. We are heading to improve the Ac01PcieLib and addressing your comments in the PciSegmentLib, by decoupling the Ac01PcieLib into hierarchical modules. Hoping it will look better to you. Best regards, Nhi On 28/09/2021 17:34, Leif Lindholm wrote: > Apart from the request to break out Ac01PcieConfigRW and > Ac01PcieCfgIn/Out# I noticed a further thing. > > On Wed, Sep 15, 2021 at 22:55:11 +0700, Nhi Pham wrote: >> +/** >> + Get RootBridge disable status. >> + >> + @param[in] HBIndex Index to identify of PCIE Host bridge. >> + @param[in] RBIndex Index to identify of underneath PCIE Root bridge. >> + >> + @retval BOOLEAN Return RootBridge disable status. >> +**/ >> +BOOLEAN >> +Ac01PcieCheckRootBridgeDisabled ( >> + IN UINTN HBIndex, >> + IN UINTN RBIndex >> + ) >> +{ >> + UINTN RCIndex; >> + INT8 Ret; >> + >> + RCIndex = HBIndex; >> + Ret = !RCList[RCIndex].Active; >> + if (Ret) { >> + PciList[HBIndex] = -1; >> + } else { >> + PciList[HBIndex] = HBIndex; >> + } >> + if (HBIndex == (AC01_MAX_PCIE_ROOT_COMPLEX -1)) { >> + SortPciList (PciList); >> + if (!IsSlaveSocketPresent ()) { >> + AcpiPatchPciMem32 (PciList); >> + } >> + AcpiInstallMcfg (PciList); >> + AcpiInstallIort (PciList); > Should a function named "Check if RootBridge is Disabled" really have > the undocumented side effect of going off and installing ACPI tables? > > Please move that logic over to PciHostBridgeReadyToBootEvent. > > With that, I think this revision is fully reviewed. > > / > Leif > >> + } >> + return Ret; >> +}