| Message ID | 20150730165553.33962.20257.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Dan Williams |
| Headers | show |
On Thu, 2015-07-30 at 12:55 -0400, Dan Williams wrote: > Update memremap_pmem() to query the architecture for the mapping type of > the given persistent memory range and then pass those flags to generic > memremap(). arch_memremap_pmem_flags() is provided an address range to > evaluate in the event an arch has a need for different mapping types by > address range. For example the ACPI NFIT carries EFI mapping types in > its memory range description table. > > Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > arch/x86/include/asm/io.h | 2 +- > arch/x86/mm/ioremap.c | 16 ++++++++++------ > include/linux/pmem.h | 26 +++++++++++++++----------- > 3 files changed, 26 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h > index 1a9d44ee9ed0..26c81b01419c 100644 > --- a/arch/x86/include/asm/io.h > +++ b/arch/x86/include/asm/io.h > @@ -245,7 +245,7 @@ static inline void flush_write_buffers(void) > #endif > } > > -void __pmem *arch_memremap_pmem(resource_size_t offset, size_t size); > +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size); > #endif /* __KERNEL__ */ > > extern void native_io_delay(void); > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c > index ffbfcf2e701b..5a41e3e4ea1e 100644 > --- a/arch/x86/mm/ioremap.c > +++ b/arch/x86/mm/ioremap.c > @@ -205,6 +205,16 @@ err_free_memtype: > return NULL; > } > > +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size) > +{ > + /* > + * TODO: check the mapping type provided by platform firmware, > + * per range. > + */ > + return MEMREMAP_WB; > +} > +EXPORT_SYMBOL(arch_memremap_pmem_flags); I'm not sure that the architecture code has the right information to be able to tell the caller what the correct mapping is. I think what you are ultimately looking for is the "Address Range Memory Mapping Attribute" found in the System Physical Address Range Structure of the NFIT, right? (ACPI 6.0, table 5-128) How can we get to that structure from nfit-independent architecture code like arch_memremap_pmem_flags()? It seems like that if we want to make the mapping type dynamic we should get the flags from ND when the SPA range is provided by ND, and provide a reasonable default (WB, it seems) when we are dealing with legacy PMEM? If PMEM had the flag from whatever source, it could then pass it in to memremap_pmem(), and get a mapping of the appropriate caching. Another option of course is to just ignore the mapping attribute in the NFIT like we've been doing, and just say "we know WB will be good enough", and remove the TODO.
On Mon, Aug 3, 2015 at 10:21 AM, Ross Zwisler <ross.zwisler@linux.intel.com> wrote: > On Thu, 2015-07-30 at 12:55 -0400, Dan Williams wrote: >> Update memremap_pmem() to query the architecture for the mapping type of >> the given persistent memory range and then pass those flags to generic >> memremap(). arch_memremap_pmem_flags() is provided an address range to >> evaluate in the event an arch has a need for different mapping types by >> address range. For example the ACPI NFIT carries EFI mapping types in >> its memory range description table. >> >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> arch/x86/include/asm/io.h | 2 +- >> arch/x86/mm/ioremap.c | 16 ++++++++++------ >> include/linux/pmem.h | 26 +++++++++++++++----------- >> 3 files changed, 26 insertions(+), 18 deletions(-) >> >> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h >> index 1a9d44ee9ed0..26c81b01419c 100644 >> --- a/arch/x86/include/asm/io.h >> +++ b/arch/x86/include/asm/io.h >> @@ -245,7 +245,7 @@ static inline void flush_write_buffers(void) >> #endif >> } >> >> -void __pmem *arch_memremap_pmem(resource_size_t offset, size_t size); >> +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size); >> #endif /* __KERNEL__ */ >> >> extern void native_io_delay(void); >> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c >> index ffbfcf2e701b..5a41e3e4ea1e 100644 >> --- a/arch/x86/mm/ioremap.c >> +++ b/arch/x86/mm/ioremap.c >> @@ -205,6 +205,16 @@ err_free_memtype: >> return NULL; >> } >> >> +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size) >> +{ >> + /* >> + * TODO: check the mapping type provided by platform firmware, >> + * per range. >> + */ >> + return MEMREMAP_WB; >> +} >> +EXPORT_SYMBOL(arch_memremap_pmem_flags); > > I'm not sure that the architecture code has the right information to be able > to tell the caller what the correct mapping is. > It does in this instance as it has to pick a mapping type that is compatible with memcpy_to_pmem() and wmb_pmem(). > I think what you are ultimately looking for is the "Address Range Memory > Mapping Attribute" found in the System Physical Address Range Structure of the > NFIT, right? (ACPI 6.0, table 5-128) How can we get to that structure from > nfit-independent architecture code like arch_memremap_pmem_flags()? It seems > like that if we want to make the mapping type dynamic we should get the flags > from ND when the SPA range is provided by ND, and provide a reasonable default > (WB, it seems) when we are dealing with legacy PMEM? If PMEM had the flag > from whatever source, it could then pass it in to memremap_pmem(), and get a > mapping of the appropriate caching. We can do what ioremap() does and look it up through other means. > Another option of course is to just ignore the mapping attribute in the NFIT > like we've been doing, and just say "we know WB will be good enough", and > remove the TODO. I agree, but there may be a non-zero chance that a platform implementation wants strict adherence. Maybe move the TODO to a dev_dbg() that flags platforms where the NFIT mapping type is not "WB"?
On Mon, 2015-08-03 at 11:01 -0700, Dan Williams wrote: > On Mon, Aug 3, 2015 at 10:21 AM, Ross Zwisler > <ross.zwisler@linux.intel.com> wrote: > > On Thu, 2015-07-30 at 12:55 -0400, Dan Williams wrote: > >> Update memremap_pmem() to query the architecture for the mapping type of > >> the given persistent memory range and then pass those flags to generic > >> memremap(). arch_memremap_pmem_flags() is provided an address range to > >> evaluate in the event an arch has a need for different mapping types by > >> address range. For example the ACPI NFIT carries EFI mapping types in > >> its memory range description table. > >> > >> Cc: Ross Zwisler <ross.zwisler@linux.intel.com> > >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h index 1a9d44ee9ed0..26c81b01419c 100644 --- a/arch/x86/include/asm/io.h +++ b/arch/x86/include/asm/io.h @@ -245,7 +245,7 @@ static inline void flush_write_buffers(void) #endif } -void __pmem *arch_memremap_pmem(resource_size_t offset, size_t size); +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size); #endif /* __KERNEL__ */ extern void native_io_delay(void); diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index ffbfcf2e701b..5a41e3e4ea1e 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -205,6 +205,16 @@ err_free_memtype: return NULL; } +unsigned long arch_memremap_pmem_flags(resource_size_t offset, size_t size) +{ + /* + * TODO: check the mapping type provided by platform firmware, + * per range. + */ + return MEMREMAP_WB; +} +EXPORT_SYMBOL(arch_memremap_pmem_flags); + /** * ioremap_nocache - map bus memory into CPU space * @phys_addr: bus address of the memory @@ -310,12 +320,6 @@ void *arch_memremap(resource_size_t phys_addr, size_t size, } EXPORT_SYMBOL(arch_memremap); -void __pmem *arch_memremap_pmem(resource_size_t offset, size_t size) -{ - return (void __pmem *) arch_memremap(offset, size, MEMREMAP_WB); -} -EXPORT_SYMBOL(arch_memremap_pmem); - void __iomem *ioremap_prot(resource_size_t phys_addr, unsigned long size, unsigned long prot_val) { diff --git a/include/linux/pmem.h b/include/linux/pmem.h index 1bf74c735fa0..03f9d73f3e13 100644 --- a/include/linux/pmem.h +++ b/include/linux/pmem.h @@ -28,10 +28,10 @@ static inline bool __arch_has_wmb_pmem(void) return false; } -static inline void __pmem *arch_memremap_pmem(resource_size_t offset, +static inline unsigned long arch_memremap_pmem_flags(resource_size_t offset, unsigned long size) { - return NULL; + return 0; } static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src, @@ -43,8 +43,8 @@ static inline void arch_memcpy_to_pmem(void __pmem *dst, const void *src, /* * Architectures that define ARCH_HAS_PMEM_API must provide - * implementations for arch_memremap_pmem(), arch_memcpy_to_pmem(), - * arch_wmb_pmem(), and __arch_has_wmb_pmem(). + * implementations for arch_memremap_pmem_flags(), + * arch_memcpy_to_pmem(), arch_wmb_pmem(), and __arch_has_wmb_pmem(). */ static inline void memcpy_from_pmem(void *dst, void __pmem const *src, size_t size) @@ -54,7 +54,7 @@ static inline void memcpy_from_pmem(void *dst, void __pmem const *src, size_t si static inline void memunmap_pmem(void __pmem *addr) { - iounmap((void __force __iomem *) addr); + memunmap((void __force *) addr); } /** @@ -85,16 +85,15 @@ static inline bool arch_has_pmem_api(void) * default_memremap_pmem + default_memcpy_to_pmem is sufficient for * making data durable relative to i/o completion. */ -static void default_memcpy_to_pmem(void __pmem *dst, const void *src, +static inline void default_memcpy_to_pmem(void __pmem *dst, const void *src, size_t size) { memcpy((void __force *) dst, src, size); } -static void __pmem *default_memremap_pmem(resource_size_t offset, - unsigned long size) +static inline unsigned long default_memremap_pmem_flags(void) { - return (void __pmem *) memremap(offset, size, MEMREMAP_WT); + return MEMREMAP_WT; } /** @@ -112,9 +111,14 @@ static void __pmem *default_memremap_pmem(resource_size_t offset, static inline void __pmem *memremap_pmem(resource_size_t offset, unsigned long size) { + unsigned long flags; + if (arch_has_pmem_api()) - return arch_memremap_pmem(offset, size); - return default_memremap_pmem(offset, size); + flags = arch_memremap_pmem_flags(offset, size); + else + flags = default_memremap_pmem_flags(); + + return (void __pmem *) memremap(offset, size, flags); } /**
Update memremap_pmem() to query the architecture for the mapping type of the given persistent memory range and then pass those flags to generic memremap(). arch_memremap_pmem_flags() is provided an address range to evaluate in the event an arch has a need for different mapping types by address range. For example the ACPI NFIT carries EFI mapping types in its memory range description table. Cc: Ross Zwisler <ross.zwisler@linux.intel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- arch/x86/include/asm/io.h | 2 +- arch/x86/mm/ioremap.c | 16 ++++++++++------ include/linux/pmem.h | 26 +++++++++++++++----------- 3 files changed, 26 insertions(+), 18 deletions(-)