[PATCH] arm64/xen: redefine xen_remap on arm64

[PATCH] arm64/xen: redefine xen_remap on arm64

[Chen Baozi]

xen_remap used to be defined as ioremap_cached on arm64. In commit
c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
was deleted, while xen_remap stays the same. This would lead to
the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
on arm64 as ioremap_cache on arm64 to fix it.

Signed-off-by: Chen Baozi <baozich@gmail.com>
---
 arch/arm/include/asm/xen/page.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 75579a9..b3368df 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -117,6 +117,10 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
 	return __set_phys_to_machine(pfn, mfn);
 }
 
+#ifdef CONFIG_ARM64
+#define xen_remap(cookie, size) ioremap_cache((cookie), (size))
+#else
 #define xen_remap(cookie, size) ioremap_cached((cookie), (size));
+#endif
 
 #endif /* _ASM_ARM_XEN_PAGE_H */
-- 
1.8.4.3

[Xen-devel] [PATCH] arm64/xen: redefine xen_remap on arm64

[Wei Liu]

On Mon, Dec 30, 2013 at 6:55 AM, Chen Baozi <baozich@gmail.com> wrote:
> xen_remap used to be defined as ioremap_cached on arm64. In commit
> c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> was deleted, while xen_remap stays the same. This would lead to
> the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> on arm64 as ioremap_cache on arm64 to fix it.
>
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  arch/arm/include/asm/xen/page.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> index 75579a9..b3368df 100644
> --- a/arch/arm/include/asm/xen/page.h
> +++ b/arch/arm/include/asm/xen/page.h
> @@ -117,6 +117,10 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
>         return __set_phys_to_machine(pfn, mfn);
>  }
>
> +#ifdef CONFIG_ARM64
> +#define xen_remap(cookie, size) ioremap_cache((cookie), (size))
> +#else
>  #define xen_remap(cookie, size) ioremap_cached((cookie), (size));

Looks like there's an redundant semicolon in the original implementation.

Wei.

> +#endif
>
>  #endif /* _ASM_ARM_XEN_PAGE_H */
> --
> 1.8.4.3
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel at lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH] arm64/xen: redefine xen_remap on arm64

[Mark Salter]

On Mon, 2013-12-30 at 14:55 +0800, Chen Baozi wrote:
> xen_remap used to be defined as ioremap_cached on arm64. In commit
> c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> was deleted, while xen_remap stays the same. This would lead to
> the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> on arm64 as ioremap_cache on arm64 to fix it.
> 

I missed that include of arm header by arm64 when looking for users
of arm64's ioremap_cached() when working on commit c04e8e2fe. Anyway,
grepping the kernel tree, I see:

  ioremap_cached()
    defined by: arm, metag, unicore32
    used by: arch/arm/include/asm/xen/page.h
             drivers/mtd/maps/pxa2xx-flash.c

  ioremap_cache()
    defined by: arm64, sh, xtensa, ia64, x86
    used by: drivers/video/vesafb.c
             drivers/char/toshiba.c
             drivers/acpi/apei
             drivers/lguest/lguest_device.c
             drivers/sfi/sfi_core.c
             include/linux/acpi_io.h

I think it would be better to just avoid the confusion and the ifdef in
asm/xen/page.h by globally changing ioremap_cached to ioremap_cache.

--Mark

[PATCH] arm64/xen: redefine xen_remap on arm64

[Stefano Stabellini]

On Mon, 30 Dec 2013, Mark Salter wrote:
> On Mon, 2013-12-30 at 14:55 +0800, Chen Baozi wrote:
> > xen_remap used to be defined as ioremap_cached on arm64. In commit
> > c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> > was deleted, while xen_remap stays the same. This would lead to
> > the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> > on arm64 as ioremap_cache on arm64 to fix it.
> > 
> 
> I missed that include of arm header by arm64 when looking for users
> of arm64's ioremap_cached() when working on commit c04e8e2fe. Anyway,
> grepping the kernel tree, I see:
> 
>   ioremap_cached()
>     defined by: arm, metag, unicore32
>     used by: arch/arm/include/asm/xen/page.h
>              drivers/mtd/maps/pxa2xx-flash.c
> 
>   ioremap_cache()
>     defined by: arm64, sh, xtensa, ia64, x86
>     used by: drivers/video/vesafb.c
>              drivers/char/toshiba.c
>              drivers/acpi/apei
>              drivers/lguest/lguest_device.c
>              drivers/sfi/sfi_core.c
>              include/linux/acpi_io.h
> 
> I think it would be better to just avoid the confusion and the ifdef in
> asm/xen/page.h by globally changing ioremap_cached to ioremap_cache.

