From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (EUR05-DB8-obe.outbound.protection.outlook.com [40.107.20.82]) by mx.groups.io with SMTP id smtpd.web12.27226.1659973189420962556 for ; Mon, 08 Aug 2022 08:39:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=rio7Xrpq; spf=pass (domain: arm.com, ip: 40.107.20.82, mailfrom: sami.mujawar@arm.com) ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=lEgp3xfeB+LmSqrebuj+5I7mNUipPZwauGlEJJcvdLVXcuTetvpkXTwmPxooN47PdkN35ioZj0PKMJVjAFpJ2D7ikzO9iST8GXXmrpNxHuE2zbf5/2vNcPNV/f+izQA2zw6Zpm+O86Mt7TERIOI5hK5DPsuN+/mKw3KaGMU5N1340o8wMIV84pk9pACEzvvhU8Cd8N/bMH7MF0wk/ui+FTQD3/I/us0VnnVH1T35J/ZLddTw5NLf+nAFUcaKcoUbibctzTCe/BIxoIlDep/d6XYMuXPuLXMj5ThGr/RdKLjxTXKEldq3ASuleGaIjCBRCkvCpVh3s9ye0+9cBr2PqA== ARC-Message-Signature: i=2; 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=F2AXjaYE7GrdNJgsylpR9BodBcynnEq5+FR1dEpbF/Q=; b=PEdLo2otKN6F4awdSb3eksf8wwi9gt4lkDhavD/GVlHGfmrg92QIdPE9+8aULZwDst8kUWrLDxo/YZBXXOfoW3aLthc8h2T3/Atw+liu5cnblCaodv3nxuttvjcXcF1StQskLln+zDm65lrNJENUEk5jrYvRB0FRJO1Sj6Wf/EHlDipMcpo/03m0mv0O7IBPR5ulEVnZmDfQg//imdDaj23Tis2HFO26vJGbp37/EPFym31CT1YNXLEtk3j7ZOLwy/4Xx2YHyEC5U267HTlPMCKnkbI3ieqixmpFDGHId0XOKtGjor0zS8TrxS18pTTrr0mFIluIhqTM8vmr8GIz0g== ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=edk2.groups.io smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=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=F2AXjaYE7GrdNJgsylpR9BodBcynnEq5+FR1dEpbF/Q=; b=rio7Xrpq/qKpJP0G/VBMn3cUPlqd6Lp0zD3/wpP62bJc5LiQuCbUav1IssK6L27bDKxzqgzOTnGm9T5eBoM9RYUkFFD5GbJ/ty8pm7ZVmaPRoiMLwL8/iZTZApQUAJj4LbFKelbLkM38p7g6UNHce/hO2GsuqmhHpAXKhhLach4= Received: from DB6PR0601CA0009.eurprd06.prod.outlook.com (2603:10a6:4:7b::19) by DBBPR08MB4521.eurprd08.prod.outlook.com (2603:10a6:10:cd::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.17; Mon, 8 Aug 2022 15:39:40 +0000 Received: from DBAEUR03FT036.eop-EUR03.prod.protection.outlook.com (2603:10a6:4:7b:cafe::b0) by DB6PR0601CA0009.outlook.office365.com (2603:10a6:4:7b::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.14 via Frontend Transport; Mon, 8 Aug 2022 15:39:40 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;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; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DBAEUR03FT036.mail.protection.outlook.com (100.127.142.193) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.16 via Frontend Transport; Mon, 8 Aug 2022 15:39:39 +0000 Received: ("Tessian outbound 73dd6a25223d:v123"); Mon, 08 Aug 2022 15:39:39 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 096e701104344ef1 X-CR-MTA-TID: 64aa7808 Received: from bf5be43ad00d.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id E264D358-2DFC-4406-B7CE-E60B452A3719.1; Mon, 08 Aug 2022 15:39:28 +0000 Received: from EUR01-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id bf5be43ad00d.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Mon, 08 Aug 2022 15:39:28 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CD3q6wfAhCy8fYe/+XUXprX98V8T7HI80Ynkdkzdb4aS+6VaLFojgX7szllXeq/fAJmzqYuKq3zYJY06U5X86fdWTuKykHDLAzm1QN3ZNVJU6KHGaxO1S2NzM+DmIxB7tvx9IstaE9Pl1zbSMW3dmuzdHY46nW0WgjC58hlcLaqny/g8++TCXJz03VkB+FXlocvigXY79P2LXVjLWSCQYrDu7Z0mIQN8GD2GYXG6s/ejPV+fyM2HZ0w4HLZVXo62DZr0E12hvcHAUfPKZrPXTOXeUN95ETsd9EkZjthffulQk33JUhVEOhz8aY6g8rmO0J0dSqNtOLUnUSRWlg9UKA== 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=F2AXjaYE7GrdNJgsylpR9BodBcynnEq5+FR1dEpbF/Q=; b=MpmD2gCWs79eNJ3Z1nWTLyvVyqo8n6OHhbLIISXkRzvwUsqOLSzCRGQtnXObbxiXA8NoaOoyCKitzMWQHgod9Q9JBAiq2RhnLFSPeyrYFmN4n1a2M7AliRku1pGRsdjX1Cj/cz8Ia3p4cylJk4HYMBiPfVtLToUaqZmvY8i5efrKd7O1JUt852UpYSDpAmWga8djRFQ/1LgPOB0aQ0klevle5k8VohSDWMUzN5jnvzE2rSt8H7Xni2v7e7vyKc4h7qyMPCfeYk9k35LY9hzHKTyX0K0Ou4l5AOW5beCXT95P+98YN+mO2J2pwlbfvgV0/PgbL7BzTeJshZJvqaMxiA== 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=F2AXjaYE7GrdNJgsylpR9BodBcynnEq5+FR1dEpbF/Q=; b=rio7Xrpq/qKpJP0G/VBMn3cUPlqd6Lp0zD3/wpP62bJc5LiQuCbUav1IssK6L27bDKxzqgzOTnGm9T5eBoM9RYUkFFD5GbJ/ty8pm7ZVmaPRoiMLwL8/iZTZApQUAJj4LbFKelbLkM38p7g6UNHce/hO2GsuqmhHpAXKhhLach4= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by HE1PR0802MB2156.eurprd08.prod.outlook.com (2603:10a6:3:c0::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.16; Mon, 8 Aug 2022 15:39:26 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::d562:5a52:f638:7fe9]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::d562:5a52:f638:7fe9%4]) with mapi id 15.20.5504.020; Mon, 8 Aug 2022 15:39:25 +0000 Message-ID: Date: Mon, 8 Aug 2022 16:39:24 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [edk2-devel] [PATCH v3 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space From: "Sami Mujawar" To: devel@edk2.groups.io, kuqin12@gmail.com Cc: Joe Lopez , Pierre Gondois , "nd@arm.com" References: <20220731053727.536-1-kuqin12@gmail.com> <20220731053727.536-6-kuqin12@gmail.com> <7b2fb1d3-6acf-9cc8-025d-2ef598976df4@arm.com> In-Reply-To: <7b2fb1d3-6acf-9cc8-025d-2ef598976df4@arm.com> X-ClientProxiedBy: LO2P265CA0060.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:60::24) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: f34df482-3433-48c2-fad3-08da795434fa X-MS-TrafficTypeDiagnostic: HE1PR0802MB2156:EE_|DBAEUR03FT036:EE_|DBBPR08MB4521:EE_ x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: +BEwRMJQAG1ZM1XVMEOX+1W9vJs1ZWfLHwUMpx5CSZv5CZwJKoNJjgYxOTJ1d6TQn84bbhjAUHhWjR8Qxs6Q8sE9jRa+kFGeLQkEuyr0/NDUoHG7b2faDOZuSyYgsVDVMTpq08R9GdepBtjo39TOaY+TE+xJy1YxkiiLToS7Xi0EgCWeHRiK10GhmvUdGxyq6T0qkNDs2/NVq0kf558COytFM8Jhm/ZUQ73Rgg9GfT2vdktIOaw4yOrPM6rOhx5wVZNZ728joPln6EWdgzAyYmuuq7DkQE8BsXLxWFeOMNzh2VE820XSDBOC3FYyKxLXAo5x2t4YqQF17aQf4+8RB2Hut7rzCq6HQA+Xp4YKii2oF4jMVkVeP+TQ5tmIWFqaAXE3tLBJ1xHY+i+83HQPkZCsZ/FB/NZCygvMUlI5r1O+L7YqIBejJR11XqtRMvIp5tyAVo3S52xsgYpmwXE5boInR4eHUFCdOrBGptPYgLJh7D8sg7AlZ8CuUCHnqVoRKgD9H4f+4LidnpgHpUKefE6rj7a3sfi2j5cCqrJtQ3EUgEcwclr0gnF+V97/dF0TDma9Qeruz7sp/ciIm4jZe3F2VfspS5EV3j94ViNLm2SykgbjpktriIBtsvyF+5/GhjP/P3DMhpohld99NfWmVGRFiXVjW2kkMasSCAxlhju2HaT7wsjXaQXSZzsMNtLAi+xoDeJDLv3K0qzkkSUkuM5CzII77JynLr+p3LS+rIKeNZ0UQ1/YRnskUQ+gvGWYVxm+2PMB65pw2dvImKfrk5n5LdrEJgU4UeUPgCS5ymL757eBRlR8mOScfqmYFcv8uPC/MVTvwWF19HOnVWTFWwQjlY+IR44iGozb3UKu4/BDLT7cG1Q79xXK144Ox1h4 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:(13230016)(4636009)(136003)(366004)(376002)(346002)(39860400002)(396003)(66556008)(36756003)(54906003)(966005)(31686004)(66946007)(66476007)(4326008)(8676002)(5660300002)(86362001)(45080400002)(31696002)(6486002)(8936002)(478600001)(316002)(38100700002)(44832011)(41300700001)(33964004)(53546011)(6506007)(2906002)(30864003)(83380400001)(26005)(166002)(2616005)(6512007)(186003)(213903007)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0802MB2156 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: DBAEUR03FT036.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 01538f95-c134-41c1-6dd1-08da79542c4c X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Vyosvr6Zj4sUX7RR7GlCiYvDkELhw+UeZJURNQbLCY81jCQZUsRKK5/viTAY0KAgZ4QuLXPA1HoDWuL23kxhqW+cYv1XB8Dewx6Q4qqlnbLQRhxQw9XUi1Blr7TOFyA1w8pcsdqFVj8sB/W0bVWuGQ2KwEpTVqPNqINsI077IghLIl7KjspkF5VFX8ZyA4o5O/Jg3bSCmKSqopEAcrFYnPO2mLnmWOVb8jTWq7X5v1pLpICyB+DL9kIn6hkqQD/zKBiTudryOYC2IkrVCziy/fIYdLWf4qdYX72F7BMV/hrhSNA5DF6K91hzH0wn/8BYoe7sBwbQWVmVJbbCbWsLvALEDF8yZxaLmEMNMhxlI0eHVfxFlPiEhvjy2pUkAc0hm9pzLLwX/YjcIhw5ErDrw3AZjupVt/0E0RiNXscRfX0VzyGdqkoiWsXmE83/JTiwW5mhJ6pActIEW/b4LkcQ765Z8E85C1xZ/I01un+X7zqiHtwXQ/OtwO5dtSdI1W+tw1njPlbFTUJGKta8ainF14WGhArWa9oETV/gTYdalMb4jLoKjuroDuoHeadNNuHxEFHvUKex6H4CU0gzbr3vpRilu7oMeDhMpbChqBrd55bot2cN3P1cgccI32Ce8i7C0eJomecaJwYIByr0h4ld/ahic1M6UoeiSauNBLHaAXndkznj8QvAqTm+X+YVGzIpws2oKwTYdHjxHv5Ov3L+8kjOwg1dlRqhk96p7RbP5IFnNuGwijir6CzDlYEUPCugNqZvvBaxBVXjYDNfUdhaLSrs8PEI1NGJxgICWOtf5wg04jnLj5UrgPEbbL/H24B9L3d0T993fcwf7CbnYnP7i3QtSgrN4cBb5HUa7ahFDy1ceeO9W/waLV+VeRJsYopRh/DcPjZNVF3VexT4QCpfww== 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:(13230016)(4636009)(346002)(396003)(376002)(136003)(39860400002)(36840700001)(46966006)(40470700004)(31686004)(83380400001)(2616005)(26005)(6512007)(336012)(186003)(36756003)(4326008)(316002)(86362001)(166002)(966005)(33964004)(41300700001)(53546011)(2906002)(6506007)(40460700003)(47076005)(478600001)(36860700001)(6486002)(44832011)(70206006)(30864003)(82740400003)(40480700001)(5660300002)(31696002)(45080400002)(54906003)(70586007)(8676002)(81166007)(356005)(82310400005)(8936002)(213903007)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2022 15:39:39.9267 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: f34df482-3433-48c2-fad3-08da795434fa 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: DBAEUR03FT036.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR08MB4521 Content-Type: multipart/alternative; boundary="------------eCk8kl5DDrcLxGh0RKp2mIjU" --------------eCk8kl5DDrcLxGh0RKp2mIjU Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Kun, I have just tried testing your patch with Kvmtool guest firmware and think this patch may need some modifications. Also, the patch 4/6 may need some adjustment, which I will reply back on that patch separately. Regards, Sami Mujawar On 08/08/2022 02:22 pm, Sami Mujawar wrote: > Hi Kun, > > Thank you for this patch. > > These changes look good to me. > > Reviewed-by: Sami Mujawar > > Regards, > > Sami Mujawar > > On 31/07/2022 06:37 am, Kun Qin via groups.io wrote: >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998 >> >> Certain OSes will complain if the ECAM config space is not reserved in >> the ACPI namespace. >> >> This change adds a function to reserve PNP motherboard resources for a >> given PCI node. >> >> Co-authored-by: Joe Lopez >> Signed-off-by: Kun Qin >> Reviewed-by: Pierre Gondois >> --- >> >> Notes: >>      v2: >>      - Only create RES0 after config space checking [Pierre] >>           v3: >>      - Updated function names and descriptions [Pierre] >>      - Moved translation calculation to CONFIG case [Pierre] >> >> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c >> | 171 ++++++++++++++++++++ >>   1 file changed, 171 insertions(+) >> >> diff --git >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c >> >> index ceffe2838c03..658a089c8f1f 100644 >> --- >> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c >> +++ >> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c >> @@ -616,6 +616,169 @@ GeneratePciCrs ( >>     return Status; >> >>   } >> >> >> +/** Generate a RES0 device node to reserve PNP motherboard resources >> >> +  for a given PCI node. >> >> + >> >> +  @param [in]   PciNode       Parent PCI node handle of the generated >> >> +                              resource object. >> >> +  @param [out]  CrsNode       CRS node of the AML tree to populate. >> >> + >> >> +  @retval EFI_SUCCESS             The function completed successfully. >> >> +  @retval EFI_INVALID_PARAMETER   Invalid input parameter. >> >> +  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory. >> >> +**/ >> >> +STATIC >> >> +EFI_STATUS >> >> +EFIAPI >> >> +GenerateMotherboardDevice ( >> >> +  IN  AML_OBJECT_NODE_HANDLE  PciNode, >> >> +  OUT AML_OBJECT_NODE_HANDLE  *CrsNode >> >> +  ) >> >> +{ >> >> +  EFI_STATUS              Status; >> >> +  UINT32                  EisaId; >> >> +  AML_OBJECT_NODE_HANDLE  ResNode; >> >> + >> >> +  if (CrsNode == NULL) { >> >> +    ASSERT (0); >> >> +    return EFI_INVALID_PARAMETER; >> >> +  } >> >> + >> >> +  // ASL: Device (RES0) {} >> >> +  Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode); >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >> +  // ASL: Name (_HID, EISAID ("PNP0C02")) >> >> +  Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP >> Motherboard Resources */ >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >> +  Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL); >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >> +  // ASL: Name (_CRS, ResourceTemplate () {}) >> >> +  Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, CrsNode); >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >> +  return Status; >> >> +} >> >> + >> >> +/** Reserves ECAM space for PCI config space >> >> + >> >> +  @param [in]       Generator       The SSDT Pci generator. >> >> +  @param [in]       CfgMgrProtocol  Pointer to the Configuration >> Manager >> >> +                                    Protocol interface. >> >> +  @param [in]       PciInfo         Pci device information. >> >> +  @param [in, out]  PciNode         RootNode of the AML tree to >> populate. >> >> + >> >> +  @retval EFI_SUCCESS             The function completed successfully. >> >> +  @retval EFI_INVALID_PARAMETER   Invalid parameter. >> >> +  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory. >> >> +**/ >> >> +STATIC >> >> +EFI_STATUS >> >> +EFIAPI >> >> +ReserveEcamSpace ( >> >> +  IN            ACPI_PCI_GENERATOR *Generator, >> >> +  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST >> CfgMgrProtocol, >> >> +  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo, >> >> +  IN  OUT       AML_OBJECT_NODE_HANDLE PciNode >> >> +  ) >> >> +{ >> >> +  EFI_STATUS                   Status; >> >> +  AML_OBJECT_NODE_HANDLE       CrsNode; >> >> +  BOOLEAN                      Translation; >> >> +  UINT32                       Index; >> >> +  CM_ARM_OBJ_REF               *RefInfo; >> >> +  UINT32                       RefCount; >> >> +  CM_ARM_PCI_ADDRESS_MAP_INFO  *AddrMapInfo; >> >> +  BOOLEAN                      IsPosDecode; >> >> + >> >> +  // Get the array of CM_ARM_OBJ_REF referencing the >> >> +  // CM_ARM_PCI_ADDRESS_MAP_INFO objects. >> >> +  Status = GetEArmObjCmRef ( >> >> +             CfgMgrProtocol, >> >> +             PciInfo->AddressMapToken, >> >> +             &RefInfo, >> >> +             &RefCount >> >> +             ); >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >> +  for (Index = 0; Index < RefCount; Index++) { >> >> +    // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one. >> >> +    Status = GetEArmObjPciAddressMapInfo ( >> >> +               CfgMgrProtocol, >> >> +               RefInfo[Index].ReferenceToken, >> >> +               &AddrMapInfo, >> >> +               NULL >> >> +               ); >> >> +    if (EFI_ERROR (Status)) { >> >> +      ASSERT (0); >> >> +      return Status; >> >> +    } >> [SAMI] Sorry for missing this earlier in the review. However, the ECAM memory space is described byCM_ARM_PCI_CONFIG_SPACE_INFO. So, I think that needs to be used here. The CM_ARM_PCI_CONFIG_SPACE_INFO structure does not include the length of the configuration space and would probably need to be updated. Which platform are you testing these changes on? I would like to understand more about your use case. Is it possible to share some more details, please? [/SAMI] >> + >> >> +    switch (AddrMapInfo->SpaceCode) { >> >> +      case PCI_SS_CONFIG: >> >> +        Translation = (AddrMapInfo->CpuAddress != >> AddrMapInfo->PciAddress); >> >> +        if (AddrMapInfo->CpuAddress >= AddrMapInfo->PciAddress) { >> >> +          IsPosDecode = TRUE; >> >> +        } else { >> >> +          IsPosDecode = FALSE; >> >> +        } >> >> + >> >> +        Status = GenerateMotherboardDevice (PciNode, &CrsNode); >> >> +        if (EFI_ERROR (Status)) { >> >> +          ASSERT (0); >> >> +          break; >> >> +        } >> >> + >> >> +        Status = AmlCodeGenRdQWordMemory ( >> >> +                   FALSE, >> >> +                   IsPosDecode, >> >> +                   TRUE, >> >> +                   TRUE, >> >> +                   FALSE, // non-cacheable >> >> +                   TRUE, >> >> +                   0, >> >> +                   AddrMapInfo->PciAddress, >> >> +                   AddrMapInfo->PciAddress + >> AddrMapInfo->AddressSize - 1, >> >> +                   Translation ? AddrMapInfo->CpuAddress - >> AddrMapInfo->PciAddress : 0, >> >> +                   AddrMapInfo->AddressSize, >> >> +                   0, >> >> +                   NULL, >> >> +                   0, >> >> +                   TRUE, >> >> +                   CrsNode, >> >> +                   NULL >> >> +                   ); >> >> +        break; >> >> +      default: >> >> +        break; >> >> +    } // switch >> >> + >> >> +    if (EFI_ERROR (Status)) { >> >> +      ASSERT (0); >> >> +      return Status; >> >> +    } >> >> +  } >> >> + >> >> +  return Status; >> >> +} >> >> + >> >>   /** Generate a Pci device. >> >> >>     @param [in]       Generator       The SSDT Pci generator. >> >> @@ -702,9 +865,17 @@ GeneratePciDevice ( >>       return Status; >> >>     } >> >> >> +  // Add the PNP Motherboard Resources Device to reserve ECAM space >> >> +  Status = ReserveEcamSpace (Generator, CfgMgrProtocol, PciInfo, >> PciNode); >> >> +  if (EFI_ERROR (Status)) { >> >> +    ASSERT (0); >> >> +    return Status; >> >> +  } >> >> + >> >>     // Add the template _OSC method. >> >>     Status = AddOscMethod (PciInfo, PciNode); >> >>     ASSERT_EFI_ERROR (Status); >> >> + >> >>     return Status; >> >>   } >> >> --------------eCk8kl5DDrcLxGh0RKp2mIjU Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Kun,

