From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f178.google.com (mail-oi1-f178.google.com [209.85.167.178]) by mx.groups.io with SMTP id smtpd.web09.3707.1617256226184533490 for ; Wed, 31 Mar 2021 22:50:26 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@9elements.com header.s=google header.b=HzDqgD33; spf=pass (domain: 9elements.com, ip: 209.85.167.178, mailfrom: patrick.rudolph@9elements.com) Received: by mail-oi1-f178.google.com with SMTP id k25so714475oic.4 for ; Wed, 31 Mar 2021 22:50:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L5USkHbg0rYeX63+UXasrD20J7G5Adwp5TJiDRVdg+M=; b=HzDqgD334vZwNHyFk3sq+eH/Nm4egT81MxmWqWKQdsmUmNTAoA+IXtiasR9DdlqbSz o/oNzIpzbjKnU8Ann8b5qS/3+vDxuZtZ9y3De8zDnso0T7+EHrVk9m7+mTvdUKwqawg7 zj0JrHilqfZ+PP8uJYjeDZvls3JPqANTcniUcQChjv/j7iJ+Zkk9Pm5YE4hF9YvOAhu+ WSM7Wo/algMy6lWXoVgSOFrtGosxmReY7qXkhASMjoGCKNdgIJewE6mIQoOoHIKjvzxS 4RCcO9fEBfQ5lY519faUJSoUGsKINa8PCd45E1ppMyTw1JAOHJrtn60NkD9lpMEnIVZF iuCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L5USkHbg0rYeX63+UXasrD20J7G5Adwp5TJiDRVdg+M=; b=YWZFow85RwpMtEto8chANJj3ayD9HAb2CQ79iwJshsVSGmNcHeTQUa+zq4/QmT3vkK AQyc3QyuiJ2acyiR+tSh2Sn77cyUn1fw1B09kNl1ZH640m1A933W8eZbnv786XkN01HK bob+dfC5itJzkVO3trkbTNwU7TRpLWgRrqfrAPSJ0K+ICi854pBBoaivgZ2QStp/UY5N Z62GOwg0L7Q9tCMbUp3j8iUHMbctuLv1ZoZHdETFNu7z5M7NYGJ6E/cEFnmSL/0jH4Ct Lr4vQewvcQ76Rw6crDTnbWftFpvyqOcmmpujI0iLWdM1TaAW9vf8wu+y+NfomQRaXmF7 n6sw== X-Gm-Message-State: AOAM5331D2BWsrpuo6mrdhR38FiXvjgMsU7jJNLZdnaN/D9Mko8wv786 SWZ5CbG4wvR7syffPO4whxzEWMplvC+lu0PJq05UmQ== X-Google-Smtp-Source: ABdhPJy826mzqOIdvzeFHqGTgUpHcCfjCj1gNd02aRw8ycP18GyLmXPhkClWHrpyUHRKZEBhnPsYNSnLI0l3x+L19Vo= X-Received: by 2002:aca:1c14:: with SMTP id c20mr4686888oic.146.1617256225559; Wed, 31 Mar 2021 22:50:25 -0700 (PDT) MIME-Version: 1.0 References: <20210301143221.2775162-1-patrick.rudolph@9elements.com> In-Reply-To: From: "Patrick Rudolph" Date: Thu, 1 Apr 2021 07:50:14 +0200 Message-ID: Subject: Re: [edk2-devel] [PATCH - resend] MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables To: "Liu, Zhiguang" Cc: "devel@edk2.groups.io" , "Ni, Ray" , "Chaganty, Rangasai V" , "Bi, Dandan" , "Zeng, Star" , "Gao, Zhichao" , "You, Benjamin" , "philipp.deppenwiese@9elements.com" , "Ma, Maurice" , "Dong, Guo" Content-Type: text/plain; charset="UTF-8" Hi Zhiguang, this is still on my todo-list, but I'm quite busy right now. If you got time please take care of that patch. Regards, Patrick Rudolph On Thu, Apr 1, 2021 at 6:42 AM Liu, Zhiguang wrote: > > Hi Patrick Rudolph, > > I also want this feature that enables Smbios driver to parse and install existing SMBIOS table. > I notice there are some comments about your patch. If you don't have time to refine it, I can keep your signed-off-by, and continue your work based on Ray and Guo's comments. > > Thanks > Zhiguang > > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Ni, Ray > > Sent: Wednesday, March 3, 2021 5:15 PM > > To: Patrick Rudolph > > Cc: devel@edk2.groups.io; Chaganty, Rangasai V > > ; Bi, Dandan ; Zeng, > > Star ; Gao, Zhichao ; You, > > Benjamin ; > > philipp.deppenwiese@9elements.com; Ma, Maurice > > ; Dong, Guo > > Subject: Re: [edk2-devel] [PATCH - resend] > > MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables > > > > I have 5 more comments embedded, can you read and reply? > > > > > -----Original Message----- > > > From: Patrick Rudolph > > > Sent: Wednesday, March 3, 2021 4:38 PM > > > To: Ni, Ray > > > Cc: devel@edk2.groups.io; Chaganty, Rangasai V > > > ; Bi, Dandan ; > > Zeng, > > > Star ; Gao, Zhichao ; You, > > > Benjamin ; > > > philipp.deppenwiese@9elements.com; Ma, Maurice > > > ; Dong, Guo > > > Subject: Re: [edk2-devel] [PATCH - resend] > > > MdeModulePkg/Universal/SmbiosDxe: Scan for existing tables > > > > > > Hi Ray, > > > thanks for your feedback. > > > > > > Currently a single HOB containing all the SMBIOS table is exported by > > > coreboot. > > > As coreboot doesn't support multiple HOBs with the same ID, #2 isn't a > > > solution. > > > > > > I'll look into passing a HOB instead of using > > > EfiGetSystemConfigurationTable and see if I can get rid of the table > > > shadow copy. > > > > > > Regards, > > > Patrick Rudolph > > > > > > On Wed, Mar 3, 2021 at 9:13 AM Ni, Ray wrote: > > > > > > > > In general, I agree this solution that lets SMBIOS driver directly absorbs > > the > > > SMBIOS table from PEI. > > > > This can eliminate the needs of a separate driver that consumes the HOB > > > and calls SMBIOS protocol to add the SMBIOS structures. > > > > > > > > There are two options for the HOB design: > > > > 1. A single HOB that points to the SMBIOS table. > > > > 2. Multiple HOBs that each points to a SMBIOS structure. > > > > > > > > In my opinion, option #2 is more flexible because it doesn't require the > > > bootloader to consolidate all the SMBIOS structures together. > > > > The CPU module in the bootloader can produce the type 4 and 7 > > structures. > > > > The PCI module in the bootloader can produce the type 9 structures. > > > > > > > > But, I am not sure if option #2 is conflict with what coreboot does. Does > > > coreboot produce the whole SMBIOS table in a single buffer? > > > > Option #2 also doesn't care whether it's a SMBIOS 3.0 table or 2.x table. > > > > > > > > >+ Status = EfiGetSystemConfigurationTable ( > > > > > > > > 1. Why don't you directly get the data from HOB list? This can eliminate > > the > > > code in BlSupportDxe that gets data in HOB and publishes to > > > > configuration table. > > > > > > > > > +ValidateSmbios20Table( > > > > > +ValidateSmbios30Table( > > > > > > > > 2. I will defer to experts (Dandan, Star and Zhichao) to review whether > > the > > > above two functions are implemented properly. > > > > > > > > > > > > > > +ParseAndAddExistingSmbiosTable( > > > > > + IN EFI_HANDLE ImageHandle, > > > > > + IN SMBIOS_STRUCTURE_POINTER Smbios, > > > > > + IN UINTN Length > > > > > +) { > > > > > + EFI_STATUS Status; > > > > > + CHAR8 *String; > > > > > + EFI_SMBIOS_HANDLE SmbiosHandle; > > > > > + SMBIOS_STRUCTURE_POINTER SmbiosEnd; > > > > > + > > > > > + SmbiosEnd.Raw = Smbios.Raw + Length; > > > > > + > > > > > + do { > > > > > + // Check for end marker > > > > > + if (Smbios.Hdr->Type == 127) { > > > > > > > > 3. Please use SMBIOS_TYPE_END_OF_TABLE instead of hardcode 127. > > > > > > > > > > > > > > + CopyMem (Smbios.Raw, (VOID *)Smbios30Table, Smbios30Table- > > > > > >TableMaximumSize); > > > > > > > > 4. Should we copy from Smbios30Table->TableAddress instead of > > > Smbios30Table? > > > > > > > > > > > > > > + Status = ParseAndAddExistingSmbiosTable(ImageHandle, Smbios, > > > > > Smbios30Table->TableMaximumSize); > > > > > > > > 5. Can you explain in specific why SMBIOS table should be duplicated > > > before parsing? > > > > > > > > > > > > > > > > >