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 D679ED80281 for ; Wed, 17 Apr 2024 23:53:38 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=cvV+HgnyAaON6iIyoI1wJLUBXy/gTkaUzZXoDl6nbHI=; c=relaxed/simple; d=groups.io; h=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:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Language; s=20240206; t=1713398017; v=1; b=fD66EHkrIJu8yUTKyOb+QLv3Gulr7lGwb6vRKja4e/UnDbAtmL+/MIWHIN1XfJSJ/Kl0tEJK v0KAEN20kdVx553hXD2aUtZ1zOrRyv67N6E4aA60U51JqK/noAi0cUKwL/7+Rjm2dd1vVr/iX7z h0U12h6r6K7C0dskgVf/RXiTK+JGMKW/bYMhKXfj9xZgZb7dTeIRaxPhNVMW2rNv8QhwmpN01kt Atq0n4X5hnc1fE2X8Zte5QEbjIqC9JTun0Q+EsdtPNWFsOqmi4SfstrDxcRdvUDUYDqOpyahv8R 1C99gDpJCYMJnCRUsib3WpklhX71q2C9Rf8ueFcNkcmZA== X-Received: by 127.0.0.2 with SMTP id wjPQYY7687511xsUJzGHFCDw; Wed, 17 Apr 2024 16:53:37 -0700 X-Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) by mx.groups.io with SMTP id smtpd.web10.946.1713398016188047286 for ; Wed, 17 Apr 2024 16:53:36 -0700 X-Received: by mail-pl1-f169.google.com with SMTP id d9443c01a7336-1e86d56b3bcso2874355ad.1 for ; Wed, 17 Apr 2024 16:53:36 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCWQ2lbN9MrJUqsAI0zF+SAKRrCOotDiUQ6g4sa+shSKqtSVHe/azqCd2LMRZKeQk70gZyWxDEzWRcyaAdoaShXB2OU0Lw== X-Gm-Message-State: W9kFuCE2EiuEtIn8zvBr7qiqx7686176AA= X-Google-Smtp-Source: AGHT+IEj7il60xH8+PzGM1n5YCnArHQtOuoETPaNWb69ID/PZV1Xmpdt8cF5EADHzW67rwgsWVbb+Q== X-Received: by 2002:a17:90a:e20a:b0:2a1:f586:d203 with SMTP id a10-20020a17090ae20a00b002a1f586d203mr1047854pjz.41.1713398015384; Wed, 17 Apr 2024 16:53:35 -0700 (PDT) X-Received: from [10.0.4.35] ([4.155.48.121]) by smtp.gmail.com with ESMTPSA id lo8-20020a170903434800b001e546a10c50sm205355plb.286.2024.04.17.16.53.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Apr 2024 16:53:35 -0700 (PDT) Message-ID: <140baa4f-b082-4121-bc34-7c03002d8de6@gmail.com> Date: Wed, 17 Apr 2024 16:53:34 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows To: "Huang, Yanbo" , "devel@edk2.groups.io" , "Bi, Dandan" Cc: "Wang, Jian J" , "Gao, Liming" , "Zhou, Jianfeng" References: <20231127181818.411-1-taylor.d.beebe@gmail.com> <20231127181818.411-11-taylor.d.beebe@gmail.com> <45b9b2a8-4bbb-4d67-94a9-6c6d6607feb7@gmail.com> <045edd40-88e7-4af2-ab15-61aa8701f9a4@gmail.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 Resent-Date: Wed, 17 Apr 2024 16:53:36 -0700 Resent-From: taylor.d.beebe@gmail.com Reply-To: devel@edk2.groups.io,taylor.d.beebe@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="------------fvM2Nhu8qL0uzF8j7n9ubUur" Content-Language: en-CA X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=fD66EHkr; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none) --------------fvM2Nhu8qL0uzF8j7n9ubUur Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Yanbo, I didn't do it in the way you suggest for the same reason that the SplitTable() logic doesn't set attributes on descriptors of type EfiRuntimeServicesData or other memory types. The purpose of the SplitTable() function is to use the input image records to split descriptors so each image section has it's own descriptor. I think it's reasonable to set the attributes on descriptors associated with images because those new descriptors are the intended side effect of the function, but I don't think setting attributes on other descriptors is a good design pattern. This pattern matches what's done in PiSmmCore/MemoryAttributesTable.c. Also, even if we did it the way you suggest, we would still need call EnforceMemoryMapAttribute() later to set XP on the EfiRuntimeServicesData descriptors. Can you or Dandan explain the origin of the extra EfiRuntimeServicesCode regions which aren't part of loaded runtime images? It would be a good datapoint for our discussion on the proposed fix. -Taylor On 4/17/2024 7:04 AM, Huang, Yanbo wrote: > > Hi Taylor, > > Thanks for your update. > > After test, issue can be fixed by your patch. > > But why we not set the EFI_MEMORY_XP or EFI_MEMORY_RO attribute in > SplitRecord API? > > If we set the attribute in the beginning of the NewRecord created, it > seems we don’t need to EnforceMemoryMapAttribute later? > > Best Regards, > > Yanbo Huang > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117937): https://edk2.groups.io/g/devel/message/117937 Mute This Topic: https://groups.io/mt/105477564/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- --------------fvM2Nhu8qL0uzF8j7n9ubUur Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi Yanbo,

I didn't do it in the way you suggest for the same reason that the SplitTable() logic doesn't set attributes
on descriptors of type EfiRuntimeServicesData or other memory types. The purpose of the SplitTable() function
is to use the input image records to split descriptors so each image section has it's own descriptor. I think
it's reasonable to set the attributes on descriptors associated with images because those new descriptors are the
intended side effect of the function, but I don't think setting attributes on other descriptors is a good design pattern.
This pattern matches what's done in PiSmmCore/MemoryAttributesTable.c. Also, even if we did
it the way you suggest, we would still need call EnforceMemoryMapAttribute() later to set XP on the
EfiRuntimeServicesData descriptors.

Can you or Dandan explain the origin of the extra  EfiRuntimeServicesCode regions which aren't
part of loaded runtime images? It would be a good datapoint for our discussion on the proposed fix.

-Taylor
On 4/17/2024 7:04 AM, Huang, Yanbo wrote:

Hi Taylor,

 

Thanks for your update.

After test, issue can be fixed by your patch.

But why we not set the EFI_MEMORY_XP or EFI_MEMORY_RO attribute in SplitRecord API?

If we set the attribute in the beginning of the NewRecord created, it seems we don’t need to EnforceMemoryMapAttribute later?

 

Best Regards,

Yanbo Huang

_._,_._,_
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#117937) | | Mute This Topic | New Topic
Your Subscription | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------fvM2Nhu8qL0uzF8j7n9ubUur--