public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Du Lin" <du.lin@intel.com>
To: Oliver Smith-Denny <osde@linux.microsoft.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"mikuback@linux.microsoft.com" <mikuback@linux.microsoft.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>, "Lin, Du" <du.lin@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Add the EFI_RESOURCE_ATTRIBUTE_SPECIAL_PURPOSE attribute
Date: Thu, 23 May 2024 09:17:12 +0000	[thread overview]
Message-ID: <BL1PR11MB54638FC25419C8B5905F20A19BF42@BL1PR11MB5463.namprd11.prod.outlook.com> (raw)
In-Reply-To: <aac6e33e-5726-4a38-94ab-9aff49324933@linux.microsoft.com>

Thanks for the quick response.

Agree that the PI and UEFI specs are vague on SP. That is also why I opted to minimize code changes to DXE core for SP support in patch https://edk2.groups.io/g/devel/message/118712.

Would it make more sense to let the caller determine if SP memory is available for UEFI via EFI resource types (e.g., EFI_RESOURCE_SYSTEM_MEMORY vs EFI_RESOURCE_MEMORY_RESERVED)?

CDAT can be read in PEI phase via DOE method and CDAT is important to support CXL 2.0. I believe CDAT spec is referencing EFI_MEMORY_TYPE and Memory Attributes defined in UEFI spec section 7.2. "EfiConventionalMemory Type with EFI_MEMORY_SP Attribute" may suggest that the memory type shall be EfiConventionalMemory and the attribute shall have SP set when reporting the memory to OS. And the concern is whether this combination can still be supported if we always mark resource HOBs with SP set as EfiGcdMemoryTypeReserved.

BRs,
Lin, Du

-----Original Message-----
From: Oliver Smith-Denny <osde@linux.microsoft.com> 
Sent: Wednesday, May 22, 2024 11:04 PM
To: devel@edk2.groups.io; Lin, Du <du.lin@intel.com>; mikuback@linux.microsoft.com
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Add the EFI_RESOURCE_ATTRIBUTE_SPECIAL_PURPOSE attribute

On 5/21/2024 8:40 PM, Du Lin wrote:
> Coherent Device Attribute Table (CDAT) specification defines a EFI memory type and attribute "EfiConventionalMemory Type with EFI_MEMORY_SP Attribute".
> Can we still support this type if assigning the GCD memory type "EfiGcdMemoryTypeReserved" for resource HOBs with the SPECIAL_PURPOSE attribute set?
> 
> CDAT 1.04 specification: 
> https://uefi.org/sites/default/files/resources/Coherent%20Device%20Att
> ribute%20Table_1.04%20published_0.pdf

Thanks for pointing out the CDAT specification. I think one of the issues here is that the PI and UEFI specs are very vague on EFI_MEMORY_SP, to the point that there is no user story on it.
This CDAT spec helps close some of the gap, but I think the PI and UEFI spec should be updated to be more verbose in the intended usage of this flag.

Does any CDAT code exist in edk2? A naive search didn't turn up anything (and even looking at some of the CXL structures I didn't see any usage outside of the header). Is the intention that the CDAT would be read in PEI and a resource descriptor HOB created from it? If so, I agree this would prevent EfiConventionalMemory from being set. I think the CDAT spec should not be referencing EFI memory types if that is the case, because resource descriptor HOBs deal with resource types, not EFI types, so there is a mismatch.

If the CDAT is read at a different time and a resource descriptor HOB is not created from it, then this should not affect it.

The reason that GCD reserved type was chosen here was that the assumption for CXL or other remote memory is that UEFI would not want to use it (what if the bus went down and DXE core was allocated there). By marking it reserved with the EFI_MEMORY_SP bit set, we ensure that UEFI does not use it and that the OS knows that this is usable, but the kernel itself may not want to use it, for the same reason (or for performance). Again, the UEFI spec does not describe what memory type this should be, so if there is a different use case here, please do share.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119137): https://edk2.groups.io/g/devel/message/119137
Mute This Topic: https://groups.io/mt/106165072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2024-05-23  9:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-18  0:57 [edk2-devel] [PATCH v3 1/1] MdeModulePkg: Add the EFI_RESOURCE_ATTRIBUTE_SPECIAL_PURPOSE attribute Michael Kubacki
2024-05-22  3:40 ` Du Lin
2024-05-22 15:03   ` Oliver Smith-Denny
2024-05-23  9:17     ` Du Lin [this message]
2024-05-23 21:05       ` Oliver Smith-Denny
2024-05-31  3:44         ` Du Lin
2024-05-31 17:00           ` Oliver Smith-Denny
2024-06-03  1:27             ` Du Lin
2024-06-10  1:44 ` Dhaval Sharma
2024-06-11 17:03   ` Oliver Smith-Denny
2024-06-11 17:30     ` Michael D Kinney
2024-06-11 17:53       ` Oliver Smith-Denny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL1PR11MB54638FC25419C8B5905F20A19BF42@BL1PR11MB5463.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox