Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Thu, 2015-07-09 at 03:40 +0200, Luis R. Rodriguez wrote:
> On Tue, Jul 07, 2015 at 05:10:58PM -0600, Toshi Kani wrote:
> > On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote:
> > > On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM
> > > Linux
> > > wrote:
> > :
> > > > On ARM, we (probably) have a lot of cases where ioremap() is
> > > > used
> > > > multiple
> > > > times for the same physical address space, so we shouldn't rule
> > > > out
> > > > having
> > > > multiple mappings of the same type.
> > >
> > > Why is that done? Don't worry if you are not sure why but only
> > > speculate of the
> > > practice's existence (sloppy drivers or lazy driver developers).
> > > FWIW
> > > for x86
> > > IIRC I ended up concluding that overlapping ioremap() calls with
> > > the
> > > same type
> > > would work but not if they differ in type. Although I haven't
> > > written a
> > > grammer rule to hunt down overlapping ioremap() I suspected its
> > > use
> > > was likely
> > > odd and likely should be reconsidered. Would this be true for ARM
> > > too
> > > ? Or are
> > > you saying this should be a feature ? I don't expect an answer
> > > now
> > > but I'm
> > > saying we *should* all together decide on this, and if you're
> > > inclined to
> > > believe that this should ideally be avoided I'd like to hear
> > > that. If
> > > you feel
> > > strongly though this should be a feature I would like to know
> > > why.
> >
> > There are multiple mapping interfaces, and overlapping can happen
> > among
> > them as well. For instance, remap_pfn_range() (and
> > io_remap_pfn_range(), which is the same as remap_pfn_range() on
> > x86)
> > creates a mapping to user space. The same physical ranges may be
> > mapped to kernel and user spaces. /dev/mem is one example that may
> > create a user space mapping to a physical address that is already
> > mapped with ioremap() by other module.
>
> Thanks for the feedback. The restriction seems to be differing cache
> types
> requirements, other than this, are there any other concerns ? For
> instance are
> we completley happy with aliasing so long as cache types match
> everywhere? I'd
> expect no architecture would want cache types to differ when
> aliasing, what
> should differ then I think would just be how to verify this and it
> doesn't seem
> we may be doing this for all architectures.
>
> Even for userspace we seem to be covered -- we enable userspace
> mmap() calls to
> get their mapped space with a cache type, on the kernel we'd say use
> pgprot_writecombine() on the vma->vm_page_prot prior to the
> io_remap_pfn_range() -- that maps to remap_pfn_range() on x86 and as
> you note
> that checks cache type via reserve_memtype() -- but only on x86...
>
> Other than this differing cache type concern are we OK with aliasing
> in
> userspace all the time ?
>
> If we want to restrict aliasing either for the kernel or userspace
> mapping
> we might be able to do it, I just want to know if we want to or not
> care
> at all.
Yes, we allow to create multiple mappings to a same physical page as
long as their cache type is the same. There are multiple use-cases
that depend on this ability.
> > pmem and DAX also create mappings to the same NVDIMM ranges. DAX
> > calls
> > vm_insert_mixed(), which is particularly a problematic since
> > vm_insert_mixed() does not verify aliasing. ioremap() and
> > remap_pfn_range()
> > call reserve_memtype() to verify aliasing on x86.
> > reserve_memtype() is
> > x86-specific and there is no arch-generic wrapper for such check.
>
> As clarified by Matthew Wilcox via commit d92576f1167cacf7844 ("dax:
> does not
> work correctly with virtual aliasing caches") caches are virtually
> mapped for
> some architectures, it seems it should be possible to fix this for
> DAX somehow
> though.
I simply described this DAX case as an example of how two modules might
request different cache types. Yes, we should be able to fix this
case.
> > I think DAX could get a cache type from pmem to keep them in sync,
> > though.
>
> pmem is x86 specific right now, are other folks going to expose
> something
> similar ? Otherwise we seem to only be addressing these deep concerns
> for
> x86 so far.
pmem is a generic driver and is not x86-specific.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote: > diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h > index d8f8622fa044..4789b1cec313 100644 > --- a/include/asm-generic/iomap.h > +++ b/include/asm-generic/iomap.h > @@ -62,14 +62,6 @@ extern void __iomem *ioport_map(unsigned long port, > unsigned int nr); > extern void ioport_unmap(void __iomem *); > #endif > > -#ifndef ARCH_HAS_IOREMAP_WC > -#define ioremap_wc ioremap_nocache > -#endif > - > -#ifndef ARCH_HAS_IOREMAP_WT > -#define ioremap_wt ioremap_nocache > -#endif > - > #ifdef CONFIG_PCI > /* Destroy a virtual mapping cookie for a PCI BAR (memory or IO) */ > struct pci_dev; While at it we should also detangle ioremap() variants default implementations from requiring !CONFIG_MMU, so to be clear, if you have CONFIG_MMU you should implement ioremap() and iounmap(), then additionally if you have a way to support an ioremap_*() variant you should do so as well. You can include asm-generic/iomap.h to help complete ioremap_*() variants you may not have defined but note below. ***Big fat note**: this however assumes we have a *safe* general ioremap() to default to for all architectures but for a slew of reasons we cannot have this today and further discussion is needed to see if it may be possible one day. In the meantime we must then settle to advocate architecture developers to provide their own ioremap_*() variant implementations. We can do this two ways: 1) make new defaults return NULL - to avoid improper behaviour 2) revisit current default implementations on asm-generic for ioremap_*() variants and vet that they are safe for each architecture actually using them, if they are safe tuck under each arch its own mapping. After all this then convert default to return NULL. This will prevent future issues with other architectures. 3) long term: work towards the lofty objective of defining an architecturally sane iorema_*() variant default. This can only be done once all the semantics of all the others are well established. I'll provide a small demo patch with a very specific fix. We can either address this as separate work prior to your patchset or mesh this work together. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote: diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h index d8f8622fa044..4789b1cec313 100644 --- a/include/asm-generic/iomap.h +++ b/include/asm-generic/iomap.h @@ -62,14 +62,6 @@ extern void __iomem *ioport_map(unsigned long port, unsigned int nr); extern void ioport_unmap(void __iomem *); #endif -#ifndef ARCH_HAS_IOREMAP_WC -#define ioremap_wc ioremap_nocache -#endif - -#ifndef ARCH_HAS_IOREMAP_WT -#define ioremap_wt ioremap_nocache -#endif - #ifdef CONFIG_PCI /* Destroy a virtual mapping cookie for a PCI BAR (memory or IO) */ struct pci_dev; While at it we should also detangle ioremap() variants default implementations from requiring !CONFIG_MMU, so to be clear, if you have CONFIG_MMU you should implement ioremap() and iounmap(), then additionally if you have a way to support an ioremap_*() variant you should do so as well. You can include asm-generic/iomap.h to help complete ioremap_*() variants you may not have defined but note below. ***Big fat note**: this however assumes we have a *safe* general ioremap() to default to for all architectures but for a slew of reasons we cannot have this today and further discussion is needed to see if it may be possible one day. In the meantime we must then settle to advocate architecture developers to provide their own ioremap_*() variant implementations. We can do this two ways: 1) make new defaults return NULL - to avoid improper behaviour 2) revisit current default implementations on asm-generic for ioremap_*() variants and vet that they are safe for each architecture actually using them, if they are safe tuck under each arch its own mapping. After all this then convert default to return NULL. This will prevent future issues with other architectures. 3) long term: work towards the lofty objective of defining an architecturally sane iorema_*() variant default. This can only be done once all the semantics of all the others are well established. I'll provide a small demo patch with a very specific fix. We can either address this as separate work prior to your patchset or mesh this work together. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Thu, 2015-07-09 at 03:40 +0200, Luis R. Rodriguez wrote: On Tue, Jul 07, 2015 at 05:10:58PM -0600, Toshi Kani wrote: On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote: On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux wrote: : On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. Why is that done? Don't worry if you are not sure why but only speculate of the practice's existence (sloppy drivers or lazy driver developers). FWIW for x86 IIRC I ended up concluding that overlapping ioremap() calls with the same type would work but not if they differ in type. Although I haven't written a grammer rule to hunt down overlapping ioremap() I suspected its use was likely odd and likely should be reconsidered. Would this be true for ARM too ? Or are you saying this should be a feature ? I don't expect an answer now but I'm saying we *should* all together decide on this, and if you're inclined to believe that this should ideally be avoided I'd like to hear that. If you feel strongly though this should be a feature I would like to know why. There are multiple mapping interfaces, and overlapping can happen among them as well. For instance, remap_pfn_range() (and io_remap_pfn_range(), which is the same as remap_pfn_range() on x86) creates a mapping to user space. The same physical ranges may be mapped to kernel and user spaces. /dev/mem is one example that may create a user space mapping to a physical address that is already mapped with ioremap() by other module. Thanks for the feedback. The restriction seems to be differing cache types requirements, other than this, are there any other concerns ? For instance are we completley happy with aliasing so long as cache types match everywhere? I'd expect no architecture would want cache types to differ when aliasing, what should differ then I think would just be how to verify this and it doesn't seem we may be doing this for all architectures. Even for userspace we seem to be covered -- we enable userspace mmap() calls to get their mapped space with a cache type, on the kernel we'd say use pgprot_writecombine() on the vma-vm_page_prot prior to the io_remap_pfn_range() -- that maps to remap_pfn_range() on x86 and as you note that checks cache type via reserve_memtype() -- but only on x86... Other than this differing cache type concern are we OK with aliasing in userspace all the time ? If we want to restrict aliasing either for the kernel or userspace mapping we might be able to do it, I just want to know if we want to or not care at all. Yes, we allow to create multiple mappings to a same physical page as long as their cache type is the same. There are multiple use-cases that depend on this ability. pmem and DAX also create mappings to the same NVDIMM ranges. DAX calls vm_insert_mixed(), which is particularly a problematic since vm_insert_mixed() does not verify aliasing. ioremap() and remap_pfn_range() call reserve_memtype() to verify aliasing on x86. reserve_memtype() is x86-specific and there is no arch-generic wrapper for such check. As clarified by Matthew Wilcox via commit d92576f1167cacf7844 (dax: does not work correctly with virtual aliasing caches) caches are virtually mapped for some architectures, it seems it should be possible to fix this for DAX somehow though. I simply described this DAX case as an example of how two modules might request different cache types. Yes, we should be able to fix this case. I think DAX could get a cache type from pmem to keep them in sync, though. pmem is x86 specific right now, are other folks going to expose something similar ? Otherwise we seem to only be addressing these deep concerns for x86 so far. pmem is a generic driver and is not x86-specific. Thanks, -Toshi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 05:10:58PM -0600, Toshi Kani wrote:
> On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote:
> > On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux
> > wrote:
> :
> > > On ARM, we (probably) have a lot of cases where ioremap() is used
> > > multiple
> > > times for the same physical address space, so we shouldn't rule out
> > > having
> > > multiple mappings of the same type.
> >
> > Why is that done? Don't worry if you are not sure why but only
> > speculate of the
> > practice's existence (sloppy drivers or lazy driver developers). FWIW
> > for x86
> > IIRC I ended up concluding that overlapping ioremap() calls with the
> > same type
> > would work but not if they differ in type. Although I haven't
> > written a
> > grammer rule to hunt down overlapping ioremap() I suspected its use
> > was likely
> > odd and likely should be reconsidered. Would this be true for ARM too
> > ? Or are
> > you saying this should be a feature ? I don't expect an answer now
> > but I'm
> > saying we *should* all together decide on this, and if you're
> > inclined to
> > believe that this should ideally be avoided I'd like to hear that. If
> > you feel
> > strongly though this should be a feature I would like to know why.
>
> There are multiple mapping interfaces, and overlapping can happen among
> them as well. For instance, remap_pfn_range() (and
> io_remap_pfn_range(), which is the same as remap_pfn_range() on x86)
> creates a mapping to user space. The same physical ranges may be
> mapped to kernel and user spaces. /dev/mem is one example that may
> create a user space mapping to a physical address that is already
> mapped with ioremap() by other module.
Thanks for the feedback. The restriction seems to be differing cache types
requirements, other than this, are there any other concerns ? For instance are
we completley happy with aliasing so long as cache types match everywhere? I'd
expect no architecture would want cache types to differ when aliasing, what
should differ then I think would just be how to verify this and it doesn't seem
we may be doing this for all architectures.
Even for userspace we seem to be covered -- we enable userspace mmap() calls to
get their mapped space with a cache type, on the kernel we'd say use
pgprot_writecombine() on the vma->vm_page_prot prior to the
io_remap_pfn_range() -- that maps to remap_pfn_range() on x86 and as you note
that checks cache type via reserve_memtype() -- but only on x86...
Other than this differing cache type concern are we OK with aliasing in
userspace all the time ?
If we want to restrict aliasing either for the kernel or userspace mapping
we might be able to do it, I just want to know if we want to or not care
at all.
> pmem and DAX also create mappings to the same NVDIMM ranges. DAX calls
> vm_insert_mixed(), which is particularly a problematic since
> vm_insert_mixed() does not verify aliasing. ioremap() and remap_pfn_range()
> call reserve_memtype() to verify aliasing on x86. reserve_memtype() is
> x86-specific and there is no arch-generic wrapper for such check.
As clarified by Matthew Wilcox via commit d92576f1167cacf7844 ("dax: does not
work correctly with virtual aliasing caches") caches are virtually mapped for
some architectures, it seems it should be possible to fix this for DAX somehow
though.
> I think DAX could get a cache type from pmem to keep them in sync, though.
pmem is x86 specific right now, are other folks going to expose something
similar ? Otherwise we seem to only be addressing these deep concerns for
x86 so far.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 05:10:58PM -0600, Toshi Kani wrote: On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote: On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux wrote: : On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. Why is that done? Don't worry if you are not sure why but only speculate of the practice's existence (sloppy drivers or lazy driver developers). FWIW for x86 IIRC I ended up concluding that overlapping ioremap() calls with the same type would work but not if they differ in type. Although I haven't written a grammer rule to hunt down overlapping ioremap() I suspected its use was likely odd and likely should be reconsidered. Would this be true for ARM too ? Or are you saying this should be a feature ? I don't expect an answer now but I'm saying we *should* all together decide on this, and if you're inclined to believe that this should ideally be avoided I'd like to hear that. If you feel strongly though this should be a feature I would like to know why. There are multiple mapping interfaces, and overlapping can happen among them as well. For instance, remap_pfn_range() (and io_remap_pfn_range(), which is the same as remap_pfn_range() on x86) creates a mapping to user space. The same physical ranges may be mapped to kernel and user spaces. /dev/mem is one example that may create a user space mapping to a physical address that is already mapped with ioremap() by other module. Thanks for the feedback. The restriction seems to be differing cache types requirements, other than this, are there any other concerns ? For instance are we completley happy with aliasing so long as cache types match everywhere? I'd expect no architecture would want cache types to differ when aliasing, what should differ then I think would just be how to verify this and it doesn't seem we may be doing this for all architectures. Even for userspace we seem to be covered -- we enable userspace mmap() calls to get their mapped space with a cache type, on the kernel we'd say use pgprot_writecombine() on the vma-vm_page_prot prior to the io_remap_pfn_range() -- that maps to remap_pfn_range() on x86 and as you note that checks cache type via reserve_memtype() -- but only on x86... Other than this differing cache type concern are we OK with aliasing in userspace all the time ? If we want to restrict aliasing either for the kernel or userspace mapping we might be able to do it, I just want to know if we want to or not care at all. pmem and DAX also create mappings to the same NVDIMM ranges. DAX calls vm_insert_mixed(), which is particularly a problematic since vm_insert_mixed() does not verify aliasing. ioremap() and remap_pfn_range() call reserve_memtype() to verify aliasing on x86. reserve_memtype() is x86-specific and there is no arch-generic wrapper for such check. As clarified by Matthew Wilcox via commit d92576f1167cacf7844 (dax: does not work correctly with virtual aliasing caches) caches are virtually mapped for some architectures, it seems it should be possible to fix this for DAX somehow though. I think DAX could get a cache type from pmem to keep them in sync, though. pmem is x86 specific right now, are other folks going to expose something similar ? Otherwise we seem to only be addressing these deep concerns for x86 so far. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote: > On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux > wrote: : > > On ARM, we (probably) have a lot of cases where ioremap() is used > > multiple > > times for the same physical address space, so we shouldn't rule out > > having > > multiple mappings of the same type. > > Why is that done? Don't worry if you are not sure why but only > speculate of the > practice's existence (sloppy drivers or lazy driver developers). FWIW > for x86 > IIRC I ended up concluding that overlapping ioremap() calls with the > same type > would work but not if they differ in type. Although I haven't > written a > grammer rule to hunt down overlapping ioremap() I suspected its use > was likely > odd and likely should be reconsidered. Would this be true for ARM too > ? Or are > you saying this should be a feature ? I don't expect an answer now > but I'm > saying we *should* all together decide on this, and if you're > inclined to > believe that this should ideally be avoided I'd like to hear that. If > you feel > strongly though this should be a feature I would like to know why. There are multiple mapping interfaces, and overlapping can happen among them as well. For instance, remap_pfn_range() (and io_remap_pfn_range(), which is the same as remap_pfn_range() on x86) creates a mapping to user space. The same physical ranges may be mapped to kernel and user spaces. /dev/mem is one example that may create a user space mapping to a physical address that is already mapped with ioremap() by other module. pmem and DAX also create mappings to the same NVDIMM ranges. DAX calls vm_insert_mixed(), which is particularly a problematic since vm_insert_mixed() does not verify aliasing. ioremap() and remap_pfn_range() call reserve_memtype() to verify aliasing on x86. reserve_memtype() is x86-specific and there is no arch-generic wrapper for such check. I think DAX could get a cache type from pmem to keep them in sync, though. Thanks, -Toshi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux wrote: > On Tue, Jul 07, 2015 at 11:50:12AM +0200, Luis R. Rodriguez wrote: > > mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache > > drivers/| wc -l > > 359 > > Yes, it's because we have: > (a) LDD telling people they should be using ioremap_nocache() for mapping > devices. Sounds like LDD could use some love from ARM folks. Not a requirement if we set out on this semantics / grammar / documentation crusade appropriatley. > (b) We have documentation in the Documentation/ subdirectory telling people > to use ioremap_nocache() for the same. That obviously needs to be fixed, I take it you're on it. > > This is part of the work I've been doing lately. The > > eventual goal once we have the write-combing areas properly split with > > ioremap_wc() and using the new proper preferred architecture agnostic > > modifier > > (arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to > > use > > strong UC for PAT enabled systems for *both* ioremap() and > > ioremap_nocache(). > > Please note that on ARM, ioremap_wc() gives what's termed in ARM ARM > speak "normal memory, non-cacheable" - which can be subject to speculation, > write combining, multiple accesses, etc. The important point is that > such mapping is not suitable for device registers, but is suitable for > device regions that have "memory like" properties (iow, a chunk of RAM, > like video drivers.) It does support unaligned accesses. Thanks that helps. > > Because of these grammatical issues and the issues with > > unaligned access with ARM I think its important we put some effort > > to care a bit more about defining clear semantics through grammar > > for new APIs or as we rewrite APIs. We have tools to do this these > > days, best make use of them. > > I'm in support of anything which more clearly specifies the requirements > for these APIs. Great! > > While we're at it and reconsidering all this, a few items I wish for > > us to address as well then, most of them related to grammar, some > > procedural clarification: > > > > * Document it as not supported to have overlapping ioremap() calls. > > No one seems to have a clue if this should work, but clearly this > > is just a bad idea. I don't see why we should support the complexity > > of having this. It seems we can write grammar rules to prevent this. > > On ARM, we (probably) have a lot of cases where ioremap() is used multiple > times for the same physical address space, so we shouldn't rule out having > multiple mappings of the same type. Why is that done? Don't worry if you are not sure why but only speculate of the practice's existence (sloppy drivers or lazy driver developers). FWIW for x86 IIRC I ended up concluding that overlapping ioremap() calls with the same type would work but not if they differ in type. Although I haven't written a grammer rule to hunt down overlapping ioremap() I suspected its use was likely odd and likely should be reconsidered. Would this be true for ARM too ? Or are you saying this should be a feature ? I don't expect an answer now but I'm saying we *should* all together decide on this, and if you're inclined to believe that this should ideally be avoided I'd like to hear that. If you feel strongly though this should be a feature I would like to know why. > However, differing types would be a problem on ARM. Great. > > * We seem to care about device drivers / kernel code doing unaligned > > accesses with certain ioremap() variants. At least for ARM you should > > not do unaligned accesses on ioremap_nocache() areas. > > ... and ioremap() areas. > > If we can stop the "abuse" of ioremap_nocache() to map device registers, OK when *should* ioremap_nocache() be used then ? That is, we can easily write a rule to go and switch drivers away from ioremap_nocache() but do we have a grammatical white-list for when its intended goal was appropriate ? For x86 the goal is to use it for MMIO registers, keep in mind we'd ideally want an effective ioremap_uc() on those long term, we just can't go flipping the switch just yet unless we get all the write-combined areas right first and smake sure that we split them out. That's the effort I've been working on lately. > then we could potentially switch ioremap_nocache() to be a normal-memory > like mapping, which would allow it to support unaligned accesses. Great. Can you elaborate on why that could happen *iff* the abuse stops ? > > I am not sure > > if we can come up with grammar to vet for / warn for unaligned access > > type of code in driver code on some memory area when some ioremap() > > variant is used, but this could be looked into. I believe we may > > want rules for unaligned access maybe in general, and not attached > > to certain calls due to performance considerations, so this work > > may be welcomed regardless (refer to > >
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 7, 2015 at 12:13 PM, Russell King - ARM Linux
wrote:
> Another issue is... the use of memcpy()/memset() directly on memory
> returned from ioremap*(). The pmem driver does this. This fails sparse
> checks. However, years ago, x86 invented the memcpy_fromio()/memcpy_toio()
> memset_io() functions, which took a __iomem pointer (which /presumably/
> means they're supposed to operate on the memory associated with an
> ioremap'd region.)
>
> Should these functions always be used for mappings via ioremap*(), and
> the standard memcpy()/memset() be avoided? To me, that sounds like a
> very good thing, because that gives us more control over the
> implementation of the functions used to access ioremap'd regions,
> and the arch can decide to prevent GCC inlining its own memset() or
> memcpy() code if desired.
Yes they should. Not doing that is a typical portability bug (works on x86,
not everywhere).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 11:50:12AM +0200, Luis R. Rodriguez wrote: > mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache > drivers/| wc -l > 359 Yes, it's because we have: (a) LDD telling people they should be using ioremap_nocache() for mapping devices. (b) We have documentation in the Documentation/ subdirectory telling people to use ioremap_nocache() for the same. > This is part of the work I've been doing lately. The > eventual goal once we have the write-combing areas properly split with > ioremap_wc() and using the new proper preferred architecture agnostic modifier > (arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to use > strong UC for PAT enabled systems for *both* ioremap() and ioremap_nocache(). Please note that on ARM, ioremap_wc() gives what's termed in ARM ARM speak "normal memory, non-cacheable" - which can be subject to speculation, write combining, multiple accesses, etc. The important point is that such mapping is not suitable for device registers, but is suitable for device regions that have "memory like" properties (iow, a chunk of RAM, like video drivers.) It does support unaligned accesses. > Because of these grammatical issues and the issues with > unaligned access with ARM I think its important we put some effort > to care a bit more about defining clear semantics through grammar > for new APIs or as we rewrite APIs. We have tools to do this these > days, best make use of them. I'm in support of anything which more clearly specifies the requirements for these APIs. > While we're at it and reconsidering all this, a few items I wish for > us to address as well then, most of them related to grammar, some > procedural clarification: > > * Document it as not supported to have overlapping ioremap() calls. > No one seems to have a clue if this should work, but clearly this > is just a bad idea. I don't see why we should support the complexity > of having this. It seems we can write grammar rules to prevent this. On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. However, differing types would be a problem on ARM. > * We seem to care about device drivers / kernel code doing unaligned > accesses with certain ioremap() variants. At least for ARM you should > not do unaligned accesses on ioremap_nocache() areas. ... and ioremap() areas. If we can stop the "abuse" of ioremap_nocache() to map device registers, then we could potentially switch ioremap_nocache() to be a normal-memory like mapping, which would allow it to support unaligned accesses. > I am not sure > if we can come up with grammar to vet for / warn for unaligned access > type of code in driver code on some memory area when some ioremap() > variant is used, but this could be looked into. I believe we may > want rules for unaligned access maybe in general, and not attached > to certain calls due to performance considerations, so this work > may be welcomed regardless (refer to > Documentation/unaligned-memory-access.txt) > > * We seem to want to be pedantic about adding new ioremap() variants, the > unaligned issue on ARM is one reason, do we ideally then want *all* > architecture maintainers to provide an Acked-by for any new ioremap > variants ? /If/ we get the current mess sorted out so that we have a safe fallback, and we have understanding of the different architecture variants (iow, documented what the safe fallback is) I don't see any reason why we'd need acks from arch maintainers. Unfortunately, we're not in that situation today, because of the poorly documented mess that ioremap*() currently is (and yes, I'm partly to blame for that too by not documenting ARMs behaviour here.) I have some patches (prepared last week, I was going to push them out towards the end of the merge window) which address that, but unfortunately the ARM autobuilders have been giving a number of seemingly random boot failures, and I'm not yet sure what's going on... so I'm holding that back until stuff has settled down. Another issue is... the use of memcpy()/memset() directly on memory returned from ioremap*(). The pmem driver does this. This fails sparse checks. However, years ago, x86 invented the memcpy_fromio()/memcpy_toio() memset_io() functions, which took a __iomem pointer (which /presumably/ means they're supposed to operate on the memory associated with an ioremap'd region.) Should these functions always be used for mappings via ioremap*(), and the standard memcpy()/memset() be avoided? To me, that sounds like a very good thing, because that gives us more control over the implementation of the functions used to access ioremap'd regions, and the arch can decide to prevent GCC inlining its own memset() or memcpy() code if desired. Note that on x86, these three
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 09:28:28AM +0200, Christoph Hellwig wrote:
> On Wed, Jul 01, 2015 at 09:19:29AM +0200, Geert Uytterhoeven wrote:
> > >> So it would be the responsibility of the caller to fall back from
> > >> ioremap(..., CACHED) to ioremap(..., UNCACHED)?
> > >> I.e. all drivers using it should be changed...
> > >
> > > All of the zero users we currently have will need to be changed, yes.
> >
> > Good. Less work to convert all of these ;-)
>
> And I didn't have enough coffee yet. We of course have a few users of
> ioremap_cache(), and two implememantions but no users of ioremap_cached().
> Looks like the implementations can't even agree on the name.
Yies, that naming is icky... we also have quite a bit of ioremap_nocache()
users:
mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache drivers/|
wc -l
359
On x86 the default ioremap() happens to map to ioremap_nocache() anyway as well.
This is on purpose, there is an ongoing effort to streamline ioremap_nocache()
for registers on the x86 front with the long term goal then of making PAT
strong UC the default preference for both ioremap() and ioremap_nocache() for
PAT enabled systems. This would prevent things like write-combining modifiers
from having any effect on the area. This comes with a small architectural
driver cost, it means all write-combining desired areas must be split out in
drivers properly. This is part of the work I've been doing lately. The
eventual goal once we have the write-combing areas properly split with
ioremap_wc() and using the new proper preferred architecture agnostic modifier
(arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to use
strong UC for PAT enabled systems for *both* ioremap() and ioremap_nocache().
This was aleady done once but reverted later due to the regression issues on
video drivers not haveing the right ioremap_wc() calls. I'm finishing this
effort and am about a few patches away...
Once done and once things cool down we should go back and may consider flipping
the switch again to make strong UC default. For details refer to commit
de33c442ed2a465 ("x86 PAT: fix performance drop for glx, use UC minus
for ioremap(), ioremap_nocache() and pci_mmap_page_range()").
All this is fine in theory -- but Benjamin Herrenschmidt recently also
noted that on powerpc the write-combining may end up requiring each
register read/write with its own specific API. That is, we'd lose the
magic of having things being done behind the scenes, and that would
also mean tons of reads/writes may need to be converted over to be
explicit about write-combining preferences...
I will note that upon discussions it seems that the above requirement
may have been a slight mishap on not being explicit about our semantics
and requirements on ioremap() variants, technically it may be possible
that effectively PowerPC may not get any write-combining effects on
infiniband / networking / anything not doing write-combining on
userspace such as framebuffer... from what I gather that needs to
be fixed. Because of these grammatical issues and the issues with
unaligned access with ARM I think its important we put some effort
to care a bit more about defining clear semantics through grammar
for new APIs or as we rewrite APIs. We have tools to do this these
days, best make use of them.
While we're at it and reconsidering all this, a few items I wish for
us to address as well then, most of them related to grammar, some
procedural clarification:
* Document it as not supported to have overlapping ioremap() calls.
No one seems to have a clue if this should work, but clearly this
is just a bad idea. I don't see why we should support the complexity
of having this. It seems we can write grammar rules to prevent this.
* We seem to care about device drivers / kernel code doing unaligned
accesses with certain ioremap() variants. At least for ARM you should
not do unaligned accesses on ioremap_nocache() areas. I am not sure
if we can come up with grammar to vet for / warn for unaligned access
type of code in driver code on some memory area when some ioremap()
variant is used, but this could be looked into. I believe we may
want rules for unaligned access maybe in general, and not attached
to certain calls due to performance considerations, so this work
may be welcomed regardless (refer to
Documentation/unaligned-memory-access.txt)
* We seem to want to be pedantic about adding new ioremap() variants, the
unaligned issue on ARM is one reason, do we ideally then want *all*
architecture maintainers to provide an Acked-by for any new ioremap
variants ? Are we going to have to sit and wait for a kumbaya every time
a helper comes along to see how it all fits well for all architectures?
The asm-generic io.h seemed to have set in place the ability to let
architectures define things *when* they get to it, that seems like a much
more fair
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux wrote: On Tue, Jul 07, 2015 at 11:50:12AM +0200, Luis R. Rodriguez wrote: mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache drivers/| wc -l 359 Yes, it's because we have: (a) LDD telling people they should be using ioremap_nocache() for mapping devices. Sounds like LDD could use some love from ARM folks. Not a requirement if we set out on this semantics / grammar / documentation crusade appropriatley. (b) We have documentation in the Documentation/ subdirectory telling people to use ioremap_nocache() for the same. That obviously needs to be fixed, I take it you're on it. This is part of the work I've been doing lately. The eventual goal once we have the write-combing areas properly split with ioremap_wc() and using the new proper preferred architecture agnostic modifier (arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to use strong UC for PAT enabled systems for *both* ioremap() and ioremap_nocache(). Please note that on ARM, ioremap_wc() gives what's termed in ARM ARM speak normal memory, non-cacheable - which can be subject to speculation, write combining, multiple accesses, etc. The important point is that such mapping is not suitable for device registers, but is suitable for device regions that have memory like properties (iow, a chunk of RAM, like video drivers.) It does support unaligned accesses. Thanks that helps. Because of these grammatical issues and the issues with unaligned access with ARM I think its important we put some effort to care a bit more about defining clear semantics through grammar for new APIs or as we rewrite APIs. We have tools to do this these days, best make use of them. I'm in support of anything which more clearly specifies the requirements for these APIs. Great! While we're at it and reconsidering all this, a few items I wish for us to address as well then, most of them related to grammar, some procedural clarification: * Document it as not supported to have overlapping ioremap() calls. No one seems to have a clue if this should work, but clearly this is just a bad idea. I don't see why we should support the complexity of having this. It seems we can write grammar rules to prevent this. On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. Why is that done? Don't worry if you are not sure why but only speculate of the practice's existence (sloppy drivers or lazy driver developers). FWIW for x86 IIRC I ended up concluding that overlapping ioremap() calls with the same type would work but not if they differ in type. Although I haven't written a grammer rule to hunt down overlapping ioremap() I suspected its use was likely odd and likely should be reconsidered. Would this be true for ARM too ? Or are you saying this should be a feature ? I don't expect an answer now but I'm saying we *should* all together decide on this, and if you're inclined to believe that this should ideally be avoided I'd like to hear that. If you feel strongly though this should be a feature I would like to know why. However, differing types would be a problem on ARM. Great. * We seem to care about device drivers / kernel code doing unaligned accesses with certain ioremap() variants. At least for ARM you should not do unaligned accesses on ioremap_nocache() areas. ... and ioremap() areas. If we can stop the abuse of ioremap_nocache() to map device registers, OK when *should* ioremap_nocache() be used then ? That is, we can easily write a rule to go and switch drivers away from ioremap_nocache() but do we have a grammatical white-list for when its intended goal was appropriate ? For x86 the goal is to use it for MMIO registers, keep in mind we'd ideally want an effective ioremap_uc() on those long term, we just can't go flipping the switch just yet unless we get all the write-combined areas right first and smake sure that we split them out. That's the effort I've been working on lately. then we could potentially switch ioremap_nocache() to be a normal-memory like mapping, which would allow it to support unaligned accesses. Great. Can you elaborate on why that could happen *iff* the abuse stops ? I am not sure if we can come up with grammar to vet for / warn for unaligned access type of code in driver code on some memory area when some ioremap() variant is used, but this could be looked into. I believe we may want rules for unaligned access maybe in general, and not attached to certain calls due to performance considerations, so this work may be welcomed regardless (refer to Documentation/unaligned-memory-access.txt) * We seem to want to be pedantic about adding new
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, 2015-07-07 at 18:07 +0200, Luis R. Rodriguez wrote: On Tue, Jul 07, 2015 at 11:13:30AM +0100, Russell King - ARM Linux wrote: : On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. Why is that done? Don't worry if you are not sure why but only speculate of the practice's existence (sloppy drivers or lazy driver developers). FWIW for x86 IIRC I ended up concluding that overlapping ioremap() calls with the same type would work but not if they differ in type. Although I haven't written a grammer rule to hunt down overlapping ioremap() I suspected its use was likely odd and likely should be reconsidered. Would this be true for ARM too ? Or are you saying this should be a feature ? I don't expect an answer now but I'm saying we *should* all together decide on this, and if you're inclined to believe that this should ideally be avoided I'd like to hear that. If you feel strongly though this should be a feature I would like to know why. There are multiple mapping interfaces, and overlapping can happen among them as well. For instance, remap_pfn_range() (and io_remap_pfn_range(), which is the same as remap_pfn_range() on x86) creates a mapping to user space. The same physical ranges may be mapped to kernel and user spaces. /dev/mem is one example that may create a user space mapping to a physical address that is already mapped with ioremap() by other module. pmem and DAX also create mappings to the same NVDIMM ranges. DAX calls vm_insert_mixed(), which is particularly a problematic since vm_insert_mixed() does not verify aliasing. ioremap() and remap_pfn_range() call reserve_memtype() to verify aliasing on x86. reserve_memtype() is x86-specific and there is no arch-generic wrapper for such check. I think DAX could get a cache type from pmem to keep them in sync, though. Thanks, -Toshi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 7, 2015 at 12:13 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
Another issue is... the use of memcpy()/memset() directly on memory
returned from ioremap*(). The pmem driver does this. This fails sparse
checks. However, years ago, x86 invented the memcpy_fromio()/memcpy_toio()
memset_io() functions, which took a __iomem pointer (which /presumably/
means they're supposed to operate on the memory associated with an
ioremap'd region.)
Should these functions always be used for mappings via ioremap*(), and
the standard memcpy()/memset() be avoided? To me, that sounds like a
very good thing, because that gives us more control over the
implementation of the functions used to access ioremap'd regions,
and the arch can decide to prevent GCC inlining its own memset() or
memcpy() code if desired.
Yes they should. Not doing that is a typical portability bug (works on x86,
not everywhere).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 09:28:28AM +0200, Christoph Hellwig wrote: On Wed, Jul 01, 2015 at 09:19:29AM +0200, Geert Uytterhoeven wrote: So it would be the responsibility of the caller to fall back from ioremap(..., CACHED) to ioremap(..., UNCACHED)? I.e. all drivers using it should be changed... All of the zero users we currently have will need to be changed, yes. Good. Less work to convert all of these ;-) And I didn't have enough coffee yet. We of course have a few users of ioremap_cache(), and two implememantions but no users of ioremap_cached(). Looks like the implementations can't even agree on the name. Yies, that naming is icky... we also have quite a bit of ioremap_nocache() users: mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache drivers/| wc -l 359 On x86 the default ioremap() happens to map to ioremap_nocache() anyway as well. This is on purpose, there is an ongoing effort to streamline ioremap_nocache() for registers on the x86 front with the long term goal then of making PAT strong UC the default preference for both ioremap() and ioremap_nocache() for PAT enabled systems. This would prevent things like write-combining modifiers from having any effect on the area. This comes with a small architectural driver cost, it means all write-combining desired areas must be split out in drivers properly. This is part of the work I've been doing lately. The eventual goal once we have the write-combing areas properly split with ioremap_wc() and using the new proper preferred architecture agnostic modifier (arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to use strong UC for PAT enabled systems for *both* ioremap() and ioremap_nocache(). This was aleady done once but reverted later due to the regression issues on video drivers not haveing the right ioremap_wc() calls. I'm finishing this effort and am about a few patches away... Once done and once things cool down we should go back and may consider flipping the switch again to make strong UC default. For details refer to commit de33c442ed2a465 (x86 PAT: fix performance drop for glx, use UC minus for ioremap(), ioremap_nocache() and pci_mmap_page_range()). All this is fine in theory -- but Benjamin Herrenschmidt recently also noted that on powerpc the write-combining may end up requiring each register read/write with its own specific API. That is, we'd lose the magic of having things being done behind the scenes, and that would also mean tons of reads/writes may need to be converted over to be explicit about write-combining preferences... I will note that upon discussions it seems that the above requirement may have been a slight mishap on not being explicit about our semantics and requirements on ioremap() variants, technically it may be possible that effectively PowerPC may not get any write-combining effects on infiniband / networking / anything not doing write-combining on userspace such as framebuffer... from what I gather that needs to be fixed. Because of these grammatical issues and the issues with unaligned access with ARM I think its important we put some effort to care a bit more about defining clear semantics through grammar for new APIs or as we rewrite APIs. We have tools to do this these days, best make use of them. While we're at it and reconsidering all this, a few items I wish for us to address as well then, most of them related to grammar, some procedural clarification: * Document it as not supported to have overlapping ioremap() calls. No one seems to have a clue if this should work, but clearly this is just a bad idea. I don't see why we should support the complexity of having this. It seems we can write grammar rules to prevent this. * We seem to care about device drivers / kernel code doing unaligned accesses with certain ioremap() variants. At least for ARM you should not do unaligned accesses on ioremap_nocache() areas. I am not sure if we can come up with grammar to vet for / warn for unaligned access type of code in driver code on some memory area when some ioremap() variant is used, but this could be looked into. I believe we may want rules for unaligned access maybe in general, and not attached to certain calls due to performance considerations, so this work may be welcomed regardless (refer to Documentation/unaligned-memory-access.txt) * We seem to want to be pedantic about adding new ioremap() variants, the unaligned issue on ARM is one reason, do we ideally then want *all* architecture maintainers to provide an Acked-by for any new ioremap variants ? Are we going to have to sit and wait for a kumbaya every time a helper comes along to see how it all fits well for all architectures? The asm-generic io.h seemed to have set in place the ability to let architectures define things *when* they get to it, that seems like a much more fair approach *if* and *when
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jul 07, 2015 at 11:50:12AM +0200, Luis R. Rodriguez wrote: mcgrof@ergon ~/linux-next (git::kill-mtrr)$ git grep ioremap_nocache drivers/| wc -l 359 Yes, it's because we have: (a) LDD telling people they should be using ioremap_nocache() for mapping devices. (b) We have documentation in the Documentation/ subdirectory telling people to use ioremap_nocache() for the same. This is part of the work I've been doing lately. The eventual goal once we have the write-combing areas properly split with ioremap_wc() and using the new proper preferred architecture agnostic modifier (arch_phys_wc_add()) is to change the default ioremap behaviour on x86 to use strong UC for PAT enabled systems for *both* ioremap() and ioremap_nocache(). Please note that on ARM, ioremap_wc() gives what's termed in ARM ARM speak normal memory, non-cacheable - which can be subject to speculation, write combining, multiple accesses, etc. The important point is that such mapping is not suitable for device registers, but is suitable for device regions that have memory like properties (iow, a chunk of RAM, like video drivers.) It does support unaligned accesses. Because of these grammatical issues and the issues with unaligned access with ARM I think its important we put some effort to care a bit more about defining clear semantics through grammar for new APIs or as we rewrite APIs. We have tools to do this these days, best make use of them. I'm in support of anything which more clearly specifies the requirements for these APIs. While we're at it and reconsidering all this, a few items I wish for us to address as well then, most of them related to grammar, some procedural clarification: * Document it as not supported to have overlapping ioremap() calls. No one seems to have a clue if this should work, but clearly this is just a bad idea. I don't see why we should support the complexity of having this. It seems we can write grammar rules to prevent this. On ARM, we (probably) have a lot of cases where ioremap() is used multiple times for the same physical address space, so we shouldn't rule out having multiple mappings of the same type. However, differing types would be a problem on ARM. * We seem to care about device drivers / kernel code doing unaligned accesses with certain ioremap() variants. At least for ARM you should not do unaligned accesses on ioremap_nocache() areas. ... and ioremap() areas. If we can stop the abuse of ioremap_nocache() to map device registers, then we could potentially switch ioremap_nocache() to be a normal-memory like mapping, which would allow it to support unaligned accesses. I am not sure if we can come up with grammar to vet for / warn for unaligned access type of code in driver code on some memory area when some ioremap() variant is used, but this could be looked into. I believe we may want rules for unaligned access maybe in general, and not attached to certain calls due to performance considerations, so this work may be welcomed regardless (refer to Documentation/unaligned-memory-access.txt) * We seem to want to be pedantic about adding new ioremap() variants, the unaligned issue on ARM is one reason, do we ideally then want *all* architecture maintainers to provide an Acked-by for any new ioremap variants ? /If/ we get the current mess sorted out so that we have a safe fallback, and we have understanding of the different architecture variants (iow, documented what the safe fallback is) I don't see any reason why we'd need acks from arch maintainers. Unfortunately, we're not in that situation today, because of the poorly documented mess that ioremap*() currently is (and yes, I'm partly to blame for that too by not documenting ARMs behaviour here.) I have some patches (prepared last week, I was going to push them out towards the end of the merge window) which address that, but unfortunately the ARM autobuilders have been giving a number of seemingly random boot failures, and I'm not yet sure what's going on... so I'm holding that back until stuff has settled down. Another issue is... the use of memcpy()/memset() directly on memory returned from ioremap*(). The pmem driver does this. This fails sparse checks. However, years ago, x86 invented the memcpy_fromio()/memcpy_toio() memset_io() functions, which took a __iomem pointer (which /presumably/ means they're supposed to operate on the memory associated with an ioremap'd region.) Should these functions always be used for mappings via ioremap*(), and the standard memcpy()/memset() be avoided? To me, that sounds like a very good thing, because that gives us more control over the implementation of the functions used to access ioremap'd regions, and the arch can decide to prevent GCC inlining its own memset() or memcpy() code if desired. Note that on x86, these three functions are merely wrappers around standard
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 1:09 AM, Russell King - ARM Linux wrote: > On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: >> On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig wrote: >> >> One useful feature of the ifdef mess as implemented in the patch is >> >> that you could test for whether ioremap_cache() is actually >> >> implemented or falls back to default ioremap(). I think for >> >> completeness archs should publish an ioremap type capabilities mask >> >> for drivers that care... (I can imagine pmem caring), or default to >> >> being permissive if something like IOREMAP_STRICT is not set. There's >> >> also the wrinkle of archs that can only support certain types of >> >> mappings at a given alignment. >> > >> > I think doing this at runtime might be a better idea. E.g. a >> > ioremap_flags with the CACHED argument will return -EOPNOTSUP unless >> > actually implemented. On various architectures different CPUs or >> > boards will have different capabilities in this area. >> >> So it would be the responsibility of the caller to fall back from >> ioremap(..., CACHED) to ioremap(..., UNCACHED)? >> I.e. all drivers using it should be changed... > > Another important point here is to define what the properties of the > mappings are. It's no good just saying "uncached". > > We've recently been around this over the PMEM driver and the broken > addition of ioremap_wt() on ARM... > > By "properties" I mean stuff like whether unaligned accesses permitted, > any kind of atomic access (eg, xchg, cmpxchg, etc). > > This matters: on ARM, a mapping suitable for a device does not support > unaligned accesses or atomic accesses - only "memory-like" mappings > support those. However, memory-like mappings are not required to > preserve access size, number of accesses, etc which makes them unsuitable > for device registers. I'm proposing that we explicitly switch "memory-like" use cases over to a separate set of "memremap()" apis, as these are no longer "__iomem" [1]. > The problem with ioremap_uncached() in particular is that we have LDD > and other documentation telling people to use it to map device registers, > so we can't define ioremap_uncached() on ARM to have memory-like > properties, and it doesn't support unaligned accesses. > > I have a series of patches which fix up 32-bit ARM for the broken > ioremap_wt() stuff that was merged during this merge window, which I > intend to push out into linux-next at some point (possibly during the > merge window, if not after -rc1) which also move ioremap*() out of line > on ARM but more importantly, adds a load of documentation about the > properties of the resulting mapping on ARM. Sounds good, I'll look for that before proceeding on this clean up. [1]: https://lists.01.org/pipermail/linux-nvdimm/2015-June/001331.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: > On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig wrote: > >> One useful feature of the ifdef mess as implemented in the patch is > >> that you could test for whether ioremap_cache() is actually > >> implemented or falls back to default ioremap(). I think for > >> completeness archs should publish an ioremap type capabilities mask > >> for drivers that care... (I can imagine pmem caring), or default to > >> being permissive if something like IOREMAP_STRICT is not set. There's > >> also the wrinkle of archs that can only support certain types of > >> mappings at a given alignment. > > > > I think doing this at runtime might be a better idea. E.g. a > > ioremap_flags with the CACHED argument will return -EOPNOTSUP unless > > actually implemented. On various architectures different CPUs or > > boards will have different capabilities in this area. > > So it would be the responsibility of the caller to fall back from > ioremap(..., CACHED) to ioremap(..., UNCACHED)? > I.e. all drivers using it should be changed... Another important point here is to define what the properties of the mappings are. It's no good just saying "uncached". We've recently been around this over the PMEM driver and the broken addition of ioremap_wt() on ARM... By "properties" I mean stuff like whether unaligned accesses permitted, any kind of atomic access (eg, xchg, cmpxchg, etc). This matters: on ARM, a mapping suitable for a device does not support unaligned accesses or atomic accesses - only "memory-like" mappings support those. However, memory-like mappings are not required to preserve access size, number of accesses, etc which makes them unsuitable for device registers. The problem with ioremap_uncached() in particular is that we have LDD and other documentation telling people to use it to map device registers, so we can't define ioremap_uncached() on ARM to have memory-like properties, and it doesn't support unaligned accesses. I have a series of patches which fix up 32-bit ARM for the broken ioremap_wt() stuff that was merged during this merge window, which I intend to push out into linux-next at some point (possibly during the merge window, if not after -rc1) which also move ioremap*() out of line on ARM but more importantly, adds a load of documentation about the properties of the resulting mapping on ARM. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 09:19:29AM +0200, Geert Uytterhoeven wrote: > >> So it would be the responsibility of the caller to fall back from > >> ioremap(..., CACHED) to ioremap(..., UNCACHED)? > >> I.e. all drivers using it should be changed... > > > > All of the zero users we currently have will need to be changed, yes. > > Good. Less work to convert all of these ;-) And I didn't have enough coffee yet. We of course have a few users of ioremap_cache(), and two implememantions but no users of ioremap_cached(). Looks like the implementations can't even agree on the name. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 8:59 AM, Christoph Hellwig wrote:
> On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote:
>> >
>> > I think doing this at runtime might be a better idea. E.g. a
>> > ioremap_flags with the CACHED argument will return -EOPNOTSUP unless
>> > actually implemented. On various architectures different CPUs or
>> > boards will have different capabilities in this area.
>>
>> So it would be the responsibility of the caller to fall back from
>> ioremap(..., CACHED) to ioremap(..., UNCACHED)?
>> I.e. all drivers using it should be changed...
>
> All of the zero users we currently have will need to be changed, yes.
Good. Less work to convert all of these ;-)
> Note that I propose to leave ioremap(), aka ioremap_flags(..., 0) as
> a default that always has to work, -EOPNOTSUP is only a valid return
> value for non-default flaga.
OK.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: > > > > I think doing this at runtime might be a better idea. E.g. a > > ioremap_flags with the CACHED argument will return -EOPNOTSUP unless > > actually implemented. On various architectures different CPUs or > > boards will have different capabilities in this area. > > So it would be the responsibility of the caller to fall back from > ioremap(..., CACHED) to ioremap(..., UNCACHED)? > I.e. all drivers using it should be changed... All of the zero users we currently have will need to be changed, yes. Note that I propose to leave ioremap(), aka ioremap_flags(..., 0) as a default that always has to work, -EOPNOTSUP is only a valid return value for non-default flaga. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig wrote:
>> One useful feature of the ifdef mess as implemented in the patch is
>> that you could test for whether ioremap_cache() is actually
>> implemented or falls back to default ioremap(). I think for
>> completeness archs should publish an ioremap type capabilities mask
>> for drivers that care... (I can imagine pmem caring), or default to
>> being permissive if something like IOREMAP_STRICT is not set. There's
>> also the wrinkle of archs that can only support certain types of
>> mappings at a given alignment.
>
> I think doing this at runtime might be a better idea. E.g. a
> ioremap_flags with the CACHED argument will return -EOPNOTSUP unless
> actually implemented. On various architectures different CPUs or
> boards will have different capabilities in this area.
So it would be the responsibility of the caller to fall back from
ioremap(..., CACHED) to ioremap(..., UNCACHED)?
I.e. all drivers using it should be changed...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 30, 2015 at 03:57:16PM -0700, Dan Williams wrote: > > void __iomem *ioremap_flags(resource_size_t offset, unsigned long size, > > unsigned long prot_val, unsigned flags); > > Doesn't 'flags' imply a specific 'prot_val'? Looks like the values are arch specific. So as a first step I'd like to keep them separate. As a second step we could look into unifying the actual ioremap implementations which look mostly the same. Once that is done we could look into collapsing the flags and prot_val arguments. > One useful feature of the ifdef mess as implemented in the patch is > that you could test for whether ioremap_cache() is actually > implemented or falls back to default ioremap(). I think for > completeness archs should publish an ioremap type capabilities mask > for drivers that care... (I can imagine pmem caring), or default to > being permissive if something like IOREMAP_STRICT is not set. There's > also the wrinkle of archs that can only support certain types of > mappings at a given alignment. I think doing this at runtime might be a better idea. E.g. a ioremap_flags with the CACHED argument will return -EOPNOTSUP unless actually implemented. On various architectures different CPUs or boards will have different capabilities in this area. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 1:09 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig h...@lst.de wrote: One useful feature of the ifdef mess as implemented in the patch is that you could test for whether ioremap_cache() is actually implemented or falls back to default ioremap(). I think for completeness archs should publish an ioremap type capabilities mask for drivers that care... (I can imagine pmem caring), or default to being permissive if something like IOREMAP_STRICT is not set. There's also the wrinkle of archs that can only support certain types of mappings at a given alignment. I think doing this at runtime might be a better idea. E.g. a ioremap_flags with the CACHED argument will return -EOPNOTSUP unless actually implemented. On various architectures different CPUs or boards will have different capabilities in this area. So it would be the responsibility of the caller to fall back from ioremap(..., CACHED) to ioremap(..., UNCACHED)? I.e. all drivers using it should be changed... Another important point here is to define what the properties of the mappings are. It's no good just saying uncached. We've recently been around this over the PMEM driver and the broken addition of ioremap_wt() on ARM... By properties I mean stuff like whether unaligned accesses permitted, any kind of atomic access (eg, xchg, cmpxchg, etc). This matters: on ARM, a mapping suitable for a device does not support unaligned accesses or atomic accesses - only memory-like mappings support those. However, memory-like mappings are not required to preserve access size, number of accesses, etc which makes them unsuitable for device registers. I'm proposing that we explicitly switch memory-like use cases over to a separate set of memremap() apis, as these are no longer __iomem [1]. The problem with ioremap_uncached() in particular is that we have LDD and other documentation telling people to use it to map device registers, so we can't define ioremap_uncached() on ARM to have memory-like properties, and it doesn't support unaligned accesses. I have a series of patches which fix up 32-bit ARM for the broken ioremap_wt() stuff that was merged during this merge window, which I intend to push out into linux-next at some point (possibly during the merge window, if not after -rc1) which also move ioremap*() out of line on ARM but more importantly, adds a load of documentation about the properties of the resulting mapping on ARM. Sounds good, I'll look for that before proceeding on this clean up. [1]: https://lists.01.org/pipermail/linux-nvdimm/2015-June/001331.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 30, 2015 at 03:57:16PM -0700, Dan Williams wrote: void __iomem *ioremap_flags(resource_size_t offset, unsigned long size, unsigned long prot_val, unsigned flags); Doesn't 'flags' imply a specific 'prot_val'? Looks like the values are arch specific. So as a first step I'd like to keep them separate. As a second step we could look into unifying the actual ioremap implementations which look mostly the same. Once that is done we could look into collapsing the flags and prot_val arguments. One useful feature of the ifdef mess as implemented in the patch is that you could test for whether ioremap_cache() is actually implemented or falls back to default ioremap(). I think for completeness archs should publish an ioremap type capabilities mask for drivers that care... (I can imagine pmem caring), or default to being permissive if something like IOREMAP_STRICT is not set. There's also the wrinkle of archs that can only support certain types of mappings at a given alignment. I think doing this at runtime might be a better idea. E.g. a ioremap_flags with the CACHED argument will return -EOPNOTSUP unless actually implemented. On various architectures different CPUs or boards will have different capabilities in this area. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: I think doing this at runtime might be a better idea. E.g. a ioremap_flags with the CACHED argument will return -EOPNOTSUP unless actually implemented. On various architectures different CPUs or boards will have different capabilities in this area. So it would be the responsibility of the caller to fall back from ioremap(..., CACHED) to ioremap(..., UNCACHED)? I.e. all drivers using it should be changed... All of the zero users we currently have will need to be changed, yes. Note that I propose to leave ioremap(), aka ioremap_flags(..., 0) as a default that always has to work, -EOPNOTSUP is only a valid return value for non-default flaga. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 8:59 AM, Christoph Hellwig h...@lst.de wrote:
On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote:
I think doing this at runtime might be a better idea. E.g. a
ioremap_flags with the CACHED argument will return -EOPNOTSUP unless
actually implemented. On various architectures different CPUs or
boards will have different capabilities in this area.
So it would be the responsibility of the caller to fall back from
ioremap(..., CACHED) to ioremap(..., UNCACHED)?
I.e. all drivers using it should be changed...
All of the zero users we currently have will need to be changed, yes.
Good. Less work to convert all of these ;-)
Note that I propose to leave ioremap(), aka ioremap_flags(..., 0) as
a default that always has to work, -EOPNOTSUP is only a valid return
value for non-default flaga.
OK.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 09:19:29AM +0200, Geert Uytterhoeven wrote: So it would be the responsibility of the caller to fall back from ioremap(..., CACHED) to ioremap(..., UNCACHED)? I.e. all drivers using it should be changed... All of the zero users we currently have will need to be changed, yes. Good. Less work to convert all of these ;-) And I didn't have enough coffee yet. We of course have a few users of ioremap_cache(), and two implememantions but no users of ioremap_cached(). Looks like the implementations can't even agree on the name. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 01, 2015 at 08:55:57AM +0200, Geert Uytterhoeven wrote: On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig h...@lst.de wrote: One useful feature of the ifdef mess as implemented in the patch is that you could test for whether ioremap_cache() is actually implemented or falls back to default ioremap(). I think for completeness archs should publish an ioremap type capabilities mask for drivers that care... (I can imagine pmem caring), or default to being permissive if something like IOREMAP_STRICT is not set. There's also the wrinkle of archs that can only support certain types of mappings at a given alignment. I think doing this at runtime might be a better idea. E.g. a ioremap_flags with the CACHED argument will return -EOPNOTSUP unless actually implemented. On various architectures different CPUs or boards will have different capabilities in this area. So it would be the responsibility of the caller to fall back from ioremap(..., CACHED) to ioremap(..., UNCACHED)? I.e. all drivers using it should be changed... Another important point here is to define what the properties of the mappings are. It's no good just saying uncached. We've recently been around this over the PMEM driver and the broken addition of ioremap_wt() on ARM... By properties I mean stuff like whether unaligned accesses permitted, any kind of atomic access (eg, xchg, cmpxchg, etc). This matters: on ARM, a mapping suitable for a device does not support unaligned accesses or atomic accesses - only memory-like mappings support those. However, memory-like mappings are not required to preserve access size, number of accesses, etc which makes them unsuitable for device registers. The problem with ioremap_uncached() in particular is that we have LDD and other documentation telling people to use it to map device registers, so we can't define ioremap_uncached() on ARM to have memory-like properties, and it doesn't support unaligned accesses. I have a series of patches which fix up 32-bit ARM for the broken ioremap_wt() stuff that was merged during this merge window, which I intend to push out into linux-next at some point (possibly during the merge window, if not after -rc1) which also move ioremap*() out of line on ARM but more importantly, adds a load of documentation about the properties of the resulting mapping on ARM. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Wed, Jul 1, 2015 at 8:23 AM, Christoph Hellwig h...@lst.de wrote:
One useful feature of the ifdef mess as implemented in the patch is
that you could test for whether ioremap_cache() is actually
implemented or falls back to default ioremap(). I think for
completeness archs should publish an ioremap type capabilities mask
for drivers that care... (I can imagine pmem caring), or default to
being permissive if something like IOREMAP_STRICT is not set. There's
also the wrinkle of archs that can only support certain types of
mappings at a given alignment.
I think doing this at runtime might be a better idea. E.g. a
ioremap_flags with the CACHED argument will return -EOPNOTSUP unless
actually implemented. On various architectures different CPUs or
boards will have different capabilities in this area.
So it would be the responsibility of the caller to fall back from
ioremap(..., CACHED) to ioremap(..., UNCACHED)?
I.e. all drivers using it should be changed...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 9:10 AM, Christoph Hellwig wrote:
> On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote:
>> Some archs define the first parameter to ioremap() as unsigned long,
>> while the balance define it as resource_size_t. Unify on
>> resource_size_t to enable passing ioremap function pointers. Also, some
>> archs use function-like macros for defining ioremap aliases, but
>> asm-generic/io.h expects object-like macros, unify on the latter.
>>
>> Move all handling of ioremap aliasing (i.e. ioremap_wt => ioremap) to
>> include/linux/io.h. Add a check to include/linux/io.h to warn at
>> compile time if an arch violates expectations.
>>
>> Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
>> testing for ioremap_wc, and ioremap_wt being defined. This arrangement
>> allows drivers to know when ioremap_ are being re-directed to plain
>> ioremap.
>>
>> Reported-by: kbuild test robot
>> Signed-off-by: Dan Williams
>
> Hmm, this is quite a bit of churn, and doesn't make the interface lot
> more obvious.
>
> I guess it's enough to get the pmem related bits going, but I'd really
> prefer defining the ioremap* prototype in linux/io.h and requiring
> and out of line implementation in the architectures, it's not like
> it's a fast path. And to avoid the ifdef mess make it something like:
>
> void __iomem *ioremap_flags(resource_size_t offset, unsigned long size,
> unsigned long prot_val, unsigned flags);
Doesn't 'flags' imply a specific 'prot_val'?
> static inline void __iomem *ioremap(resource_size_t offset, unsigned long
> size)
> {
> return ioremap_flags(offset, size, 0, 0);
> }
>
> static inline void __iomem *ioremap_prot(resource_size_t offset,
> unsigned long size, unsigned long prot_val)
> {
> return ioremap_flags(offset, size, prot_val, 0);
> }
>
> static inline void __iomem *ioremap_nocache(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_flags(offset, size, 0, IOREMAP_NOCACHE);
> }
>
> static inline void __iomem *ioremap_cache(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_flags(offset, size, 0, IOREMAP_CACHE);
> }
>
> static inline void __iomem *ioremap_uc(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_flags(offset, size, 0, IOREMAP_UC);
> }
>
> static inline void __iomem *ioremap_wc(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_flags(offset, size, 0, IOREMAP_WC);
> }
>
> static inline void __iomem *ioremap_wt(resource_size_t offset,
> unsigned long size)
> {
> return ioremap_flags(offset, size, 0, IOREMAP_WT);
> }
>
> With all wrappers but ioremap() itself deprecated in the long run.
>
> Besides following the one API one prototype guideline this gives
> us one proper entry point for all the variants. Additionally
> it can reject non-supported caching modes at run time, e.g. because
> different hardware may or may not support it. Additionally it
> avoids the need for all these HAVE_IOREMAP_FOO defines, which need
> constant updating.
One useful feature of the ifdef mess as implemented in the patch is
that you could test for whether ioremap_cache() is actually
implemented or falls back to default ioremap(). I think for
completeness archs should publish an ioremap type capabilities mask
for drivers that care... (I can imagine pmem caring), or default to
being permissive if something like IOREMAP_STRICT is not set. There's
also the wrinkle of archs that can only support certain types of
mappings at a given alignment.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 9:10 AM, Christoph Hellwig h...@lst.de wrote:
On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote:
Some archs define the first parameter to ioremap() as unsigned long,
while the balance define it as resource_size_t. Unify on
resource_size_t to enable passing ioremap function pointers. Also, some
archs use function-like macros for defining ioremap aliases, but
asm-generic/io.h expects object-like macros, unify on the latter.
Move all handling of ioremap aliasing (i.e. ioremap_wt = ioremap) to
include/linux/io.h. Add a check to include/linux/io.h to warn at
compile time if an arch violates expectations.
Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
testing for ioremap_wc, and ioremap_wt being defined. This arrangement
allows drivers to know when ioremap_foo are being re-directed to plain
ioremap.
Reported-by: kbuild test robot fengguang...@intel.com
Signed-off-by: Dan Williams dan.j.willi...@intel.com
Hmm, this is quite a bit of churn, and doesn't make the interface lot
more obvious.
I guess it's enough to get the pmem related bits going, but I'd really
prefer defining the ioremap* prototype in linux/io.h and requiring
and out of line implementation in the architectures, it's not like
it's a fast path. And to avoid the ifdef mess make it something like:
void __iomem *ioremap_flags(resource_size_t offset, unsigned long size,
unsigned long prot_val, unsigned flags);
Doesn't 'flags' imply a specific 'prot_val'?
static inline void __iomem *ioremap(resource_size_t offset, unsigned long
size)
{
return ioremap_flags(offset, size, 0, 0);
}
static inline void __iomem *ioremap_prot(resource_size_t offset,
unsigned long size, unsigned long prot_val)
{
return ioremap_flags(offset, size, prot_val, 0);
}
static inline void __iomem *ioremap_nocache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_NOCACHE);
}
static inline void __iomem *ioremap_cache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_CACHE);
}
static inline void __iomem *ioremap_uc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_UC);
}
static inline void __iomem *ioremap_wc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WC);
}
static inline void __iomem *ioremap_wt(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WT);
}
With all wrappers but ioremap() itself deprecated in the long run.
Besides following the one API one prototype guideline this gives
us one proper entry point for all the variants. Additionally
it can reject non-supported caching modes at run time, e.g. because
different hardware may or may not support it. Additionally it
avoids the need for all these HAVE_IOREMAP_FOO defines, which need
constant updating.
One useful feature of the ifdef mess as implemented in the patch is
that you could test for whether ioremap_cache() is actually
implemented or falls back to default ioremap(). I think for
completeness archs should publish an ioremap type capabilities mask
for drivers that care... (I can imagine pmem caring), or default to
being permissive if something like IOREMAP_STRICT is not set. There's
also the wrinkle of archs that can only support certain types of
mappings at a given alignment.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 23, 2015 at 08:04:47AM -0700, Dan Williams wrote: > Thanks, definitely a long shot at this point, but this is what one > gets for fixing rather than working around broken base infrastructure. > It would be unfortunate if we went another cycle with pmem having both > poor performance and broken persistence guarantees. Maybe we can aim for the minimal fix for 4.2 that just adds meremap and the accessors for x86 and the do the big scale cleanups for 4.3? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 23, 2015 at 08:04:47AM -0700, Dan Williams wrote: Thanks, definitely a long shot at this point, but this is what one gets for fixing rather than working around broken base infrastructure. It would be unfortunate if we went another cycle with pmem having both poor performance and broken persistence guarantees. Maybe we can aim for the minimal fix for 4.2 that just adds meremap and the accessors for x86 and the do the big scale cleanups for 4.3? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 23, 2015 at 3:07 AM, Christoph Hellwig wrote: > On Mon, Jun 22, 2015 at 10:12:40AM -0700, Dan Williams wrote: >> Is that an acked-by for this cycle with a request to go deeper for 4.3? > > I wouldn't really expect something this wide reaching to be picked up > for this cycle, but if you manage to get it in: > > Acked-by: Christoph Hellwig Thanks, definitely a long shot at this point, but this is what one gets for fixing rather than working around broken base infrastructure. It would be unfortunate if we went another cycle with pmem having both poor performance and broken persistence guarantees. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 10:12:40AM -0700, Dan Williams wrote: > Is that an acked-by for this cycle with a request to go deeper for 4.3? I wouldn't really expect something this wide reaching to be picked up for this cycle, but if you manage to get it in: Acked-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Tue, Jun 23, 2015 at 3:07 AM, Christoph Hellwig h...@lst.de wrote: On Mon, Jun 22, 2015 at 10:12:40AM -0700, Dan Williams wrote: Is that an acked-by for this cycle with a request to go deeper for 4.3? I wouldn't really expect something this wide reaching to be picked up for this cycle, but if you manage to get it in: Acked-by: Christoph Hellwig h...@lst.de Thanks, definitely a long shot at this point, but this is what one gets for fixing rather than working around broken base infrastructure. It would be unfortunate if we went another cycle with pmem having both poor performance and broken persistence guarantees. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 10:12:40AM -0700, Dan Williams wrote: Is that an acked-by for this cycle with a request to go deeper for 4.3? I wouldn't really expect something this wide reaching to be picked up for this cycle, but if you manage to get it in: Acked-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 9:10 AM, Christoph Hellwig wrote: > On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote: >> Some archs define the first parameter to ioremap() as unsigned long, >> while the balance define it as resource_size_t. Unify on >> resource_size_t to enable passing ioremap function pointers. Also, some >> archs use function-like macros for defining ioremap aliases, but >> asm-generic/io.h expects object-like macros, unify on the latter. >> >> Move all handling of ioremap aliasing (i.e. ioremap_wt => ioremap) to >> include/linux/io.h. Add a check to include/linux/io.h to warn at >> compile time if an arch violates expectations. >> >> Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just >> testing for ioremap_wc, and ioremap_wt being defined. This arrangement >> allows drivers to know when ioremap_ are being re-directed to plain >> ioremap. >> >> Reported-by: kbuild test robot >> Signed-off-by: Dan Williams > > Hmm, this is quite a bit of churn, and doesn't make the interface lot > more obvious. > > I guess it's enough to get the pmem related bits going Is that an acked-by for this cycle with a request to go deeper for 4.3? > but I'd really > prefer defining the ioremap* prototype in linux/io.h and requiring > and out of line implementation in the architectures, it's not like > it's a fast path. And to avoid the ifdef mess make it something like: > > void __iomem *ioremap_flags(resource_size_t offset, unsigned long size, > unsigned long prot_val, unsigned flags); Yes, I do like this even better. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote:
> Some archs define the first parameter to ioremap() as unsigned long,
> while the balance define it as resource_size_t. Unify on
> resource_size_t to enable passing ioremap function pointers. Also, some
> archs use function-like macros for defining ioremap aliases, but
> asm-generic/io.h expects object-like macros, unify on the latter.
>
> Move all handling of ioremap aliasing (i.e. ioremap_wt => ioremap) to
> include/linux/io.h. Add a check to include/linux/io.h to warn at
> compile time if an arch violates expectations.
>
> Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
> testing for ioremap_wc, and ioremap_wt being defined. This arrangement
> allows drivers to know when ioremap_ are being re-directed to plain
> ioremap.
>
> Reported-by: kbuild test robot
> Signed-off-by: Dan Williams
Hmm, this is quite a bit of churn, and doesn't make the interface lot
more obvious.
I guess it's enough to get the pmem related bits going, but I'd really
prefer defining the ioremap* prototype in linux/io.h and requiring
and out of line implementation in the architectures, it's not like
it's a fast path. And to avoid the ifdef mess make it something like:
void __iomem *ioremap_flags(resource_size_t offset, unsigned long size,
unsigned long prot_val, unsigned flags);
static inline void __iomem *ioremap(resource_size_t offset, unsigned long size)
{
return ioremap_flags(offset, size, 0, 0);
}
static inline void __iomem *ioremap_prot(resource_size_t offset,
unsigned long size, unsigned long prot_val)
{
return ioremap_flags(offset, size, prot_val, 0);
}
static inline void __iomem *ioremap_nocache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_NOCACHE);
}
static inline void __iomem *ioremap_cache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_CACHE);
}
static inline void __iomem *ioremap_uc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_UC);
}
static inline void __iomem *ioremap_wc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WC);
}
static inline void __iomem *ioremap_wt(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WT);
}
With all wrappers but ioremap() itself deprecated in the long run.
Besides following the one API one prototype guideline this gives
us one proper entry point for all the variants. Additionally
it can reject non-supported caching modes at run time, e.g. because
different hardware may or may not support it. Additionally it
avoids the need for all these HAVE_IOREMAP_FOO defines, which need
constant updating.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
Some archs define the first parameter to ioremap() as unsigned long,
while the balance define it as resource_size_t. Unify on
resource_size_t to enable passing ioremap function pointers. Also, some
archs use function-like macros for defining ioremap aliases, but
asm-generic/io.h expects object-like macros, unify on the latter.
Move all handling of ioremap aliasing (i.e. ioremap_wt => ioremap) to
include/linux/io.h. Add a check to include/linux/io.h to warn at
compile time if an arch violates expectations.
Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
testing for ioremap_wc, and ioremap_wt being defined. This arrangement
allows drivers to know when ioremap_ are being re-directed to plain
ioremap.
Reported-by: kbuild test robot
Signed-off-by: Dan Williams
---
arch/alpha/include/asm/io.h |7 +++--
arch/arc/include/asm/io.h|6
arch/arm/include/asm/io.h| 31
arch/arm64/include/asm/io.h | 23 ---
arch/avr32/include/asm/io.h | 22 +++---
arch/avr32/mm/ioremap.c |2 +
arch/cris/include/asm/io.h |8 +++--
arch/cris/mm/ioremap.c |2 +
arch/frv/include/asm/io.h| 23 +++
arch/hexagon/include/asm/io.h|5 +++
arch/ia64/include/asm/io.h | 10 +--
arch/ia64/mm/ioremap.c |4 +--
arch/m32r/include/asm/io.h |9 +++---
arch/m68k/include/asm/io_mm.h| 21 +-
arch/m68k/include/asm/io_no.h| 34 ++
arch/m68k/include/asm/raw_io.h |3 +-
arch/m68k/mm/kmap.c |2 +
arch/metag/include/asm/io.h | 35 ++-
arch/microblaze/include/asm/io.h |6
arch/microblaze/mm/pgtable.c |2 +
arch/mips/include/asm/io.h | 42 ++--
arch/mn10300/include/asm/io.h| 10 ---
arch/nios2/include/asm/io.h | 15 +++---
arch/openrisc/include/asm/io.h |3 +-
arch/openrisc/mm/ioremap.c |2 +
arch/parisc/include/asm/io.h |6 +---
arch/parisc/mm/ioremap.c |2 +
arch/powerpc/include/asm/io.h|7 ++---
arch/s390/include/asm/io.h |8 ++---
arch/sh/include/asm/io.h |9 +-
arch/sparc/include/asm/io_32.h |7 +
arch/sparc/include/asm/io_64.h |8 ++---
arch/sparc/kernel/ioport.c |2 +
arch/tile/include/asm/io.h | 17 +++
arch/unicore32/include/asm/io.h | 25 +---
arch/x86/include/asm/io.h|9 --
arch/xtensa/include/asm/io.h | 13 +
drivers/net/ethernet/sfc/io.h|2 +
include/asm-generic/iomap.h |8 -
include/linux/io.h | 58 ++
40 files changed, 300 insertions(+), 208 deletions(-)
diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index f05bdb4b1cb9..6b63689c27a6 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -282,10 +282,11 @@ extern inline void ioport_unmap(void __iomem *addr)
{
}
-static inline void __iomem *ioremap(unsigned long port, unsigned long size)
+static inline void __iomem *ioremap(resource_size_t port, unsigned long size)
{
return IO_CONCAT(__IO_PREFIX,ioremap) (port, size);
}
+#define ioremap ioremap
static inline void __iomem *__ioremap(unsigned long port, unsigned long size,
unsigned long flags)
@@ -293,16 +294,18 @@ static inline void __iomem *__ioremap(unsigned long port,
unsigned long size,
return ioremap(port, size);
}
-static inline void __iomem * ioremap_nocache(unsigned long offset,
+static inline void __iomem * ioremap_nocache(resource_size_t offset,
unsigned long size)
{
return ioremap(offset, size);
}
+#define ioremap_nocache ioremap_nocache
static inline void iounmap(volatile void __iomem *addr)
{
IO_CONCAT(__IO_PREFIX,iounmap)(addr);
}
+#define iounmap iounmap
static inline int __is_ioaddr(unsigned long addr)
{
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 7cc4ced5dbf4..e6f782863026 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -13,14 +13,8 @@
#include
#include
-extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
unsigned long flags);
-extern void iounmap(const void __iomem *addr);
-
-#define ioremap_nocache(phy, sz) ioremap(phy, sz)
-#define ioremap_wc(phy, sz)ioremap(phy, sz)
-#define ioremap_wt(phy, sz)ioremap(phy, sz)
/* Change struct page to physical address */
#define page_to_phys(page) (page_to_pfn(page) << PAGE_SHIFT)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 1b7677d1e5e1..ecd410c716d8
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote:
Some archs define the first parameter to ioremap() as unsigned long,
while the balance define it as resource_size_t. Unify on
resource_size_t to enable passing ioremap function pointers. Also, some
archs use function-like macros for defining ioremap aliases, but
asm-generic/io.h expects object-like macros, unify on the latter.
Move all handling of ioremap aliasing (i.e. ioremap_wt = ioremap) to
include/linux/io.h. Add a check to include/linux/io.h to warn at
compile time if an arch violates expectations.
Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
testing for ioremap_wc, and ioremap_wt being defined. This arrangement
allows drivers to know when ioremap_foo are being re-directed to plain
ioremap.
Reported-by: kbuild test robot fengguang...@intel.com
Signed-off-by: Dan Williams dan.j.willi...@intel.com
Hmm, this is quite a bit of churn, and doesn't make the interface lot
more obvious.
I guess it's enough to get the pmem related bits going, but I'd really
prefer defining the ioremap* prototype in linux/io.h and requiring
and out of line implementation in the architectures, it's not like
it's a fast path. And to avoid the ifdef mess make it something like:
void __iomem *ioremap_flags(resource_size_t offset, unsigned long size,
unsigned long prot_val, unsigned flags);
static inline void __iomem *ioremap(resource_size_t offset, unsigned long size)
{
return ioremap_flags(offset, size, 0, 0);
}
static inline void __iomem *ioremap_prot(resource_size_t offset,
unsigned long size, unsigned long prot_val)
{
return ioremap_flags(offset, size, prot_val, 0);
}
static inline void __iomem *ioremap_nocache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_NOCACHE);
}
static inline void __iomem *ioremap_cache(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_CACHE);
}
static inline void __iomem *ioremap_uc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_UC);
}
static inline void __iomem *ioremap_wc(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WC);
}
static inline void __iomem *ioremap_wt(resource_size_t offset,
unsigned long size)
{
return ioremap_flags(offset, size, 0, IOREMAP_WT);
}
With all wrappers but ioremap() itself deprecated in the long run.
Besides following the one API one prototype guideline this gives
us one proper entry point for all the variants. Additionally
it can reject non-supported caching modes at run time, e.g. because
different hardware may or may not support it. Additionally it
avoids the need for all these HAVE_IOREMAP_FOO defines, which need
constant updating.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
On Mon, Jun 22, 2015 at 9:10 AM, Christoph Hellwig h...@lst.de wrote: On Mon, Jun 22, 2015 at 04:24:27AM -0400, Dan Williams wrote: Some archs define the first parameter to ioremap() as unsigned long, while the balance define it as resource_size_t. Unify on resource_size_t to enable passing ioremap function pointers. Also, some archs use function-like macros for defining ioremap aliases, but asm-generic/io.h expects object-like macros, unify on the latter. Move all handling of ioremap aliasing (i.e. ioremap_wt = ioremap) to include/linux/io.h. Add a check to include/linux/io.h to warn at compile time if an arch violates expectations. Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just testing for ioremap_wc, and ioremap_wt being defined. This arrangement allows drivers to know when ioremap_foo are being re-directed to plain ioremap. Reported-by: kbuild test robot fengguang...@intel.com Signed-off-by: Dan Williams dan.j.willi...@intel.com Hmm, this is quite a bit of churn, and doesn't make the interface lot more obvious. I guess it's enough to get the pmem related bits going Is that an acked-by for this cycle with a request to go deeper for 4.3? but I'd really prefer defining the ioremap* prototype in linux/io.h and requiring and out of line implementation in the architectures, it's not like it's a fast path. And to avoid the ifdef mess make it something like: void __iomem *ioremap_flags(resource_size_t offset, unsigned long size, unsigned long prot_val, unsigned flags); Yes, I do like this even better. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 2/6] arch: unify ioremap prototypes and macro aliases
Some archs define the first parameter to ioremap() as unsigned long,
while the balance define it as resource_size_t. Unify on
resource_size_t to enable passing ioremap function pointers. Also, some
archs use function-like macros for defining ioremap aliases, but
asm-generic/io.h expects object-like macros, unify on the latter.
Move all handling of ioremap aliasing (i.e. ioremap_wt = ioremap) to
include/linux/io.h. Add a check to include/linux/io.h to warn at
compile time if an arch violates expectations.
Kill ARCH_HAS_IOREMAP_WC and ARCH_HAS_IOREMAP_WT in favor of just
testing for ioremap_wc, and ioremap_wt being defined. This arrangement
allows drivers to know when ioremap_foo are being re-directed to plain
ioremap.
Reported-by: kbuild test robot fengguang...@intel.com
Signed-off-by: Dan Williams dan.j.willi...@intel.com
---
arch/alpha/include/asm/io.h |7 +++--
arch/arc/include/asm/io.h|6
arch/arm/include/asm/io.h| 31
arch/arm64/include/asm/io.h | 23 ---
arch/avr32/include/asm/io.h | 22 +++---
arch/avr32/mm/ioremap.c |2 +
arch/cris/include/asm/io.h |8 +++--
arch/cris/mm/ioremap.c |2 +
arch/frv/include/asm/io.h| 23 +++
arch/hexagon/include/asm/io.h|5 +++
arch/ia64/include/asm/io.h | 10 +--
arch/ia64/mm/ioremap.c |4 +--
arch/m32r/include/asm/io.h |9 +++---
arch/m68k/include/asm/io_mm.h| 21 +-
arch/m68k/include/asm/io_no.h| 34 ++
arch/m68k/include/asm/raw_io.h |3 +-
arch/m68k/mm/kmap.c |2 +
arch/metag/include/asm/io.h | 35 ++-
arch/microblaze/include/asm/io.h |6
arch/microblaze/mm/pgtable.c |2 +
arch/mips/include/asm/io.h | 42 ++--
arch/mn10300/include/asm/io.h| 10 ---
arch/nios2/include/asm/io.h | 15 +++---
arch/openrisc/include/asm/io.h |3 +-
arch/openrisc/mm/ioremap.c |2 +
arch/parisc/include/asm/io.h |6 +---
arch/parisc/mm/ioremap.c |2 +
arch/powerpc/include/asm/io.h|7 ++---
arch/s390/include/asm/io.h |8 ++---
arch/sh/include/asm/io.h |9 +-
arch/sparc/include/asm/io_32.h |7 +
arch/sparc/include/asm/io_64.h |8 ++---
arch/sparc/kernel/ioport.c |2 +
arch/tile/include/asm/io.h | 17 +++
arch/unicore32/include/asm/io.h | 25 +---
arch/x86/include/asm/io.h|9 --
arch/xtensa/include/asm/io.h | 13 +
drivers/net/ethernet/sfc/io.h|2 +
include/asm-generic/iomap.h |8 -
include/linux/io.h | 58 ++
40 files changed, 300 insertions(+), 208 deletions(-)
diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index f05bdb4b1cb9..6b63689c27a6 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -282,10 +282,11 @@ extern inline void ioport_unmap(void __iomem *addr)
{
}
-static inline void __iomem *ioremap(unsigned long port, unsigned long size)
+static inline void __iomem *ioremap(resource_size_t port, unsigned long size)
{
return IO_CONCAT(__IO_PREFIX,ioremap) (port, size);
}
+#define ioremap ioremap
static inline void __iomem *__ioremap(unsigned long port, unsigned long size,
unsigned long flags)
@@ -293,16 +294,18 @@ static inline void __iomem *__ioremap(unsigned long port,
unsigned long size,
return ioremap(port, size);
}
-static inline void __iomem * ioremap_nocache(unsigned long offset,
+static inline void __iomem * ioremap_nocache(resource_size_t offset,
unsigned long size)
{
return ioremap(offset, size);
}
+#define ioremap_nocache ioremap_nocache
static inline void iounmap(volatile void __iomem *addr)
{
IO_CONCAT(__IO_PREFIX,iounmap)(addr);
}
+#define iounmap iounmap
static inline int __is_ioaddr(unsigned long addr)
{
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 7cc4ced5dbf4..e6f782863026 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -13,14 +13,8 @@
#include asm/byteorder.h
#include asm/page.h
-extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
unsigned long flags);
-extern void iounmap(const void __iomem *addr);
-
-#define ioremap_nocache(phy, sz) ioremap(phy, sz)
-#define ioremap_wc(phy, sz)ioremap(phy, sz)
-#define ioremap_wt(phy, sz)ioremap(phy, sz)
/* Change struct page to physical address */
#define page_to_phys(page) (page_to_pfn(page) PAGE_SHIFT)
diff --git
