From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR03-DB5-obe.outbound.protection.outlook.com (EUR03-DB5-obe.outbound.protection.outlook.com [40.107.4.62]) by mx.groups.io with SMTP id smtpd.web09.2299.1621369584169707465 for ; Tue, 18 May 2021 13:26:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=1GJYj8+x; spf=pass (domain: arm.com, ip: 40.107.4.62, mailfrom: sami.mujawar@arm.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sJ/2WQ6HgJWozARd4zQkvAe/KoL59dHSbkm81KsuPUw=; b=1GJYj8+xNzOm1b/Q9XBWpUfMAhwoSoU04X7zNGYH1EqaddCbWz9M24saMjatKP6NF8OA0Tm7r8wjMjNqqUvgn6t+0l1mMWwH1EYBDObmh/XQJs2cWMoqWPDypyqeW2hMKL0vaeIEhxpXuZhkWfRgDFTkDRkqKttiP7VRpWSmcJA= Received: from AM5PR1001CA0041.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:206:15::18) by AM0PR08MB5075.eurprd08.prod.outlook.com (2603:10a6:208:15b::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25; Tue, 18 May 2021 20:26:21 +0000 Received: from AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com (2603:10a6:206:15:cafe::6a) by AM5PR1001CA0041.outlook.office365.com (2603:10a6:206:15::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25 via Frontend Transport; Tue, 18 May 2021 20:26:21 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; edk2.groups.io; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;edk2.groups.io; dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT005.mail.protection.outlook.com (10.152.16.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25 via Frontend Transport; Tue, 18 May 2021 20:26:20 +0000 Received: ("Tessian outbound 3050e7a5b95d:v92"); Tue, 18 May 2021 20:26:20 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 76e5fefe209d9584 X-CR-MTA-TID: 64aa7808 Received: from 0a3fb6adb129.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 6A8F3656-0555-4431-8DA8-7E0A611EFB67.1; Tue, 18 May 2021 20:26:09 +0000 Received: from EUR01-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 0a3fb6adb129.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 18 May 2021 20:26:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A9Yq1ehj7QjQ/rqcLblThDiJ2FhGAqbiWSl/B7yp31hXSZVcrNYGCrWyi+g2tJqxS8TNUNvWnxkDiFJoxX/h2ncqYkctmAHGSaUmG2oeu8m9QvI3Wkrql2x4QWz6G9a37Jsni+lLFd9sFmRQLfGvuvXN3KnWhaDlr8ZipPL3h5BuOT/ApmoDuNpHNQ6KaX89lPPP3JBffl02ZBsn57nNJtGwYkeAiH6bJ/8h4v0c03ha3iNhsbE3sHytc98zokelqrmD2onjnEPp2ANkMi5GRmIbn91u0V7oEvQXGs1lAkH7Zss54dqjcHQGXXL5EQY68PF/PTMc9Zw4f/fTOFuNxA== 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-SenderADCheck; bh=sJ/2WQ6HgJWozARd4zQkvAe/KoL59dHSbkm81KsuPUw=; b=V4KrJLbFuQ2HBNZzX8epvFs12ygnS85FVRMa8ToFLcuj0gpcTdIKvWhLJPYFYMm2blXMiIWLyU39akpW4FbntWzdWv/d61tfsiSoIl9b1TsCONGKXIHzXuMHI1kNmq1W4wNhYZzMczrwun1I4O+vouYb7EmWoS3Moq4dTyNOw0VNR4eQA/psL8QS4UMAdneL8wzqtd746Ia8M/nnwexA28mUHElTh2c3WJl7O7x2WdylYL8hdSb2zbW/podlXH6iNzgbusSx1kBVPHrQ4R1JRthoRl/LZOd642pO8EI9i5HlctJCMh0t9MFRczpylFQQWozq1N66biE9hFCu6U+u6Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sJ/2WQ6HgJWozARd4zQkvAe/KoL59dHSbkm81KsuPUw=; b=1GJYj8+xNzOm1b/Q9XBWpUfMAhwoSoU04X7zNGYH1EqaddCbWz9M24saMjatKP6NF8OA0Tm7r8wjMjNqqUvgn6t+0l1mMWwH1EYBDObmh/XQJs2cWMoqWPDypyqeW2hMKL0vaeIEhxpXuZhkWfRgDFTkDRkqKttiP7VRpWSmcJA= Authentication-Results-Original: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by AM6PR08MB4213.eurprd08.prod.outlook.com (2603:10a6:20b:8e::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.26; Tue, 18 May 2021 20:26:07 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::80cb:878d:c8f1:2688]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::80cb:878d:c8f1:2688%7]) with mapi id 15.20.4129.033; Tue, 18 May 2021 20:26:07 +0000 Subject: Re: [PATCH v2 3/5] ArmVirtPkg: enable ACPI for cloud hypervisor To: Jianyong Wu , devel@edk2.groups.io, lersek@redhat.com, ardb+tianocore@kernel.org Cc: hao.a.wu@intel.com, justin.he@arm.com, nd@arm.com References: <20210517065032.82423-1-jianyong.wu@arm.com> <20210517065032.82423-4-jianyong.wu@arm.com> From: "Sami Mujawar" Message-ID: <1a22b547-344c-65a2-bbc8-fbb4c70b93ab@arm.com> Date: Tue, 18 May 2021 21:26:05 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 In-Reply-To: <20210517065032.82423-4-jianyong.wu@arm.com> X-Originating-IP: [217.140.106.52] X-ClientProxiedBy: LO4P123CA0385.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:18f::12) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [10.1.196.43] (217.140.106.52) by LO4P123CA0385.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:18f::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.25 via Frontend Transport; Tue, 18 May 2021 20:26:07 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 11c5c08c-a232-455d-8e1c-08d91a3b32ce X-MS-TrafficTypeDiagnostic: AM6PR08MB4213:|AM0PR08MB5075: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: 1H1HZLwCkhLac/0VtKgxnTJEyE99AL8lJKMbOtt0rla8nzfpT0zPL8C3TN7EGzmsBOpJmm2auWFyxCgvDY0ljsdPKYblcyeneaky7luq8JnR8z/aKr0B2dwLb+VRnLEA3CBgeYXXv94U5YtnZsSawAvu2hvTBjgT3bbUhPZUsf7vwtAGhgnlYUxPGRri3d/VvoWRkumO2H5VxD+d7uA53chq869YMC4t0rwdjAPMQ04tjKxInyxFbEC8ypv8+W5wdPWbBgnyF1ay1NbyCTgm8yo0z6JT8z3ypkvjbx94S70wcxZAu3BDAEANVP6QLQnMq/+xo+1Z8DnOFE7XAn5LuQ2NgXQs6CwvBx3XJ8sjVHqcEUtCjXXb5Igzgz2yM0+ziH45H2Vfn209jGPIRh5sllFYMRuOGx5qHaZmXobJnvD1Wh9uRTgaXfpU+lneD+5lPUnCDejewIcuZ7PZ8mD5aU/K9zj5OeBjgS67ybu5Zls5AhDqBkeyqZGbXyADeZymLPYwgEVn57uucdFEBUoZ+RlC06NhLH41HhUBCJCXWvajHn9sTow7H3HC7lX9B/ENfMMI61vrnIl6UzmEwS7PdfXwHc2ccQpSCfWw3m3jJQqaAVOZslSGff/PRVwMO7DB2p1Pi1VAdN0AdfU7Jk33mB8rJAPyBQdxycrBtY2aNBZpClY/pt+RFftVch3DvIRCIFE2loHjSBhyxJoNUF2d5Fuw64ltvCPR6ehScPUfc2VwoGe5xMpRQBBKSGxADur26pZodiLCcIWb+DmxdqvZQkq5Cfba4aBTnFbBvUQKYQw= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(346002)(376002)(136003)(396003)(39850400004)(66556008)(16576012)(316002)(66946007)(956004)(52116002)(44832011)(2616005)(966005)(36756003)(66476007)(83380400001)(53546011)(33964004)(30864003)(166002)(86362001)(5660300002)(38100700002)(6486002)(38350700002)(16526019)(26005)(2906002)(31686004)(31696002)(478600001)(8936002)(8676002)(186003)(4326008)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?azk5OVZyckpPUy9oeTFtSXJhcTZnd2lrMm9EMDV1bGxkTnc0RGxFUDcvUDV2?= =?utf-8?B?MDRxaFpqQW5iU2ppNTg2bVNaSXNwWVRVR0tGS3VWMUU3M1dWdnNiUm54dGxU?= =?utf-8?B?VHFuL2Z0dG45TGtMRnpIOU4wQzVuc2pzZ3JUMWhJM2h4eFZoTU9aOUdEeDky?= =?utf-8?B?aFZIcU1CcGU5Y0QxQUhQTFd5QzBhenR6OHhpb1N4RXJ1YVhRUjBFejZDMlZq?= =?utf-8?B?QStqSjZiVkpBVUR3NEhsYkcyT1lwa3dzQWZRV1VsMzBrUEJFaWhkQzAwbm5v?= =?utf-8?B?K21oNE1pdlhiZW5SSGYrQ0N4MmNubWJmaVlJWXNvcHJjdVhlL2l3dXF3TlNX?= =?utf-8?B?TGlrNEZ6WlgvK1c4ZXh4dWJGZjZienp1UmNScmYzdk5wb0FiWGRYMWcrbUNC?= =?utf-8?B?SkVJN3JVQ2RGaTdYdDg4eEFQVzVRZ3YzQVZEOFF5Qjd1NHcvZE4reXlPOU14?= =?utf-8?B?ZUhmdVRkYTh3TU41RjdsN0Q5S1hadjhVM2dLNExXV2syVU5WdzQraUdFN0I0?= =?utf-8?B?WkVHcy9VbitNbUZkcysyUGs1TWJ4Q2dIUDhBMi9YOE5DMmZNSXlZRy84K3VE?= =?utf-8?B?d2UrK0g5WnB6c3NCeVd6MDFVUmhTSnFsYjg5L0I4dHBFVFBJUzhhcytHWldj?= =?utf-8?B?WmZYcnJOZFZyVHVIbEF2dkxuZXZWRHFtZ3JRZjZGbmNsYWRSUUY2YVNGM2cy?= =?utf-8?B?Tm5VTkdHZzVHNkg4cXNjdDhpeWo1aXE3dUVobHF4amk2SGFOdjEybXJjYVhC?= =?utf-8?B?T04rSzZFYWFMdDVOa3RPT2I3ME81dGxXeHJERnBUejVpTytlVXI0WlgweUJj?= =?utf-8?B?L2kzbHpnOWYwcmdHNlBkUUZiZUxZMEpiT0JHSG82RDdDRmJnYkdZeHluQjJj?= =?utf-8?B?RFFwMHp4aER6cWRjdGk1d0w5VVAwK3J2cVpRNVAxRWtQSHcyQ3U3WWduN1l1?= =?utf-8?B?OTg2OGI3eTUzdisvMlVXY09GVFcrTG9qbml0MEQwd3pldEZlZ1FiSUQ5azBx?= =?utf-8?B?cG9RbG8zc2RHUFBmZkU0UkNuem9tKzBnT25HejNuN2liVVRFREtYNmxKalZL?= =?utf-8?B?OGZxbFMrdXdOTWM4RitxbVRJOGZNaHVDa3ZHdXdaclViZDZLL3pXeTBpM2lk?= =?utf-8?B?WEhZUFFRR2RDR2MxbnBOTHNTc1JoUzhndER2dlRYQnhEVkROVjZGeEtGZEI5?= =?utf-8?B?K2Y2aHRuRko0VWg5RUV1Rk1FZ3I4V0hvTVRqZGJJMDd2T09NbHM0cEp4T01m?= =?utf-8?B?RkM4VENXUVlIenJtWWhzZ3MzYmVFbEdycWZJOW1Xa2c0Y2NQQThVRVA2dzc1?= =?utf-8?B?bnFmWVZvT2s1RThFelJZMkQyMnc4U0FxK2ovMHlxT0xnVnZ4b2gyS1JQN3JF?= =?utf-8?B?UExVdm1JUXgxckROTkFGSk1ib01wb0JLeTFZY2ZKTTN0ZWo0dmF0MVk1YVp3?= =?utf-8?B?dWY0Z0RIUmpmNkNQNEYrdWRCUjdoUVFpbE1zQWtIcWNxU092K3NDNk9ER1Bj?= =?utf-8?B?eTI2VThaa3d4YkozTSt1RmxvTVJZL1lyWU9PUUs1M1dBbGExNnJDaDArZ2ta?= =?utf-8?B?WUhqMXVueE9nb1ZKa3ZYajJENEZCWUdBOVZjVVpMVmh4RW5ZbkdMMHlWU21L?= =?utf-8?B?UWwyVHh5QjF5UUx6UncxMjJpTnRDcGlGN2V2UnJIbURtazREQWpYaXlEVG44?= =?utf-8?B?dFNERzNORFpHalNUU1NZYW1Ta09XTXZPR0QzODZyUmRZaElwQSt3UEJ2bjNt?= =?utf-8?Q?a7uvl6WX7V+VRnkC0Z/pyAfpcRnK4YCp3AH1D+p?= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB4213 Original-Authentication-Results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: 66933d92-cc78-4cb2-eb64-08d91a3b2ab3 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: njR5pNQVmwYlwgO5sqMr8awxxEC9zixxbG/52MeF1Q5D8ILEW/UXP/9cKsJJScye22CuNL4MqecQD014OgC4xpe+6rdOzVIF2x+1pyCQylOV/JVbEz0+UvTf6XM2InVt97H+w2B0uQtM1R0yHpsQ4OScy9owYbCl3/9DSCj/GqtlmISOit1MgTU+NKYVRTzLRpuJR/NdxueoS8kEbqWcNC9UkoDScaw5lanU4odeqd+bWjUuaOIRWPDqcY/vB97kL62F2tZNVIXnZHMs118OdKvS30NtiyLgre1x9BFz5eSGQbZVfPnxpa+dLGb335keM2a4zeuDQtxQ7ohSi8pJw3xzDzG2rO4jMsOldDTTkYk5DsL3AykbNN0Kpnv7fw0ezE78RRQvMCk7hW4PvQ1RQDGZ0LxVf+8GW++a+c/Ab+PSHFzKD9jzI/4Va7cukdywJj+9dAlyf5KsENx8hG6D7OMvqUbAuc9wK4SxZ/hUhWCZW5OOUvu+IqC13mKlsnKZg3xMERB72eUL2P5Pxv2OLaorz3JaCpQvzV2+ccrhYpvBqKGYQo3Xka300Qy4UVxXF05muccxLNbh+U4GvasFap3XlDiXhYakvo8BIZSdojtrPk9Pa3vdNRRHEZLKp8x1KairjI45AvqwUbpis/W0TlSgo1v2kGx1+ZVNYu7SBs5ljnn8vZ+orQFiYdlXOUabXtwJVrPm9PX0w6bPHw4vVLQnSdzxqloRAP0YkmmRQBGYcSJ1Vgim8u+bMtBtHfxPWsIvwQ7l6ovOJhVGhIXTiBB7gvCmDW3Pcwf6IuQa5Yk3jL15aHsXAHlofOSVfqLp X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(4636009)(376002)(136003)(396003)(39850400004)(346002)(36840700001)(46966006)(2906002)(82310400003)(4326008)(82740400003)(356005)(81166007)(16576012)(166002)(966005)(478600001)(83380400001)(44832011)(70206006)(33964004)(31696002)(53546011)(316002)(6486002)(70586007)(8676002)(5660300002)(36860700001)(30864003)(26005)(8936002)(36756003)(16526019)(2616005)(47076005)(336012)(86362001)(956004)(186003)(31686004)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 May 2021 20:26:20.6726 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 11c5c08c-a232-455d-8e1c-08d91a3b32ce X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT005.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB5075 Content-Type: multipart/alternative; boundary="------------CB655411E2D5CDF4EF25407F" Content-Language: en-GB --------------CB655411E2D5CDF4EF25407F Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Jianyon, Thank you for this patch. Please find my response inline marked [SAMI]. Regards, Sami Mujawar On 17/05/2021 07:50 AM, Jianyong Wu wrote: > The current implementation of PlatformHasAcpiDt is not a common > library and is on behalf of qemu. So give a specific version for > Cloud Hypervisor here. > > There is no device like Fw-cfg in qemu in Cloud Hypervisor, so a specific > Acpi handler is introduced here. > > The handler implemented here is in a very simple way: > firstly, aquire the Rsdp address from the PCD varaible in the top > ".dsc"; > secondly, get the Xsdp address from Rsdp structure; > thirdly, get the Acpi tables following the Xsdp structrue and install it > one by one. > > Signed-off-by: Jianyong Wu > --- > .../CloudHvAcpiPlatformDxe.inf | 51 +++++++++++++ > .../CloudHvHasAcpiDtDxe.inf | 43 +++++++++++ > .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c | 73 +++++++++++++++++++ > .../CloudHvHasAcpiDtDxe.c | 69 ++++++++++++++++++ > 4 files changed, 236 insertions(+) > create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf > create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf > create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c > create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c [SAMI] I think these should be split as 2 patches covering CloudHvAcpiPlatformDx/* and CloudHvPlatformHasAcpiDtDx/* > > diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf > new file mode 100644 > index 000000000000..63c74e84eb27 > --- /dev/null > +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf > @@ -0,0 +1,51 @@ > +## @file > +# OVMF ACPI Platform Driver for Cloud Hypervisor [SAMI] I think the reference to OVMF can be removed, right? > +# > +# Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = ClhFwCfgAcpiPlatform [SAMI] Can ClhFwCfgAcpiPlatform be changed to CloudHvAcpiPlatform? > + FILE_GUID = 6c76e407-73f2-dc1c-938f-5d6c4691ea93 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = CloudHvAcpiPlatformEntryPoint > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 ARM AARCH64 [SAMI] Minor. The above line is just a comment, but can you revisit this, please? > +# > + > +[Sources] > + CloudHvAcpi.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + MemoryAllocationLib > + OrderedCollectionLib > + UefiBootServicesTableLib > + UefiDriverEntryPoint > + > +[Protocols] > + gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED > + gEfiPciIoProtocolGuid # PROTOCOL SOMETIMES_CONSUMED [SAMI] gEfiPciIoProtocolGuidis not used in this module. > + > +[Guids] > + gRootBridgesConnectedEventGroupGuid [SAMI] gRootBridgesConnectedEventGroupGuid not used in this module. > + > +[Pcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration [SAMI] PcdPciDisableBusEnumerationis not used in this module. > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiRsdpBaseAddress > + > +[Depex] > + gEfiAcpiTableProtocolGuid > diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf > new file mode 100644 > index 000000000000..f511a4f5064c > --- /dev/null > +++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf > @@ -0,0 +1,43 @@ > +## @file > +# Decide whether the firmware should expose an ACPI- and/or a Device Tree-based > +# hardware description to the operating system. > +# > +# Copyright (c) 2017, Red Hat, Inc. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +## > + > +[Defines] > + INF_VERSION = 1.25 > + BASE_NAME = ClhPlatformHasAcpiDtDxe [SAMI] Is it possible to change the Clh prefix to CloudHv. Same comment for other places in this patch series. > + FILE_GUID = 71fe72f9-6dc1-199d-5054-13b4200ee88d > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = PlatformHasAcpiDt > + > +[Sources] > + CloudHvHasAcpiDtDxe.c > + > +[Packages] > + ArmVirtPkg/ArmVirtPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + PcdLib > + UefiBootServicesTableLib > + UefiDriverEntryPoint > + > +[Guids] > + gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL > + gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL > + > +[Pcd] > + gArmVirtTokenSpaceGuid.PcdForceNoAcpi > + > +[Depex] > + gEfiVariableArchProtocolGuid > diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c > new file mode 100644 > index 000000000000..c2344e7b29fa > --- /dev/null > +++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c > @@ -0,0 +1,73 @@ [SAMI] File license header is missing. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define ACPI_ENTRY_SIZE 8 > +#define XSDT_LEN 36 > + > +STATIC > +EFI_ACPI_TABLE_PROTOCOL * > +FindAcpiTableProtocol ( > + VOID > + ) [SAMI] Please add doxygen header for function. Same comment for other functions introduced in this series. > +{ > + EFI_STATUS Status; > + EFI_ACPI_TABLE_PROTOCOL *AcpiTable; > + > + Status = gBS->LocateProtocol ( > + &gEfiAcpiTableProtocolGuid, > + NULL, > + (VOID**)&AcpiTable > + ); > + ASSERT_EFI_ERROR (Status); > + return AcpiTable; > +} > + > +EFI_STATUS > +EFIAPI > +InstallCloudHvAcpiTables ( > + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol > + ) > +{ > + UINTN InstalledKey, TableSize; > + UINT64 Rsdp, Xsdt, table_offset, PointerValue; [SAMI] Please have a look at variable naming conventions in EDKII coding standard at https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/4_naming_conventions > + EFI_STATUS Status = 0; [SAMI] Status = EFI_SUCCESS. > + int size; [SAMI] UINTN should be used instead of 'int'. Also variable name does not confirm to coding standard. > + > + Rsdp = PcdGet64 (PcdAcpiRsdpBaseAddress); > + Xsdt = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER *)Rsdp)->XsdtAddress; > + PointerValue = Xsdt; > + table_offset = Xsdt; > + size = ((EFI_ACPI_COMMON_HEADER *)PointerValue)->Length - XSDT_LEN; > + table_offset += XSDT_LEN; [SAMI] I think the macro name XSDT_LEN is a bit misleading. 'sizeof (EFI_ACPI_DESCRIPTION_HEADER)' should be used instead of XSDT_LEN. > + > + while(!Status && size > 0) { [SAMI] Status is not boolean, use'EFI_ERROR (Status)' instead. Also use paranthesis to add clarity. > + PointerValue = *(UINT64 *)table_offset; > + TableSize = ((EFI_ACPI_COMMON_HEADER *)PointerValue)->Length; > + Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol, > + (VOID *)(UINT64)PointerValue, TableSize, > + &InstalledKey); [SAMI] See coding convention for spacing rules at https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/52_spacing > + table_offset += ACPI_ENTRY_SIZE; > + size -= ACPI_ENTRY_SIZE; > + } [SAMI] Can you look at simplifying the code in this function, please? You could refer to https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c#L131..L139 > + > + return Status; > +} > + > +EFI_STATUS > +EFIAPI > +CloudHvAcpiPlatformEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + > + Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ()); > + return Status; > +} > + > diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c > new file mode 100644 > index 000000000000..ae07c91f5705 > --- /dev/null > +++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c > @@ -0,0 +1,69 @@ > +/** @file > + Decide whether the firmware should expose an ACPI- and/or a Device Tree-based > + hardware description to the operating system. > + > + Copyright (c) 2017, Red Hat, Inc. > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +EFI_STATUS > +EFIAPI > +PlatformHasAcpiDt ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + > + // > + // If we fail to install any of the necessary protocols below, the OS will be > + // unbootable anyway (due to lacking hardware description), so tolerate no > + // errors here. > + // > + if (MAX_UINTN == MAX_UINT64 && > + !PcdGetBool (PcdForceNoAcpi)) > + { > + Status = gBS->InstallProtocolInterface ( > + &ImageHandle, > + &gEdkiiPlatformHasAcpiGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + goto Failed; > + } > + > + return Status; > + } > + > + // > + // Expose the Device Tree otherwise. > + // > + Status = gBS->InstallProtocolInterface ( > + &ImageHandle, > + &gEdkiiPlatformHasDeviceTreeGuid, > + EFI_NATIVE_INTERFACE, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + goto Failed; > + } > + > + return Status; > + > +Failed: > + ASSERT_EFI_ERROR (Status); > + CpuDeadLoop (); > + // > + // Keep compilers happy. > + // > + return Status; > +} --------------CB655411E2D5CDF4EF25407F Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

Hi Jianyon,

Thank you for this patch.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 17/05/2021 07:50 AM, Jianyong Wu wrote:
The current implementation of PlatformHasAcpiDt is not a common
library and is on behalf of qemu. So give a specific version for
Cloud Hypervisor here.

There is no device like Fw-cfg in qemu in Cloud Hypervisor, so a specific
Acpi handler is introduced here.

The handler implemented here is in a very simple way:
firstly, aquire the Rsdp address from the PCD varaible in the top
".dsc";
secondly, get the Xsdp address from Rsdp structure;
thirdly, get the Acpi tables following the Xsdp structrue and install it
one by one.

Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
---
 .../CloudHvAcpiPlatformDxe.inf                | 51 +++++++++++++
 .../CloudHvHasAcpiDtDxe.inf                   | 43 +++++++++++
 .../CloudHvAcpiPlatformDxe/CloudHvAcpi.c      | 73 +++++++++++++++++++
 .../CloudHvHasAcpiDtDxe.c                     | 69 ++++++++++++++++++
 4 files changed, 236 insertions(+)
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
 create mode 100644 ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
 create mode 100644 ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
[SAMI] I think these should be split as 2 patches covering CloudHvAcpiPlatformDx/* and CloudHvPlatformHasAcpiDtDx/* 

diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
new file mode 100644
index 000000000000..63c74e84eb27
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpiPlatformDxe.inf
@@ -0,0 +1,51 @@
+## @file
+#  OVMF ACPI Platform Driver for Cloud Hypervisor
[SAMI] I think the reference to OVMF can be removed, right?
+#
+#  Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = ClhFwCfgAcpiPlatform
[SAMI] Can ClhFwCfgAcpiPlatform  be changed to CloudHvAcpiPlatform? 
+  FILE_GUID                      = 6c76e407-73f2-dc1c-938f-5d6c4691ea93
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = CloudHvAcpiPlatformEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
[SAMI] Minor. The above line is just a comment, but can you revisit this, please?
+#
+
+[Sources]
+  CloudHvAcpi.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  MemoryAllocationLib
+  OrderedCollectionLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
+  gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
[SAMI] gEfiPciIoProtocolGuidis not used in this module.
+
+[Guids]
+  gRootBridgesConnectedEventGroupGuid
[SAMI] gRootBridgesConnectedEventGroupGuid not used in this module. 
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
[SAMI] PcdPciDisableBusEnumerationis not used in this module.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiRsdpBaseAddress
+
+[Depex]
+  gEfiAcpiTableProtocolGuid
diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
new file mode 100644
index 000000000000..f511a4f5064c
--- /dev/null
+++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.inf
@@ -0,0 +1,43 @@
+## @file
+# Decide whether the firmware should expose an ACPI- and/or a Device Tree-based
+# hardware description to the operating system.
+#
+# Copyright (c) 2017, Red Hat, Inc.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 1.25
+  BASE_NAME                      = ClhPlatformHasAcpiDtDxe
[SAMI] Is it possible to change the Clh prefix to CloudHv. Same comment for other places in this patch series. 
+  FILE_GUID                      = 71fe72f9-6dc1-199d-5054-13b4200ee88d
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = PlatformHasAcpiDt
+
+[Sources]
+  CloudHvHasAcpiDtDxe.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[Guids]
+  gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
+  gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+
+[Depex]
+  gEfiVariableArchProtocolGuid
diff --git a/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
new file mode 100644
index 000000000000..c2344e7b29fa
--- /dev/null
+++ b/ArmVirtPkg/CloudHvAcpiPlatformDxe/CloudHvAcpi.c
@@ -0,0 +1,73 @@
[SAMI] File license header is missing.
+#include <Library/BaseLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <IndustryStandard/Acpi63.h>
+#include <Protocol/AcpiTable.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/DebugLib.h>
+
+#define ACPI_ENTRY_SIZE 8
+#define XSDT_LEN 36
+
+STATIC
+EFI_ACPI_TABLE_PROTOCOL *
+FindAcpiTableProtocol (
+  VOID
+  )
[SAMI] Please add doxygen header for function. Same comment for other functions introduced in this series.
+{
+  EFI_STATUS              Status;
+  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
+
+  Status = gBS->LocateProtocol (
+                  &gEfiAcpiTableProtocolGuid,
+                  NULL,
+                  (VOID**)&AcpiTable
+                  );
+  ASSERT_EFI_ERROR (Status);
+  return AcpiTable;
+}
+
+EFI_STATUS
+EFIAPI
+InstallCloudHvAcpiTables (
+ IN     EFI_ACPI_TABLE_PROTOCOL       *AcpiProtocol
+ )
+{
+  UINTN InstalledKey, TableSize;
+  UINT64 Rsdp, Xsdt, table_offset, PointerValue;
[SAMI] Please have a look at variable naming conventions in EDKII coding standard at https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/4_naming_conventions 
+  EFI_STATUS Status = 0;
[SAMI] Status = EFI_SUCCESS.
+  int size;
[SAMI] UINTN should be used instead of 'int'. Also variable name does not confirm to coding standard.
+
+  Rsdp = PcdGet64 (PcdAcpiRsdpBaseAddress);
+  Xsdt = ((EFI_ACPI_6_3_ROOT_SYSTEM_DESCRIPTION_POINTER *)Rsdp)->XsdtAddress;
+  PointerValue = Xsdt;
+  table_offset = Xsdt;
+  size = ((EFI_ACPI_COMMON_HEADER *)PointerValue)->Length - XSDT_LEN;
+  table_offset += XSDT_LEN;
[SAMI] I think the macro name XSDT_LEN is a bit misleading. 'sizeof (EFI_ACPI_DESCRIPTION_HEADER)' should be used instead of XSDT_LEN. 
+
+  while(!Status && size > 0) {
[SAMI] Status is not boolean, use 'EFI_ERROR (Status)' instead. Also use paranthesis to add clarity.
 
+    PointerValue = *(UINT64 *)table_offset;
+    TableSize = ((EFI_ACPI_COMMON_HEADER *)PointerValue)->Length;
+    Status = AcpiProtocol->InstallAcpiTable (AcpiProtocol,
+             (VOID *)(UINT64)PointerValue, TableSize,
+             &InstalledKey);
[SAMI] See coding convention for spacing rules at https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/52_spacing
+    table_offset += ACPI_ENTRY_SIZE;
+    size -= ACPI_ENTRY_SIZE;
+  }
[SAMI] Can you look at simplifying the code in this function, please? You could refer to https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Xsdt/XsdtParser.c#L131..L139

+
+  return Status;
+}
+
+EFI_STATUS
+EFIAPI
+CloudHvAcpiPlatformEntryPoint (
+  IN EFI_HANDLE         ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS                         Status;
+
+  Status = InstallCloudHvAcpiTables (FindAcpiTableProtocol ());
+  return Status;
+}
+
diff --git a/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
new file mode 100644
index 000000000000..ae07c91f5705
--- /dev/null
+++ b/ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/CloudHvHasAcpiDtDxe.c
@@ -0,0 +1,69 @@
+/** @file
+  Decide whether the firmware should expose an ACPI- and/or a Device Tree-based
+  hardware description to the operating system.
+
+  Copyright (c) 2017, Red Hat, Inc.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Guid/PlatformHasAcpi.h>
+#include <Guid/PlatformHasDeviceTree.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+EFI_STATUS
+EFIAPI
+PlatformHasAcpiDt (
+  IN EFI_HANDLE       ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_STATUS           Status;
+
+  //
+  // If we fail to install any of the necessary protocols below, the OS will be
+  // unbootable anyway (due to lacking hardware description), so tolerate no
+  // errors here.
+  //
+  if (MAX_UINTN == MAX_UINT64 &&
+      !PcdGetBool (PcdForceNoAcpi))
+  {
+    Status = gBS->InstallProtocolInterface (
+                    &ImageHandle,
+                    &gEdkiiPlatformHasAcpiGuid,
+                    EFI_NATIVE_INTERFACE,
+                    NULL
+                    );
+    if (EFI_ERROR (Status)) {
+      goto Failed;
+    }
+
+    return Status;
+  }
+
+  //
+  // Expose the Device Tree otherwise.
+  //
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gEdkiiPlatformHasDeviceTreeGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    goto Failed;
+  }
+
+  return Status;
+
+Failed:
+  ASSERT_EFI_ERROR (Status);
+  CpuDeadLoop ();
+  //
+  // Keep compilers happy.
+  //
+  return Status;
+}

--------------CB655411E2D5CDF4EF25407F--