I have just tried testing your patch with Kvmtool guest firmware and think this patch may need some modifications.

Also, the patch 4/6 may need some adjustment, which I will reply back on that patch separately.

Regards,

Sami Mujawar

On 08/08/2022 02:22 pm, Sami Mujawar wrote:
Hi Kun,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 31/07/2022 06:37 am, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998

Certain OSes will complain if the ECAM config space is not reserved in
the ACPI namespace.

This change adds a function to reserve PNP motherboard resources for a
given PCI node.

Co-authored-by: Joe Lopez <joelopez@microsoft.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---

Notes:
     v2:
     - Only create RES0 after config space checking [Pierre]
          v3:
     - Updated function names and descriptions [Pierre]
     - Moved translation calculation to CONFIG case [Pierre]

  DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c | 171 ++++++++++++++++++++
  1 file changed, 171 insertions(+)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index ceffe2838c03..658a089c8f1f 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -616,6 +616,169 @@ GeneratePciCrs (
    return Status;

  }

 
+/** Generate a RES0 device node to reserve PNP motherboard resources

+  for a given PCI node.

+

+  @param [in]   PciNode       Parent PCI node handle of the generated

+                              resource object.

+  @param [out]  CrsNode       CRS node of the AML tree to populate.

+

+  @retval EFI_SUCCESS             The function completed successfully.

+  @retval EFI_INVALID_PARAMETER   Invalid input parameter.

+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+GenerateMotherboardDevice (

+  IN  AML_OBJECT_NODE_HANDLE  PciNode,

+  OUT AML_OBJECT_NODE_HANDLE  *CrsNode

+  )

+{

+  EFI_STATUS              Status;

+  UINT32                  EisaId;

+  AML_OBJECT_NODE_HANDLE  ResNode;

+

+  if (CrsNode == NULL) {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  // ASL: Device (RES0) {}

+  Status = AmlCodeGenDevice ("RES0", PciNode, &ResNode);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  // ASL: Name (_HID, EISAID ("PNP0C02"))

+  Status = AmlGetEisaIdFromString ("PNP0C02", &EisaId); /* PNP Motherboard Resources */

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  // ASL: Name (_CRS, ResourceTemplate () {})

