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 DEE2D941885 for ; Thu, 20 Jul 2023 18:40:11 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=xb/WbmkqCxHaPwGtlNM5QQX/bcRQzpZNNY2jv7QIKhw=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:X-Received:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To: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=1689878410; v=1; b=FV59qxDtkq8XxYOh2J/ncFNoeBJJ9kfMdMF4kNaTotWX49GrHu3wyb6InjYotBiafSldqdhX BobQz5hGCi2aw28Te9nfz6oYb7IAKOyekJTCmrbqwXnxqCwBfwFOaYHKDq7mPwASY2yUqvLhq6z s3ulGT9Eg51VuAbtLIsyda/w= X-Received: by 127.0.0.2 with SMTP id 43AJYY7687511x03yxU2KpBJ; Thu, 20 Jul 2023 11:40:10 -0700 X-Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by mx.groups.io with SMTP id smtpd.web11.1078.1689878409197736885 for ; Thu, 20 Jul 2023 11:40:09 -0700 X-Received: by mail-pf1-f171.google.com with SMTP id d2e1a72fcca58-668711086f4so794983b3a.1 for ; Thu, 20 Jul 2023 11:40:09 -0700 (PDT) X-Gm-Message-State: P2Dr2qumWvvCAXSL4QxEaaLwx7686176AA= X-Google-Smtp-Source: APBJJlHMBBUbKfoqSO4PGbT/YZwp70W67J0ntlJNcU6Qgnl8ae4KIYQDKCKs1K6I1pwQUg9CJ6h1Bw== X-Received: by 2002:a05:6a20:3d8b:b0:132:842f:8adb with SMTP id s11-20020a056a203d8b00b00132842f8adbmr359499pzi.23.1689878408208; Thu, 20 Jul 2023 11:40:08 -0700 (PDT) X-Received: from [192.168.50.35] ([50.46.230.135]) by smtp.gmail.com with ESMTPSA id i127-20020a639d85000000b0055c656ef91asm1573250pgd.77.2023.07.20.11.40.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 20 Jul 2023 11:40:07 -0700 (PDT) Message-ID: Date: Thu, 20 Jul 2023 11:40:06 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v2 0/9] Add ImagePropertiesRecordLib and Fix MAT Bugs To: devel@edk2.groups.io, ray.ni@intel.com Cc: Andrew Fish , Ard Biesheuvel , "Bi, Dandan" , "Dong, Eric" , Gerd Hoffmann , "Dong, Guo" , "Guo, Gua" , "Lu, James" , "Wang, Jian J" , "Yao, Jiewen" , "Justen, Jordan L" , Leif Lindholm , "Gao, Liming" , "Kumar, Rahul R" , Sami Mujawar , "Rhodes, Sean" References: <20230720000544.146-1-t@taylorbeebe.com> From: "Taylor Beebe" In-Reply-To: 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,t@taylorbeebe.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: 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=FV59qxDt; dmarc=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 On 7/19/23 10:19 PM, Ni, Ray wrote: > Taylor, > Thank you for your effort for removing the duplicated logic in Dxe/Smm Co= re and fixing the bugs. >=20 > Two general comments: > #1. Can you refactor your patch series in a way that the new lib code is = like a "git move" instead of "git add"? For example, you could add an empty= lib first and update all DSC to depend on the new lib. Then move the lib c= ode from DxeCore to the lib folder. So that when reviewing the code changes= , they are relative smaller. >=20 There are enough differences between the DXE and SMM MAT logic that the "git move" you suggest won't resolve as cleanly as you hope without first creating separate library instances then walking them on to a common implementation. Is there another way I can make this more digestible? The MAT logic is very dense, but the most complex part is the=20 SplitTable() routine. The host-based unit test should prove that the expected input/output of the SplitTable() function is correct in this update. > #2. I see that you directly move the code to lib and update consumer to c= all the lib APIs. Do you think it's feasible to refine the code further suc= h that the responsibilities of DxeCore and the lib can be clearer and with = that the lib APIs can be more meaningful? >=20 Yes, the currently exposed functions can be renamed and have more descriptive function headers. I can also round out missing functionality to attempt to further reduce the code footprint in the MemoryAttributesTable.c files. For example: RemoveImagePropertiesRecord() InsertImagePropertiesRecord() DeleteImagePropertiesRecord() CreateImagePropertiesRecord() -=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 (#107110): https://edk2.groups.io/g/devel/message/107110 Mute This Topic: https://groups.io/mt/100246933/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-