On Wed, Jun 24, 2020 at 05:25:59PM -0700, Dave Hansen wrote: > On 6/24/20 4:54 PM, Sean Christopherson wrote: > >> Does this actually work? > >> > >> The node span (->node_start_pfn through start+->node_spanned_pages) only > >> contains pages which the OS is actively managing, usually RAM but > >> sometimes also persistent memory. This has some assumption that the SGX > >> PFNs are within the node's span. I would only _expect_ that to happen > >> if the node was built like this: > >> > >> | Node-X RAM | EPC | Node-X RAM | > >> > >> If the EPC was on either end: > >> > >> | Node-X RAM | EPC | > >> or > >> | EPC | Node-X RAM | > >> > >> I suspect that the pgdat span wouldn't include EPC. EPC is, if I > >> remember correctly, a E820_RESERVED region. > > It is indeed E820_RESERVED, but the BIOS WG for ICX states that EPC regions > > should be enumerated in ACPI SRAT along with regular memory. > > > > But, I haven't actually verified that info makes its way into the kernel's > > pgdata stuff. > > Considering this, are we all agreed that this patch is in no condition > to be submitted upstream? Yes, it needs to be tested first. I like the resulting code more than what we have now, but I see no reason to change it at this stage unless one of the maintainers actually complains.