[PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[Paul Burton]

Use CPHYSADDR to implement the __pa macro converting from a virtual to a
physical address for MIPS32, much as is already done for MIPS64 (though
without the complication of having both compatibility & XKPHYS
segments).

This allows for __pa to work regardless of whether the address being
translated is in kseg0 or kseg1, unlike the previous subtraction based
approach which only worked for addresses in kseg0. Working for kseg1
addresses is important if __pa is used on addresses allocated by
dma_alloc_coherent, where on systems with non-coherent I/O we provide
addresses in kseg1. If this address is then used with
dma_map_single_attrs then it is provided to virt_to_page, which in turn
calls virt_to_phys which is a wrapper around __pa. The result is that we
end up with a physical address 0x20000000 bytes (ie. the size of kseg0)
too high.

In addition to providing consistency with MIPS64 & fixing the kseg1 case
above this has the added bonus of generating smaller code for systems
implementing MIPS32r2 & beyond, where a single ext instruction can
extract the physical address rather than needing to load an immediate
into a temp register & subtract it. This results in ~1.3KB savings for a
boston_defconfig kernel adjusted to set CONFIG_32BIT=y.

This patch does not change the EVA case, which may or may not have
similar issues around handling both cached & uncached addresses but is
beyond the scope of this patch.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

Changes in v2:
- Leave the EVA case as-is.

 arch/mips/include/asm/page.h | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 21ed715..ac0c1b7 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -169,8 +169,24 @@ typedef struct { unsigned long pgprot; } pgprot_t;
     __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
 })
 #else
-#define __pa(x)								\
-    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
+static inline unsigned long __pa(unsigned long x)
+{
+	if (!config_enabled(CONFIG_EVA)) {
+		/*
+		 * We're using the standard MIPS32 legacy memory map, ie.
+		 * the address x is going to be in kseg0 or kseg1. We can
+		 * handle either case by masking out the desired bits using
+		 * CPHYSADDR.
+		 */
+		return CPHYSADDR(x);
+	}
+
+	/*
+	 * EVA is in use so the memory map could be anything, making it not
+	 * safe to just mask out bits.
+	 */
+	return x - PAGE_OFFSET + PHYS_OFFSET;
+}
 #endif
 #define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
 #include <asm/io.h>
-- 
2.7.1

[PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[Paul Burton]

Use CPHYSADDR to implement the __pa macro converting from a virtual to a
physical address for MIPS32, much as is already done for MIPS64 (though
without the complication of having both compatibility & XKPHYS
segments).

This allows for __pa to work regardless of whether the address being
translated is in kseg0 or kseg1, unlike the previous subtraction based
approach which only worked for addresses in kseg0. Working for kseg1
addresses is important if __pa is used on addresses allocated by
dma_alloc_coherent, where on systems with non-coherent I/O we provide
addresses in kseg1. If this address is then used with
dma_map_single_attrs then it is provided to virt_to_page, which in turn
calls virt_to_phys which is a wrapper around __pa. The result is that we
end up with a physical address 0x20000000 bytes (ie. the size of kseg0)
too high.

In addition to providing consistency with MIPS64 & fixing the kseg1 case
above this has the added bonus of generating smaller code for systems
implementing MIPS32r2 & beyond, where a single ext instruction can
extract the physical address rather than needing to load an immediate
into a temp register & subtract it. This results in ~1.3KB savings for a
boston_defconfig kernel adjusted to set CONFIG_32BIT=y.

This patch does not change the EVA case, which may or may not have
similar issues around handling both cached & uncached addresses but is
beyond the scope of this patch.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

Changes in v2:
- Leave the EVA case as-is.

 arch/mips/include/asm/page.h | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 21ed715..ac0c1b7 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -169,8 +169,24 @@ typedef struct { unsigned long pgprot; } pgprot_t;
     __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
 })
 #else
-#define __pa(x)								\
-    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
+static inline unsigned long __pa(unsigned long x)
+{
+	if (!config_enabled(CONFIG_EVA)) {
+		/*
+		 * We're using the standard MIPS32 legacy memory map, ie.
+		 * the address x is going to be in kseg0 or kseg1. We can
+		 * handle either case by masking out the desired bits using
+		 * CPHYSADDR.
+		 */
+		return CPHYSADDR(x);
+	}
+
+	/*
+	 * EVA is in use so the memory map could be anything, making it not
+	 * safe to just mask out bits.
+	 */
+	return x - PAGE_OFFSET + PHYS_OFFSET;
+}
 #endif
 #define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
 #include <asm/io.h>
-- 
2.7.1

