From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 91281AC0104 for ; Thu, 11 Jul 2024 01:59:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=bD5TlnXXM1e7s6VGGhJUcwNJFcXh1u7JdCjHO3dpb28=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:User-Agent:Subject:To:CC:References:From:In-Reply-To:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1720663143; v=1; b=nVkqdwOBpMNf9GbAveyU1RkPzXb6NDx5GC5dS/wp0A7dBIQvT0X0/N5swOSiOh9YERdSmcfI RG/zRflCBOeL6GEZP7T6tmWVmiGEyj5QL7KPJTi4Djy+FPH3CIuji1CQM8EXq4j0Ah1CLaOM+h5 Edruivxbn0gXBLPUHYFeFoF5F5f+T9WExpIPMWc/QZAnB6votF2Zb8S0LqmhAl3cfP/ZgZhYFWM QRn3PRSIEFm42MJHnu5MfV9W8jysjxr04gw/kZBO2/qefDMZm5zNxRtZTN0Qc+F6eNflClZYaiJ XBIStKT7V6MU20PfrXxuh8V9mB+mT5y/VSNOz50Et0KaQ== X-Received: by 127.0.0.2 with SMTP id VDjAYY7687511xwmPrqJiOxj; Wed, 10 Jul 2024 18:59:02 -0700 X-Received: from DM1PR04CU001.outbound.protection.outlook.com (DM1PR04CU001.outbound.protection.outlook.com [52.101.61.85]) by mx.groups.io with SMTP id smtpd.web10.2758.1720663141379819799 for ; Wed, 10 Jul 2024 18:59:01 -0700 X-Received: from PH0PR01MB7287.prod.exchangelabs.com (2603:10b6:510:10a::21) by BY1PR01MB9065.prod.exchangelabs.com (2603:10b6:a03:5af::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7741.35; Thu, 11 Jul 2024 01:58:58 +0000 X-Received: from PH0PR01MB7287.prod.exchangelabs.com ([fe80::fc79:e629:93aa:8b8f]) by PH0PR01MB7287.prod.exchangelabs.com ([fe80::fc79:e629:93aa:8b8f%4]) with mapi id 15.20.7741.033; Thu, 11 Jul 2024 01:58:58 +0000 Message-ID: <7c34ac8f-8097-42a3-b996-49a18f3e934f@os.amperecomputing.com> Date: Thu, 11 Jul 2024 08:58:49 +0700 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2] ManageabilityPkg: add support for the phosphor ipmi blob transfer protocol To: Nickle Wang , "devel@edk2.groups.io" CC: Abner Chang , Abdul Lateef Attar , Tinh Nguyen , Thang Nguyen OS , Mike Maslenkin References: <20240515150629.236739-1-nicklew@nvidia.com> <7d928ca7-e312-4a2e-9470-cbd96fbdc643@os.amperecomputing.com> From: "Nhi Pham via groups.io" In-Reply-To: X-ClientProxiedBy: SG2PR04CA0159.apcprd04.prod.outlook.com (2603:1096:4::21) To PH0PR01MB7287.prod.exchangelabs.com (2603:10b6:510:10a::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: PH0PR01MB7287:EE_|BY1PR01MB9065:EE_ X-MS-Office365-Filtering-Correlation-Id: 9b9f9d20-e8f9-4b02-4791-08dca14d06f5 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?hO+dGkyeLLxkaLmVg0CwL7lNm6W2nGWkvVMu0Lf9K5qkZZMg7YJK/hmR/Wku?= =?us-ascii?Q?1sCrMtYQTFrtP7hcowgg38ywvPjrwDFRR9BQmZIPXKOTz1MYgfv/2gwISEow?= =?us-ascii?Q?XQOKvzWRpYhD1NfRVNShc+u82/cW+1E1ZIZWYnW+Q4hIHY8we8wuZGlBPQM6?= =?us-ascii?Q?S5stEjraZQKUXd44MfIPwDC0o70snFFLzxQOwnGuH/QE4YSelEGHZhS1vD/A?= =?us-ascii?Q?eC30LW/uzO5tktJF5aqAN3ekBaTH9FQ+v75xLCwcE2CMDzap09qCebbO+zaJ?= =?us-ascii?Q?QJb8bBWbo5eScaXPe4f9xNuA7NK6B8ErW/VE3OGqdujM0Wwx3VEe/CX5vyMZ?= =?us-ascii?Q?IMyASD1Smdb+2mnfhmPWRoTbU3slVfhQKfpHzQVhi/zLgB6JNm0/lhuev1ce?= =?us-ascii?Q?nVflkvCui3gjeg1EDkUfgbsZJ6+EJlwiukRwCYEZ4zQH6wZdV+0sUc6BKgVt?= =?us-ascii?Q?LjUjdoaMHiQHbNJToA5rWSUD4gGjLAYr6aimM2X0Ml4k5/cx0jDo/+pv7Ibc?= =?us-ascii?Q?Hx31N28DOJ6EBgXt4lESOva+IDVihMNQY+d3tnLG+eWdl0vKKbfX/vlMrsig?= =?us-ascii?Q?hNvpyQ43+zmeOlfYVga19dq3uTBTu2ynW/8Jw9KwndgmvhiZ3AJ4UmjOe+oE?= =?us-ascii?Q?sQwfwTOlw5GpLSjYr852HNmIT0zbQTdcAAGjKvLMLFtWCxuZXvcxY3pulxeD?= =?us-ascii?Q?Kutv+7AJS9DTCWgY6wzonZ2mZPfPlyrzy33FPyTvpFxqz7Vmn/hseP2Xg/ZM?= =?us-ascii?Q?qe/ExL8Oc5q31TLNxu+B13YYBjpRtI9RbNkduKvmcSPUbmG1PxKrXHnfxPk+?= =?us-ascii?Q?r9JM+ZZWRrh5wLiMlAXpKklc8C10uUAAuZ5EtnQ3kKVtTxe0141EtOsbOW7i?= =?us-ascii?Q?2qkckdrz3FtOb71gB6qqcEu3vK49oF6hJzDuplMC4zCehiVj1UL5sQGkdkvS?= =?us-ascii?Q?2+kcFQI0cjX8+FAOW6ZD4HRds0Bg1v0l8AmolnOJO6mDZzod0XEIKbDNZjdg?= =?us-ascii?Q?SQli8Nzg7OWo3Nban4YwkO5sZG8Qn6Xc5wYhU4zD0asMxBZ8ao4s5mgtwHPQ?= =?us-ascii?Q?lEra6raW4B0JXTlmIt1QydhQI5KQ+YVJc+EaTJZUs3cpyKyJTYIpfC7/tOEO?= =?us-ascii?Q?bSiL+TO9GsMUuWrmAS7UCD9aT/akCsVgt9M7GRNo945xFxqX9BwTm8LFDm4y?= =?us-ascii?Q?/beEGNMn/FMMu1KGoIVMZ6+97TzpyylB8904FeOerD3/aVowV+0jkjIuCPJK?= =?us-ascii?Q?eiul9UxKZdVMtWnktnDFKqX4Yz9ojk0XQ/SR997R8P8bJVwwHJQus6+vxKxY?= =?us-ascii?Q?ZcA=3D?= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?eNfeosotD4AF7BmMdES7qB2DvqpOd5I3l7RWzgFoyxgA4d1ZSrceEE8ywAjS?= =?us-ascii?Q?5oeZnX0sP2u53doRds1nwQCH53KBqkxm4cQEzs/BLracLg7PylhUlEMGOJj2?= =?us-ascii?Q?8rWy2d2nusvA9tyu+NZHtjJIjIXJulBS0TUizr2JQOr5PF2S3H5cy4X61Qdf?= =?us-ascii?Q?Sdl6NgM4ct+cdS4i8w9E0NNG2mhAn2sa2XoOdpuo1KtB/BNBCuwLoig9Jgla?= =?us-ascii?Q?BKP4RcEAabO8reh6MHMeifO0gsPiIDUZkZY0ty1Zr83RzDdcFZ8wHq6ULhyo?= =?us-ascii?Q?yWKcKAruGOVVjl4hd+lzZjEHnKFut2CP0VuEHoyXzlHQt6msySLJ83Lg7FA4?= =?us-ascii?Q?aC3kAn95vliuCTULChjWq484E6IAXZrP0uNIdQJkQMnPDN/V0ZDokv8eyqbx?= =?us-ascii?Q?DFsbEIGXDZQN5cVH8SxsVeTodUWGo2agglZG9FNcnGbbAWoc0dki/RGu7jxR?= =?us-ascii?Q?K6Eo3BK6GV8LXl73ngeswDIh4m4PeQOIv7J275TXXbTQledHTKTA+k6wlrzo?= =?us-ascii?Q?lwTf/7vF9DkYQ/2jE1vQAJvU0tDzFQtpyueEAZgD9kQcAHHNptbMZcLpQ6iX?= =?us-ascii?Q?+UGPH32c2+8NDplStq748vK06wws6XEDSZBmaMcioCVAxRVWjxoNvHdQDRGm?= =?us-ascii?Q?fyZ4CsbLdJKW2ROrFdBB4NMRYIzUkO3kfmxND4dUU/0J9Da6HNuI/ZfATVbQ?= =?us-ascii?Q?B8Dawj5EFgXQeNEYv39W2OFrzwnjdVCCYZf9fIdLXFooq85Li68FyLF0plaX?= =?us-ascii?Q?FUovFke04zn3yzCYagFjJcbAInTe0VnBxFhUL3+MgXWlp9GJkfJ3tgE45khz?= =?us-ascii?Q?RjzJ1oA8hfspXNspBhO+qUxGtLJg8yN0+jix979BIi5IFItRisbHLSSV7kZO?= =?us-ascii?Q?zwbgerNCLZyxr0YtghWeZ+reNcMGOGf+ww/v+y4xMMTsHP1q62tmE8JpnJiG?= =?us-ascii?Q?KB9B+NtFgfvjTD5+gijwhfqlO7o58w8inI55ZuvX9qIqBYo9yJXgcWu6C3yI?= =?us-ascii?Q?MsHlQdLQ1M6F8Ttyu2nNA8wGhaN7nvFboexeCMCjuLAdI+75RDKWksZvpqGK?= =?us-ascii?Q?e5rD8QkmufFtubCDiPek4JOg5RZLthb2/Z4DB2a786LEQ82e6Tq8UFXarRmH?= =?us-ascii?Q?JubrhbXAF2zZnGJ0fcACTYgyXmUuGSD9AYWKTiEtsGPrrIZfkOhkOhjkVkpr?= =?us-ascii?Q?iPw90wFtODDpFwUmB7PJay/KfBbC4RmfVgP6cyRpw8omkbrEZVssvW31dkDK?= =?us-ascii?Q?HivRdkDZYYW53y6wh9AlKF5OAkv+pnw7f8TpG0/BiwL+lLcFWWz9n0IToy1e?= =?us-ascii?Q?Q7il7IwZc1Ypy5HUm/jxPVSs3kc361cPDE+IhOZHQyqt8XQFWMoAmqpZbNQj?= =?us-ascii?Q?TI0Hr3HyG+vstMmXKSN41eNfDsRNrmsOockg5bIBucup+uCqh0CQyDBvrryM?= =?us-ascii?Q?XyZhtTG0j69CTiAT/UHu9VajFLY+rlLpR5XSkz7z9Jk+PDRgtuU2S/QBmyDN?= =?us-ascii?Q?eBfdDaz3ahUjlTw9rQ+mOxd2VHBWR/oZ/3H2hsVjvPw8s2vT8hgQ3HQewm4P?= =?us-ascii?Q?MBePAVkp+LLG/REu4vEg4/KUzfyTpqb1NttVLZQLWS+iGa0Ftv0czEcXs2/2?= =?us-ascii?Q?Jw=3D=3D?= X-OriginatorOrg: os.amperecomputing.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9b9f9d20-e8f9-4b02-4791-08dca14d06f5 X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB7287.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jul 2024 01:58:58.2270 (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: LYy6RDepkhBlBZwWvJF1x3qeFDpl1qmbjkYfpwVIwz4u5IkY1pR9uZTkVtuu5Ffo6arnZay5RTkEEeSJLTnnastZfvjQN8Gupq/26VMY8oc= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR01MB9065 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 Resent-Date: Wed, 10 Jul 2024 18:59:01 -0700 Resent-From: nhi@os.amperecomputing.com Reply-To: devel@edk2.groups.io,nhi@os.amperecomputing.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 7eD7G3DF3NE07fr13mAogRUcx7686176AA= 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=20240206 header.b=nVkqdwOB; dmarc=pass (policy=none) header.from=groups.io; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io On 7/10/2024 10:12 PM, Nickle Wang wrote: > Hi @Nhi Pham , >=20 > I am sorry for taking so long to address your review comments. I updated= =20 > pull request here: https://github.com/tianocore/edk2-platforms/pull/76=20 > Thanks, Nickle. I will continue reviewing (if any comments) on the PR=20 instead. >=20 > I addressed most of comments and I have questions about below review=20 > comments. Please find my feedback inline below. >=20 > > Should we add a PCD for the OEN to be configured by platform specific >=20 > > BMC? Or this protocol is only to support OpenBMC. >=20 > Yes, per description in Readme.md, this is the protocol only to support= =20 > OpenBMC implementation. >=20 > > I'm thinking the caller sequence here. Typically, the caller might che= ck >=20 > > the presence of a blob by calling GetCount () and Enumerate () before >=20 > > opening a blob session. This check here could waste time. Or, do we ca= ll >=20 > > open direction the blob session without pre-checking? >=20 > With this implementation, user can call open directly and it will check= =20 > the presence of blob for us. And yes, this is how we use it in NVIDIA=20 > driver. >=20 > > There might be developer mistake when executing the transfer before >=20 > > opening the session. How do we handle this failure path? Do we need to >=20 > > maintain a state machine for that? >=20 > Caller can only get the session ID by calling open(). Session ID is=20 > required when calling read, write, and commit. If caller make up a=20 > session ID, caller is expected to see error return. >=20 > > How do callers know the data format of metadata for writing correctly? >=20 > I check the spec here:=20 > https://github.com/openbmc/phosphor-ipmi-blobs?tab=3Dreadme-ov-file#bmcbl= obwritemeta-10 =C2=A0 And there is no description about data = format.=C2=A0 Since we don't use this function in our driver, may I know if= you have suggestion for this? I don't have a strong opinion here (**That was just my curiosity**). So,=20 we can just leave it as your implementation or drop it as there is no=20 consumer. >=20 > > +IpmiBlobTransferDxeDriverEntryPoint ( >=20 > > +=C2=A0 IN EFI_HANDLE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ImageH= andle, >=20 > > +=C2=A0 IN EFI_SYSTEM_TABLE=C2=A0 *SystemTable >=20 > > +=C2=A0 ) >=20 > > +{ >=20 > > +=C2=A0 return gBS->InstallMultipleProtocolInterfaces ( >=20 > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 &ImageHandle, >=20 > > Nit: Typically, we could also use gImageHandle instead. >=20 > Sorry I don't follow you here. >=20 > gImageHandle is used as global variable in driver. But here we just use= =20 > the ImageHandle from function parameter and we don't keep it as global=20 > variable, right? The pointers should be identical since they are cached by the=20 UefiBootServicesTableLib's constructor prior to the call to=20 IpmiBlobTransferDxeDriverEntryPoint. Thanks, Nhi -=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 (#119879): https://edk2.groups.io/g/devel/message/119879 Mute This Topic: https://groups.io/mt/106115743/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-