+  Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, CrsNode);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  return Status;

+}

+

+/** Reserves ECAM space for PCI config space

+

+  @param [in]       Generator       The SSDT Pci generator.

+  @param [in]       CfgMgrProtocol  Pointer to the Configuration Manager

+                                    Protocol interface.

+  @param [in]       PciInfo         Pci device information.

+  @param [in, out]  PciNode         RootNode of the AML tree to populate.

+

+  @retval EFI_SUCCESS             The function completed successfully.

+  @retval EFI_INVALID_PARAMETER   Invalid parameter.

+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.

+**/

+STATIC

+EFI_STATUS

+EFIAPI

+ReserveEcamSpace (

+  IN            ACPI_PCI_GENERATOR                            *Generator,

+  IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,

+  IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO                  *PciInfo,

+  IN  OUT       AML_OBJECT_NODE_HANDLE                        PciNode

+  )

+{

+  EFI_STATUS                   Status;

+  AML_OBJECT_NODE_HANDLE       CrsNode;

+  BOOLEAN                      Translation;

+  UINT32                       Index;

+  CM_ARM_OBJ_REF               *RefInfo;

+  UINT32                       RefCount;

+  CM_ARM_PCI_ADDRESS_MAP_INFO  *AddrMapInfo;

+  BOOLEAN                      IsPosDecode;

+

+  // Get the array of CM_ARM_OBJ_REF referencing the

+  // CM_ARM_PCI_ADDRESS_MAP_INFO objects.

+  Status = GetEArmObjCmRef (

+             CfgMgrProtocol,

+             PciInfo->AddressMapToken,

+             &RefInfo,

+             &RefCount

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  for (Index = 0; Index < RefCount; Index++) {

+    // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.

+    Status = GetEArmObjPciAddressMapInfo (

+               CfgMgrProtocol,

+               RefInfo[Index].ReferenceToken,

+               &AddrMapInfo,

+               NULL

+               );

+    if (EFI_ERROR (Status)) {

+      ASSERT (0);

+      return Status;

+    }

[SAMI] Sorry for missing this earlier in the review. However, the ECAM memory space is described by CM_ARM_PCI_CONFIG_SPACE_INFO. So, I think that needs to be used here.

The CM_ARM_PCI_CONFIG_SPACE_INFO structure does not include the length of the configuration space and would probably need to be updated.

Which platform are you testing these changes on? I would like to understand more about your use case. Is it possible to share some more details, please?

[/SAMI]

+

+    switch (AddrMapInfo->SpaceCode) {

+      case PCI_SS_CONFIG:

+        Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);

+        if (AddrMapInfo->CpuAddress >= AddrMapInfo->PciAddress) {

+          IsPosDecode = TRUE;

+        } else {

+          IsPosDecode = FALSE;

+        }

+

+        Status = GenerateMotherboardDevice (PciNode, &CrsNode);

+        if (EFI_ERROR (Status)) {

+          ASSERT (0);

+          break;

+        }

+

+        Status = AmlCodeGenRdQWordMemory (

+                   FALSE,

+                   IsPosDecode,

+                   TRUE,

+                   TRUE,

+                   FALSE, // non-cacheable

+                   TRUE,

+                   0,

+                   AddrMapInfo->PciAddress,

+                   AddrMapInfo->PciAddress + AddrMapInfo->AddressSize - 1,

+                   Translation ? AddrMapInfo->CpuAddress - AddrMapInfo->PciAddress : 0,

+                   AddrMapInfo->AddressSize,

+                   0,

+                   NULL,

+                   0,

+                   TRUE,

+                   CrsNode,

+                   NULL

+                   );

+        break;

+      default:

+        break;

+    } // switch

+

+    if (EFI_ERROR (Status)) {

+      ASSERT (0);

+      return Status;

+    }

+  }

+

+  return Status;

+}

+

  /** Generate a Pci device.

 
    @param [in]       Generator       The SSDT Pci generator.

@@ -702,9 +865,17 @@ GeneratePciDevice (
      return Status;

    }

 
+  // Add the PNP Motherboard Resources Device to reserve ECAM space

+  Status = ReserveEcamSpace (Generator, CfgMgrProtocol, PciInfo, PciNode);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

    // Add the template _OSC method.

    Status = AddOscMethod (PciInfo, PciNode);

    ASSERT_EFI_ERROR (Status);

+

    return Status;

  }

 
--------------eCk8kl5DDrcLxGh0RKp2mIjU--