While I welcome the suggestion, this is a critical fix for a regression
that I think should go in as soon as possible, maybe 3.13-rc7, while I
don't think that a global s/ioremap_cached/ioremap_cache would be
acceptable at this stage.

[Xen-devel] [PATCH] arm64/xen: redefine xen_remap on arm64

[Stefano Stabellini]

On Mon, 30 Dec 2013, Wei Liu wrote:
> On Mon, Dec 30, 2013 at 6:55 AM, Chen Baozi <baozich@gmail.com> wrote:
> > xen_remap used to be defined as ioremap_cached on arm64. In commit
> > c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> > was deleted, while xen_remap stays the same. This would lead to
> > the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> > on arm64 as ioremap_cache on arm64 to fix it.
> >
> > Signed-off-by: Chen Baozi <baozich@gmail.com>
> > ---
> >  arch/arm/include/asm/xen/page.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> > index 75579a9..b3368df 100644
> > --- a/arch/arm/include/asm/xen/page.h
> > +++ b/arch/arm/include/asm/xen/page.h
> > @@ -117,6 +117,10 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
> >         return __set_phys_to_machine(pfn, mfn);
> >  }
> >
> > +#ifdef CONFIG_ARM64
> > +#define xen_remap(cookie, size) ioremap_cache((cookie), (size))
> > +#else
> >  #define xen_remap(cookie, size) ioremap_cached((cookie), (size));
> 
> Looks like there's an redundant semicolon in the original implementation.

Well spotted. We can fix this separately.
Do you want to send a patch for it?

[PATCH] arm64/xen: redefine xen_remap on arm64

[Stefano Stabellini]

On Fri, 3 Jan 2014, Stefano Stabellini wrote:
> On Mon, 30 Dec 2013, Mark Salter wrote:
> > On Mon, 2013-12-30 at 14:55 +0800, Chen Baozi wrote:
> > > xen_remap used to be defined as ioremap_cached on arm64. In commit
> > > c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> > > was deleted, while xen_remap stays the same. This would lead to
> > > the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> > > on arm64 as ioremap_cache on arm64 to fix it.
> > > 
> > 
> > I missed that include of arm header by arm64 when looking for users
> > of arm64's ioremap_cached() when working on commit c04e8e2fe. Anyway,
> > grepping the kernel tree, I see:
> > 
> >   ioremap_cached()
> >     defined by: arm, metag, unicore32
> >     used by: arch/arm/include/asm/xen/page.h
> >              drivers/mtd/maps/pxa2xx-flash.c
> > 
> >   ioremap_cache()
> >     defined by: arm64, sh, xtensa, ia64, x86
> >     used by: drivers/video/vesafb.c
> >              drivers/char/toshiba.c
> >              drivers/acpi/apei
> >              drivers/lguest/lguest_device.c
> >              drivers/sfi/sfi_core.c
> >              include/linux/acpi_io.h
> > 
> > I think it would be better to just avoid the confusion and the ifdef in
> > asm/xen/page.h by globally changing ioremap_cached to ioremap_cache.
> 
> While I welcome the suggestion, this is a critical fix for a regression
> that I think should go in as soon as possible, maybe 3.13-rc7, while I
> don't think that a global s/ioremap_cached/ioremap_cache would be
> acceptable at this stage.

BTW Rob Herring sent a patch in November to solve the problem by
renaming ioremap_cached to ioremap_cache under arch/arm:

http://marc.info/?l=linux-arm-kernel&m=138394601804783

I wrongly assumed that was going to go in.
I think it is too late for that now. We'll have to go this this patch
for 3.13.

[PATCH] arm64/xen: redefine xen_remap on arm64

[Russell King - ARM Linux]

On Fri, Jan 03, 2014 at 01:13:57PM +0000, Stefano Stabellini wrote:
> On Mon, 30 Dec 2013, Mark Salter wrote:
> > On Mon, 2013-12-30 at 14:55 +0800, Chen Baozi wrote:
> > > xen_remap used to be defined as ioremap_cached on arm64. In commit
> > > c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> > > was deleted, while xen_remap stays the same. This would lead to
> > > the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> > > on arm64 as ioremap_cache on arm64 to fix it.
> > > 
> > 
> > I missed that include of arm header by arm64 when looking for users
> > of arm64's ioremap_cached() when working on commit c04e8e2fe. Anyway,
> > grepping the kernel tree, I see:
> > 
> >   ioremap_cached()
> >     defined by: arm, metag, unicore32
> >     used by: arch/arm/include/asm/xen/page.h
> >              drivers/mtd/maps/pxa2xx-flash.c
> > 
> >   ioremap_cache()
> >     defined by: arm64, sh, xtensa, ia64, x86
> >     used by: drivers/video/vesafb.c
> >              drivers/char/toshiba.c
> >              drivers/acpi/apei
> >              drivers/lguest/lguest_device.c
> >              drivers/sfi/sfi_core.c
> >              include/linux/acpi_io.h
> > 
> > I think it would be better to just avoid the confusion and the ifdef in
> > asm/xen/page.h by globally changing ioremap_cached to ioremap_cache.
> 
> While I welcome the suggestion, this is a critical fix for a regression
> that I think should go in as soon as possible, maybe 3.13-rc7, while I
> don't think that a global s/ioremap_cached/ioremap_cache would be
> acceptable at this stage.

