From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 0AC44740038 for ; Thu, 7 Sep 2023 17:50:21 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=rrq1lUqy+wGbW5yW1QD6kPdkZ2h6/Nfmgzvlum1Oj7s=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:Message-ID:Date:User-Agent:Subject:To:References:From:In-Reply-To:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1694109020; v=1; b=f2eXztBPIPj2qJbN+bjxG3Xlg1TFGe8BiiFeSYldWFsVG2zEJyB5vMAs4lwkHxwpApnXmBSk 7PlaDgQL9/iBcht6lizPnXmiOBHK37ULWBTqXP6PhEfanrh5osaiMpNHj1AH6dXVMzBSobNYUR4 dvjcrD1WyiXp4PwStLD1gc0A= X-Received: by 127.0.0.2 with SMTP id LBfiYY7687511x2bAPuMULZ7; Thu, 07 Sep 2023 10:50:20 -0700 X-Received: from NAM04-BN8-obe.outbound.protection.outlook.com (NAM04-BN8-obe.outbound.protection.outlook.com [40.92.47.30]) by mx.groups.io with SMTP id smtpd.web10.19976.1694109019889739941 for ; Thu, 07 Sep 2023 10:50:20 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nirCoGNlv7UzSO01+aLaVlBk3fiRZcNmMdgm3ENCmKq7jIa8urmEUXMNfwYIX0ZAoemP73JQAptvewBEizQHoRS+vO/wWBnmbe4vPtz+vxGXRyFaQIrfuSUqnp9X2sboNAvDISjVTqpIQMsxcxwCwSgfqFGjVSJyOk/Uc3DXQzlYEkJwXX2H5i52DIRRznVi5SC417Xx5TnzEZm669LJFWVhubc18qOmHzune2C2FV2XynKsrq6R5wijX0jvyzIfL/A7W5fdUsxP3NKbQel13QRRlh+u05IbbNFyniitDdZ1tPTQoX1mHRfWNKaqphj71IIOCowvzM1HV6ouSGJU/Q== 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=IcMhG8bWDdgOOuLtSNOqJACP6V6IjEQBVrI0VrB2XJQ=; b=h1F8MBTVNHhFjI7ZDqpt0b6mcXu5ARSKmnN5UQY5+UNntrKlfqb2x4oEjF5AUTUZqMbH2a9HnwqFMcpa6EP6iOgKxUDtnHzaR4pj/IFuhnNiQhTqSnMSzwIiCzT26z6yd08EdwEA+5bKFE0mXeD0/dwKbdH9+jaciFxbLZkyFqukg6JUVmhneJyZ/KZOQLYnOcIcMxi3USTmmPx/PSuw9g1OYzMtKz7tMEQib0ulovWgyZ8ZDqdH+WIfrR9hicHD6U/p3eLu3MV9hN7rAV3C7E7AeVerfFtvX3nPoBJuQwb4W+lShj/SBHV36QM8Unm/rYO39jdEyY4pQN+MD8jEQw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none X-Received: from BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) by PH8PR19MB7141.namprd19.prod.outlook.com (2603:10b6:510:217::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6745.34; Thu, 7 Sep 2023 17:50:17 +0000 X-Received: from BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::f5ef:669c:fea1:a213]) by BY3PR19MB4900.namprd19.prod.outlook.com ([fe80::f5ef:669c:fea1:a213%7]) with mapi id 15.20.6768.029; Thu, 7 Sep 2023 17:50:17 +0000 Message-ID: Date: Thu, 7 Sep 2023 10:50:16 -0700 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Enable Early Serial For DxeCore To: devel@edk2.groups.io, osde@linux.microsoft.com, lersek@redhat.com, Ard Biesheuvel References: <27912.1694092220075253890@groups.io> <1e6f37af-f407-43f2-aa14-9c5de85eb404@linux.microsoft.com> From: "Sean" In-Reply-To: <1e6f37af-f407-43f2-aa14-9c5de85eb404@linux.microsoft.com> X-TMN: [3Q//4uB1I9CUAqdOR84L1RDGmzzvgrLuQhs4vwjg3re9n7qfA2/uja/Tgy72yl6g] X-ClientProxiedBy: MW4PR04CA0050.namprd04.prod.outlook.com (2603:10b6:303:6a::25) To BY3PR19MB4900.namprd19.prod.outlook.com (2603:10b6:a03:354::11) X-Microsoft-Original-Message-ID: <80785b06-6246-4026-8ef3-8ee8162367c4@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BY3PR19MB4900:EE_|PH8PR19MB7141:EE_ X-MS-Office365-Filtering-Correlation-Id: 522872dd-125b-487d-dee5-08dbafcae58a X-Microsoft-Antispam-Message-Info: m0qI0/+CzFbP8+80YaAH9vfhcL35BAcGdz3kn4dEL72/dbcLsXx/yATRnDWTzWetsEogWkyhbK3YtiVVXqOWsap0ZJoMtuOMx4KAeuZdXlJ/PoR6RnFymMKewe7o5W45+6zac1JKa66hTlotDGZaseXHgrRDYVw26t/D94xa6dCZQUajSP+JQpEGL3DWKRYrNlN3QxzGcjVI72FSKj6d+JXLWRbYTJaMytaOP0ZYod9Mu5M49xlpED8L7f2HPILBcN8qXemYu0RSW3NvWb91wFQeV+RY28vzVNQiXrn34eM+yBO862HqeBA3zclBb1KUifd5JcWgHz7XPa0BbOjQE11X6KptsOISU/AJ0TsNtd7s6EMGlmY0xwUX8xw676pFpHfg9cGBzFOgHAbiRd9k2lt7w6YuOHZ4ALrJeR3MPOJ+cHrxJ709qEle++EZHQQLm4aCf7D99WGs6nqonXvL23LpyQNj9GSe1jh6STOUu5PBIasTEgp99RwFmpSFdyWRaIn7Kyeaqnq8M6SDwkVEi4EUbdKS7He5lK1Ui7VGdIc= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?brdF7+1lIdu5CgDFQlJv6EoWS/b2P4ZWSiKkdCgmehfaDZEBjdSgvJrwYRtM?= =?us-ascii?Q?DW5yeuyHEJDnxvzZYKcVwf1AqCp3IB2qo/SeP0rLFRz7/1k5VtJ1LT6t3WhM?= =?us-ascii?Q?snZKBJYjcoscQFS25R7EwWCeDTa2HzA7K6ImZjlk4Qw3VL4fGhERs5UUZw/W?= =?us-ascii?Q?d5+/Rgx2OpnIbb70zl4KSoss/Bxv1oX7qdBe5efxVno+nvgVBh1lzL23q4jb?= =?us-ascii?Q?9g+x4cBQWL6/h2dCLepcz+pf5wfsy/Yc93JDZjF4WKaIVaJ68Q3jc/zyse1r?= =?us-ascii?Q?YjKGlmTIMB6IVdX0eyy0CQLo7CQrWXqhxQbEafqA8mD56vsYu8yo+B2o34gZ?= =?us-ascii?Q?AaJhmt411i34RJB1ymOIEQZTuKzB9cDEQ54ar3Ni+Bu6Puhig7jc0nGz15jv?= =?us-ascii?Q?rBoGZfrtsA145TLeWszVq+8aZj/U7XWDa8bqYgOrrNZ9iiyQUI8qC/AF853I?= =?us-ascii?Q?5CNiRYFLeIYbwONEvy9TQrgx1Zs0eeJghKnanBzldIcI9PcNf/A8bSnICqZn?= =?us-ascii?Q?p5/mJUZ8oZpuy130FTEuwj9f7M+scThTyg0HpBisoMNxDx4tuuXfEUae+TCv?= =?us-ascii?Q?JWxGHQPF2RBXpnx/ganKs7CsXe0mid1+e8dFS63Atrr2VEnmJ+4nd2asXkuz?= =?us-ascii?Q?dJnAed4jzqGsZtZ7DWGunz+FUmWaCA5ERI3Uz50rU6f31JFJyidOq9WlPd6V?= =?us-ascii?Q?i/fj363HmVpiJzpWvtWx0cw0O03HuqZVjAsPQz+5wXAY25dDM6MHKcc4B0mP?= =?us-ascii?Q?XAE+oP+TGrhpgSkBYQHsQ1rh7X02lpepMYxGYQQJ7Unt8eU1tTy5AsZj1+hH?= =?us-ascii?Q?zorusAM8f0tQPRdHq5iAN3JmtBx1VKtNNYbIIaPlTe2YTh2lAWMqu4Gp7S4i?= =?us-ascii?Q?pNg+pNh/EOFGFKM7EQqZYOU+mM2aSUIJUnrhLB3YuETgEsaYVhwhgdZD++IT?= =?us-ascii?Q?12aWdT4fgqTPYObwaKYvpFYhKdXVxoiWdN9w3JA5wEyTs0bBFgpoSPcol+iP?= =?us-ascii?Q?ahSBWj737pRr5A8Lrj1SsyuYxBoHy73MRWYiTcYJ3wSH+mPdd6zT9fRHpLqQ?= =?us-ascii?Q?d6WzidciYN01SucRM52DhA0399b4nrlqSw9dS5Qk8tyqIAs+aeN/KM/8Pl9a?= =?us-ascii?Q?EnXdcqsHNYjC5+piCg9ov2cgRXwCXma5fp3iW11pZRdAtkA7nnn4sixk21iU?= =?us-ascii?Q?bXddue7DKWnlAqq3WzGHtfZFWJuBxp4ErrBfT25iry6xdx8wCDEVBQQUmvYz?= =?us-ascii?Q?7AxjqNC+PtFSaLxuevmRLdWDT8x3OuIxU2Ylbrsong=3D=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 522872dd-125b-487d-dee5-08dbafcae58a X-MS-Exchange-CrossTenant-AuthSource: BY3PR19MB4900.namprd19.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Sep 2023 17:50:17.7644 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR19MB7141 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,spbrogan@outlook.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: Rz3LUTwRIMFeJSaK78PSPbiDx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=f2eXztBP; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=outlook.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}") I would argue that by declaring that your library class supports type=20 DXE_CORE (or any core type) that you have declared you understand the=20 uniqueness of the environment and have accounted for it. For instances that support DXE_CORE or MM_CORE module types we often use=20 a global variable (to the library) to determine if the init routine has=20 been completed.=C2=A0 This does require a single byte check on each serial= =20 message write (hot path) but given all the code on that path this is not=20 noticeable in performance measurements.=C2=A0=C2=A0 In the case below this = pattern=20 could be used by the FdtPL011SerialPortLib to detect if it's init=20 routine has been called. Thanks Sean On 9/7/2023 8:24 AM, Oliver Smith-Denny wrote: > On 9/7/2023 6:10 AM, Laszlo Ersek wrote: >> (replying on the webui... sorry!) >> >> The problem is actually embedded in MdePkg and MdeModulePkg. >> >> - In DxeMain() (and in functions called by DxeMain()), we call=20 >> DebugLib APIs *before* reaching ProcessLibraryConstructorList(). >> >> - In ArmVirtQemu, we resolve the DXE Core's DebugLib dependency to=20 >> BaseDebugLibSerialPort (from MdePkg). >> >> - BaseDebugLibSerialPort has a constructor function=20 >> (BaseDebugLibSerialPortConstructor()). >> >> These already suffice for broken DebugLib behavior. APIs of a library=20 >> class should not be called because the library instance has a chance=20 >> to initialize. >> >> The rest is circumstantial. Like, BaseDebugLibSerialPortConstructor=20 >> calls SerialPortInitialize, but our SerialPortInitialize (in=20 >> FdtPL011SerialPortLib) does nothing. Well, the latter doesn't need to=20 >> do anything, because FdtPL011SerialPortLib has its own constructor=20 >> (FdtPL011SerialPortLibInitialize), thus, if constructors were called=20 >> properly, then BaseDebugLibSerialPort + FdtPL011SerialPortLib would=20 >> work properly together, regardless of SerialPortInitialize being=20 >> empty in the latter. >> >> Basically the DXE Core has a hidden requirement -- it can only use=20 >> such DebugLib instances that need no explicit initialization. >> >> The proposed patch works around the problem by satisfying that hidden=20 >> requirement one level lower down: in the SerialPortLib instance. The=20 >> initialization of BaseDebugLibSerialPort is still busted (its=20 >> constructor is not called, so it cannot call SerialPortInitialize=20 >> either), but now it is masked, because EarlyFdtPL011SerialPortLib=20 >> works withouth *both* SerialPortInitialize and construction. >> >> The real fix would be to make the DXE Core requirement explicit, by=20 >> introducing separate (dedicated) DebugLib and SerialPortLib *classes*=20 >> (whose APIs are guaranteed to work without initialization). >> >> Laszlo > > Thanks for the comprehensive breakdown! :). I completely agree that > fixing this at the upper level (and ideally documenting this > requirement) is the better move. > > I can drop this patch and take a crack at that. I'm in the last few > weeks leading up to an extended parental leave, so we'll see if I can > squeeze it in prior to then :). > > Oliver > > >=20 > > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108408): https://edk2.groups.io/g/devel/message/108408 Mute This Topic: https://groups.io/mt/101203427/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-