From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by mx.groups.io with SMTP id smtpd.web11.3286.1680907376069256465 for ; Fri, 07 Apr 2023 15:42:56 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@bsdio.com header.s=fm3 header.b=K5rBa55Z; spf=pass (domain: bsdio.com, ip: 64.147.123.21, mailfrom: rebecca@bsdio.com) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id 28ADF3200319; Fri, 7 Apr 2023 18:42:55 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 07 Apr 2023 18:42:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdio.com; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1680907374; x=1680993774; bh=PhmnLYU4uHETLtguUvfx3dgELFNHtomPISr QjEB0NY0=; b=K5rBa55Z6t1WR/vbIsz460SjWOTIq2z8EYpoh9DtlcoH9TTulPR ZKwSG/+ynSq7oNMAsqBVSZxaeFaflj3cixLFskTx10uigA7jDHPJjRVqCWoL6lyB 5cGm0n/Mg4vxQCdFCwyUHxIbSoemTEzHWTICTVtkMf8IFelOgbNwfeLCVORaIVVA q4pDYR7Kh2ca1O/cEt9cMZ94G8i7DZ6foVsJdk2xcx7u6GldAoWuePesHGQldaF3 XaCQN0anzwyuMDD7j5eyIx72GMC61FjspCoTtkQOOFzaYTRvr/zh2HfLBq2EkeeI VJWBSYLW+s5+C97qik+pt8PFTtpgh9rRksw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1680907374; x=1680993774; bh=PhmnLYU4uHETLtguUvfx3dgELFNHtomPISr QjEB0NY0=; b=Fbxt00KJm4mAr35tCx6gHBhiahWgQcYyLUYcvMjuAY6yYFaNvFh +DXteeeMEu+seMWqa+HyFkAjzWgMqe5D2n2zBgRhLxQb0e4dmECkrMKkTPOcsyGB uO94D6JG9KHZNM7ENVWrWssqdL/kpNKzdFjsoLPy+3EEf3IprhhMI6TnTU8qPXuf Aqp+hbFad/NTDCSVWueXglwb9xWzm1/FcjAWOQCsZR2EegnKAHZIpXJAwPQmIYXb DRRJ03vu95kgf3jJFr+8b+CJJlxkyQ4ivrFyw7juby+t5DGsDJSo5JWvVsichT0a Ge6bDZWNphq/wA+A+ipDp5DBoKQAJx4d1zg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdejiedguddtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtfeejnecuhfhrohhmpeftvggs vggttggrucevrhgrnhcuoehrvggsvggttggrsegsshguihhordgtohhmqeenucggtffrrg htthgvrhhnpeegfeegveduheejgfduffefhfehleehiefghfetvdejvdelhfeukefhhfdv geehveenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpe hrvggsvggttggrsegsshguihhordgtohhm X-ME-Proxy: Feedback-ID: i5b994698:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 7 Apr 2023 18:42:53 -0400 (EDT) Message-ID: <63b08db7-50cd-e319-ecab-3ec380337874@bsdio.com> Date: Fri, 7 Apr 2023 16:42:52 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH v2 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer To: "Kinney, Michael D" , "devel@edk2.groups.io" , "Liu, Zhiguang" , "Gao, Liming" Cc: Rebecca Cran References: <20230407161909.43499-1-rebecca@bsdio.com> <20230407161909.43499-2-rebecca@bsdio.com> From: "Rebecca Cran" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 4/7/23 2:25 PM, Kinney, Michael D wrote: > Comments below. > > Hopefully this lib would only be used by modules that get compressed. I guess so, but that's for the user to decide. > > Might add GLOBAL_REMOVE_IF_UNREFERENCED to the arrays of strings to > help the optimizer remove the data that is not referenced. Good idea - I'll add that. >> +CONST CHAR8 * >> +EFIAPI >> +Jep106GetManufacturerName ( >> + IN UINT8 Code, >> + IN UINT8 ContinuationBytes >> + ) >> +{ >> + UINTN Index; >> + CONST JEDEC_MANUFACTURERS *ManufacturersBank; >> + >> + Index = 0; >> + >> + if (ContinuationBytes >= JEP106_MANUFACTURERS_NUM_BANKS) { >> + ASSERT (0); > Do you really want an ASSERT() from this? If this is data from a DIMM > I doubt we want platform to ASSERT(). Perhaps just return NULL? I'll remove it. I added it for validation when I was writing the SPD parsing library. >> +UINTN >> +EFIAPI >> +Jep106GetLongestManufacturerName ( >> + VOID >> + ) > Why is this API needed? Wouldn’t you really just need the > longest of the ones present in the current boot to build the > SMBIOS record?. Not the longest of all in the banks? I added it because it's useful to know the maximum possible size of the strings that could be in the SMBIOS Type 17 table before doing the SPD parsing. I'll remove it for now, and if it's really needed I can add it back in. -- Rebecca Cran