Re: [PATCH v15 16/23] x86/sgx: Enumerate and track EPC sections

From: Jarkko Sakkinen
Date: Tue Nov 06 2018 - 07:10:30 EST


On Sat, Nov 03, 2018 at 03:22:34PM +0200, Andy Shevchenko wrote:
> On Sat, Nov 3, 2018 at 1:17 AM Jarkko Sakkinen
> <jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
> >
> > From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> >
> > Enumerate Enclave Page Cache (EPC) sections via CPUID and add the data
> > structures necessary to track EPC pages so that they can be allocated,
> > freed and managed. As a system may have multiple EPC sections, invoke
> > CPUID on SGX sub-leafs until an invalid leaf is encountered.
> >
> > On NUMA systems, a node can have at most one bank. A bank can be at
> > most part of two nodes. SGX supports both nodes with a single memory
> > controller and also sub-cluster nodes with severals memory controllers
> > on a single die.
> >
> > For simplicity, support a maximum of eight EPC sections. Current
> > client hardware supports only a single section, while upcoming server
> > hardware will support at most eight sections. Bounding the number of
> > sections also allows the section ID to be embedded along with a page's
> > offset in a single unsigned long, enabling easy retrieval of both the
> > VA and PA for a given page.
>
> > + iounmap(section->va);
>
> > + section->va = ioremap_cache(addr, size);
> > + if (!section->va)
> > + return -ENOMEM;
>
> I forgot if it's a place in code on which we discussed ioremap() vs.
> memremap() call.
> And I forgot why the conclusion is to leave ioremap().
>
> Sorry, if I'm mistaken.

As far as I can tell you are not mistaken. It happened that in the patch
set version that you gave this comment I first the fixed missing __iomem
annotations because I like to do other changes on top of code that has
no known regressions. Then I guess I simply forgot to replace it with
memremap(). I will do it for the next version.

/Jarkko