On Wed, Jun 24, 2020 at 05:57:52PM -0700, Sean Christopherson wrote: > 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. I'm cool with this. I think that this patch shows that the current candidate patch set (v33) is in a good shape: the diff adheres very cleanly on what we have. /Jarkko
