| Message ID | 20180626142245.82850-18-kirill.shutemov@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h > index ba83fba4f9b3..dbfbd955da98 100644 > --- a/arch/x86/include/asm/mktme.h > +++ b/arch/x86/include/asm/mktme.h > @@ -29,6 +29,9 @@ void arch_free_page(struct page *page, int order); > > int sync_direct_mapping(void); > > +#define page_to_virt(x) \ > + (__va(PFN_PHYS(page_to_pfn(x))) + page_keyid(x) * direct_mapping_size) Please put this in a generic header so that this hunk represents the *default* x86 implementation that is used universally on x86. Then, please do #ifndef CONFIG_MKTME_WHATEVER #define page_keyid(x) (0) #endif > #else > #define mktme_keyid_mask ((phys_addr_t)0) > #define mktme_nr_keyids 0 > diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h > index 53c32af895ab..ffad496aadad 100644 > --- a/arch/x86/include/asm/page_64.h > +++ b/arch/x86/include/asm/page_64.h > @@ -23,7 +23,7 @@ static inline unsigned long __phys_addr_nodebug(unsigned long x) > /* use the carry flag to determine if x was < __START_KERNEL_map */ > x = y + ((x > y) ? phys_base : (__START_KERNEL_map - PAGE_OFFSET)); > > - return x; > + return x % direct_mapping_size; There are almost *surely* performance implications from this that affect anyone with this compile option turned on. There's now a 64-bit integer division operation which is used in places like kfree(). That's a show-stopper for me until we've done some pretty comprehensive performance analysis of this, which means much more than one kernel compile test on one system.
On Tue, Jun 26, 2018 at 04:38:23PM +0000, Dave Hansen wrote: > > diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h > > index ba83fba4f9b3..dbfbd955da98 100644 > > --- a/arch/x86/include/asm/mktme.h > > +++ b/arch/x86/include/asm/mktme.h > > @@ -29,6 +29,9 @@ void arch_free_page(struct page *page, int order); > > > > int sync_direct_mapping(void); > > > > +#define page_to_virt(x) \ > > + (__va(PFN_PHYS(page_to_pfn(x))) + page_keyid(x) * direct_mapping_size) > > Please put this in a generic header so that this hunk represents the > *default* x86 implementation that is used universally on x86. As I said, I disagree with you on the style preference. If a maintainer prefers it to be done in your way, I'll move the macros. > Then, please do > > #ifndef CONFIG_MKTME_WHATEVER > #define page_keyid(x) (0) > #endif Default page_keyid() implementation returns 0. > > #else > > #define mktme_keyid_mask ((phys_addr_t)0) > > #define mktme_nr_keyids 0 > > diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h > > index 53c32af895ab..ffad496aadad 100644 > > --- a/arch/x86/include/asm/page_64.h > > +++ b/arch/x86/include/asm/page_64.h > > @@ -23,7 +23,7 @@ static inline unsigned long __phys_addr_nodebug(unsigned long x) > > /* use the carry flag to determine if x was < __START_KERNEL_map */ > > x = y + ((x > y) ? phys_base : (__START_KERNEL_map - PAGE_OFFSET)); > > > > - return x; > > + return x % direct_mapping_size; > > There are almost *surely* performance implications from this that affect > anyone with this compile option turned on. There's now a 64-bit integer > division operation which is used in places like kfree(). Fair point. Apparently, modern CPU is good enough to hide the overhead. I'll look into how to avoid division. After quick look the only way to get it cheap (near free on my CPU) is to have power-of-2 direct_mapping_size and mask address before returning it. If direct_mapping_size is not power-of-2, the best variant I've come up with so far costs a branch for non-encrypted memory. For encrypted it is branch, 32-bit division and some bit shifting and masking. I'll look into this more.
diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h index ba83fba4f9b3..dbfbd955da98 100644 --- a/arch/x86/include/asm/mktme.h +++ b/arch/x86/include/asm/mktme.h @@ -29,6 +29,9 @@ void arch_free_page(struct page *page, int order); int sync_direct_mapping(void); +#define page_to_virt(x) \ + (__va(PFN_PHYS(page_to_pfn(x))) + page_keyid(x) * direct_mapping_size) + #else #define mktme_keyid_mask ((phys_addr_t)0) #define mktme_nr_keyids 0 diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h index 53c32af895ab..ffad496aadad 100644 --- a/arch/x86/include/asm/page_64.h +++ b/arch/x86/include/asm/page_64.h @@ -23,7 +23,7 @@ static inline unsigned long __phys_addr_nodebug(unsigned long x) /* use the carry flag to determine if x was < __START_KERNEL_map */ x = y + ((x > y) ? phys_base : (__START_KERNEL_map - PAGE_OFFSET)); - return x; + return x % direct_mapping_size; } #ifdef CONFIG_DEBUG_VIRTUAL
Per-KeyID direct mappings require changes into how we find the right virtual address for a page and virt-to-phys address translations. page_to_virt() definition overwrites default macros provided by <linux/mm.h>. We only overwrite the macros if MTKME is enabled compile-time. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> --- arch/x86/include/asm/mktme.h | 3 +++ arch/x86/include/asm/page_64.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)