Re: [PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[Paul Burton]

On 16/02/16 17:41, Paul Burton wrote:
> Use CPHYSADDR to implement the __pa macro converting from a virtual to a
> physical address for MIPS32, much as is already done for MIPS64 (though
> without the complication of having both compatibility & XKPHYS
> segments).
>
> This allows for __pa to work regardless of whether the address being
> translated is in kseg0 or kseg1, unlike the previous subtraction based
> approach which only worked for addresses in kseg0. Working for kseg1
> addresses is important if __pa is used on addresses allocated by
> dma_alloc_coherent, where on systems with non-coherent I/O we provide
> addresses in kseg1. If this address is then used with
> dma_map_single_attrs then it is provided to virt_to_page, which in turn
> calls virt_to_phys which is a wrapper around __pa. The result is that we
> end up with a physical address 0x20000000 bytes (ie. the size of kseg0)
> too high.
>
> In addition to providing consistency with MIPS64 & fixing the kseg1 case
> above this has the added bonus of generating smaller code for systems
> implementing MIPS32r2 & beyond, where a single ext instruction can
> extract the physical address rather than needing to load an immediate
> into a temp register & subtract it. This results in ~1.3KB savings for a
> boston_defconfig kernel adjusted to set CONFIG_32BIT=y.
>
> This patch does not change the EVA case, which may or may not have
> similar issues around handling both cached & uncached addresses but is
> beyond the scope of this patch.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>

Hi Ralf,

Any thoughts on this one? It matters for Boston (where it affects the 
pch_gbe ethernet driver) which I'll be submitting again for the 4.9 cycle.

Thanks,
     Paul

> ---
>
> Changes in v2:
> - Leave the EVA case as-is.
>
>  arch/mips/include/asm/page.h | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index 21ed715..ac0c1b7 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -169,8 +169,24 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>      __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
>  })
>  #else
> -#define __pa(x)								\
> -    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> +static inline unsigned long __pa(unsigned long x)
> +{
> +	if (!config_enabled(CONFIG_EVA)) {
> +		/*
> +		 * We're using the standard MIPS32 legacy memory map, ie.
> +		 * the address x is going to be in kseg0 or kseg1. We can
> +		 * handle either case by masking out the desired bits using
> +		 * CPHYSADDR.
> +		 */
> +		return CPHYSADDR(x);
> +	}
> +
> +	/*
> +	 * EVA is in use so the memory map could be anything, making it not
> +	 * safe to just mask out bits.
> +	 */
> +	return x - PAGE_OFFSET + PHYS_OFFSET;
> +}
>  #endif
>  #define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
>  #include <asm/io.h>
>

Re: [PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[Paul Burton]

On 16/02/16 17:41, Paul Burton wrote:
> Use CPHYSADDR to implement the __pa macro converting from a virtual to a
> physical address for MIPS32, much as is already done for MIPS64 (though
> without the complication of having both compatibility & XKPHYS
> segments).
>
> This allows for __pa to work regardless of whether the address being
> translated is in kseg0 or kseg1, unlike the previous subtraction based
> approach which only worked for addresses in kseg0. Working for kseg1
> addresses is important if __pa is used on addresses allocated by
> dma_alloc_coherent, where on systems with non-coherent I/O we provide
> addresses in kseg1. If this address is then used with
> dma_map_single_attrs then it is provided to virt_to_page, which in turn
> calls virt_to_phys which is a wrapper around __pa. The result is that we
> end up with a physical address 0x20000000 bytes (ie. the size of kseg0)
> too high.
>
> In addition to providing consistency with MIPS64 & fixing the kseg1 case
> above this has the added bonus of generating smaller code for systems
> implementing MIPS32r2 & beyond, where a single ext instruction can
> extract the physical address rather than needing to load an immediate
> into a temp register & subtract it. This results in ~1.3KB savings for a
> boston_defconfig kernel adjusted to set CONFIG_32BIT=y.
>
> This patch does not change the EVA case, which may or may not have
> similar issues around handling both cached & uncached addresses but is
> beyond the scope of this patch.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>

Hi Ralf,

Any thoughts on this one? It matters for Boston (where it affects the 
pch_gbe ethernet driver) which I'll be submitting again for the 4.9 cycle.

Thanks,
     Paul

> ---
>
> Changes in v2:
> - Leave the EVA case as-is.
>
>  arch/mips/include/asm/page.h | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index 21ed715..ac0c1b7 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -169,8 +169,24 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>      __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
>  })
>  #else
> -#define __pa(x)								\
> -    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> +static inline unsigned long __pa(unsigned long x)
> +{
> +	if (!config_enabled(CONFIG_EVA)) {
> +		/*
> +		 * We're using the standard MIPS32 legacy memory map, ie.
> +		 * the address x is going to be in kseg0 or kseg1. We can
> +		 * handle either case by masking out the desired bits using
> +		 * CPHYSADDR.
> +		 */
> +		return CPHYSADDR(x);
> +	}
> +
> +	/*
> +	 * EVA is in use so the memory map could be anything, making it not
> +	 * safe to just mask out bits.
> +	 */
> +	return x - PAGE_OFFSET + PHYS_OFFSET;
> +}
>  #endif
>  #define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
>  #include <asm/io.h>
>

Re: [PATCH v2] MIPS: Use CPHYSADDR to implement mips32 __pa

[Ralf Baechle]

On Mon, Aug 01, 2016 at 05:51:10PM +0100, Paul Burton wrote:

> Any thoughts on this one? It matters for Boston (where it affects the
> pch_gbe ethernet driver) which I'll be submitting again for the 4.9 cycle.

I'm sorry.  I meant to leave this one for a while for people to comment
but then it fell through the cracks.  Applied.

Thanks!

  Ralf