Since it's just one driver, just make the change for ARM (provided the
grep is accurate.)  pxa2xx-flash is only used on ARM and not the other
two listed there, so looks like metag and unicore just decided to copy
ARM.

My grep concurs with yours.

So... just change ioremap_cached -> ioremap_cache in
arch/arm/include/asm/io.h
arch/arm/include/asm/xen/page.h
drivers/mtd/maps/pxa2xx-flash.c

to fix the problem.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[PATCH] arm64/xen: redefine xen_remap on arm64

[Stefano Stabellini]

On Fri, 3 Jan 2014, Russell King - ARM Linux wrote:
> On Fri, Jan 03, 2014 at 01:13:57PM +0000, Stefano Stabellini wrote:
> > On Mon, 30 Dec 2013, Mark Salter wrote:
> > > On Mon, 2013-12-30 at 14:55 +0800, Chen Baozi wrote:
> > > > xen_remap used to be defined as ioremap_cached on arm64. In commit
> > > > c04e8e2fe, a new ioremap_cache was implemented, and ioremap_cached
> > > > was deleted, while xen_remap stays the same. This would lead to
> > > > the failure when building with CONFIG_HVC_XEN. Redefined xen_remap
> > > > on arm64 as ioremap_cache on arm64 to fix it.
> > > > 
> > > 
> > > I missed that include of arm header by arm64 when looking for users
> > > of arm64's ioremap_cached() when working on commit c04e8e2fe. Anyway,
> > > grepping the kernel tree, I see:
> > > 
> > >   ioremap_cached()
> > >     defined by: arm, metag, unicore32
> > >     used by: arch/arm/include/asm/xen/page.h
> > >              drivers/mtd/maps/pxa2xx-flash.c
> > > 
> > >   ioremap_cache()
> > >     defined by: arm64, sh, xtensa, ia64, x86
> > >     used by: drivers/video/vesafb.c
> > >              drivers/char/toshiba.c
> > >              drivers/acpi/apei
> > >              drivers/lguest/lguest_device.c
> > >              drivers/sfi/sfi_core.c
> > >              include/linux/acpi_io.h
> > > 
> > > I think it would be better to just avoid the confusion and the ifdef in
> > > asm/xen/page.h by globally changing ioremap_cached to ioremap_cache.
> > 
> > While I welcome the suggestion, this is a critical fix for a regression
> > that I think should go in as soon as possible, maybe 3.13-rc7, while I
> > don't think that a global s/ioremap_cached/ioremap_cache would be
> > acceptable at this stage.
> 
> Since it's just one driver, just make the change for ARM (provided the
> grep is accurate.)  pxa2xx-flash is only used on ARM and not the other
> two listed there, so looks like metag and unicore just decided to copy
> ARM.
> 
> My grep concurs with yours.
> 
> So... just change ioremap_cached -> ioremap_cache in
> arch/arm/include/asm/io.h
> arch/arm/include/asm/xen/page.h
> drivers/mtd/maps/pxa2xx-flash.c
> 
> to fix the problem.

OK. That would be Rob's patch below.
Are you going to take care of sending it to Linus?

Thanks for the quick reply.

[PATCH] arm64/xen: redefine xen_remap on arm64

[Russell King - ARM Linux]

On Fri, Jan 03, 2014 at 02:49:54PM +0000, Stefano Stabellini wrote:
> OK. That would be Rob's patch below.

Looks fine to me.

> Are you going to take care of sending it to Linus?

If you want to stick it in the patch system, I'll throw it in my tree.
I need to push out the next set of fixes soon anyway.

Thanks.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

[PATCH] arm64/xen: redefine xen_remap on arm64

[Stefano Stabellini]

On Fri, 3 Jan 2014, Russell King - ARM Linux wrote:
> On Fri, Jan 03, 2014 at 02:49:54PM +0000, Stefano Stabellini wrote:
> > OK. That would be Rob's patch below.
> 
> Looks fine to me.
> 
> > Are you going to take care of sending it to Linus?
> 
> If you want to stick it in the patch system, I'll throw it in my tree.
> I need to push out the next set of fixes soon anyway.
> 
> Thanks.

Done. My first time with the patch system, please be lenient :-)