thr3ads.net - Xen devel - [PATCH V2] xen/arm: UART addresses are not always aligned to a page. [Oct 2013]

If this information is useful, please help other people find it:
Share via:

Jon Fraser

2013-Oct-30 21:18 UTC

[PATCH V2] xen/arm: UART addresses are not always aligned to a page.

When calculating the virtual address of the UART,
add the page offset of the UART to the mapped address.

Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
of FIXMAP_ADDR(FIXMAP_CONSOLE)

Signed-off-by: Jon Fraser <jfraser@broadcom.com>
---
 xen/arch/arm/arm32/debug.S         |  5 +++--
 xen/arch/arm/arm32/head.S          |  3 ++-
 xen/arch/arm/arm64/debug.S         |  5 +++--
 xen/arch/arm/arm64/head.S          |  3 ++-
 xen/include/asm-arm/early_printk.h | 10 ++++++++++
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
index ec774cd..a95d9da 100644
--- a/xen/arch/arm/arm32/debug.S
+++ b/xen/arch/arm/arm32/debug.S
@@ -18,6 +18,7 @@
  */
 
 #include <asm/config.h>
+#include <asm/early_printk.h>
 
 #ifdef EARLY_PRINTK_INC
 #include EARLY_PRINTK_INC
@@ -26,14 +27,14 @@
 /* Print a character on the UART - this function is called by C
  * r0: character to print */
 GLOBAL(early_putch)
-        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base address
*/
+        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base address */
         early_uart_ready r1, r2
         early_uart_transmit r1, r0
         mov   pc, lr
 
 /* Flush the UART - this function is called by C */
 GLOBAL(early_flush)
-        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base address
*/
+        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base address */
         early_uart_ready r1, r2
         mov   pc, lr
 
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index ce1d21a..96230ac 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -22,6 +22,7 @@
 #include <asm/processor-ca15.h>
 #include <asm/processor-ca7.h>
 #include <asm/asm_defns.h>
+#include <asm/early_printk.h>
 
 #define ZIMAGE_MAGIC_NUMBER 0x016f2818
 
@@ -356,7 +357,7 @@ paging:
         strd  r2, r3, [r1, r4]       /* Map it in the fixmap''s slot */
 
         /* Use a virtual address to access the UART. */
-        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
 #endif
         /* Map the DTB in the boot misc slot */
         teq   r12, #0                /* Only on boot CPU */
diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
index 472c157..a855358 100644
--- a/xen/arch/arm/arm64/debug.S
+++ b/xen/arch/arm/arm64/debug.S
@@ -18,6 +18,7 @@
  */
 
 #include <asm/config.h>
+#include <asm/early_printk.h>
 
 #ifdef EARLY_PRINTK_INC
 #include EARLY_PRINTK_INC
@@ -26,14 +27,14 @@
 /* Print a character on the UART - this function is called by C
  * x0: character to print */
 GLOBAL(early_putch)
-        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
         early_uart_ready x15, 1
         early_uart_transmit x15, w0
         ret
 
 /* Flush the UART - this function is called by C */
 GLOBAL(early_flush)
-        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* x15 := VA UART base address
*/
+        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA UART base address
*/
         early_uart_ready x15, 1
         ret
 
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index b8b5902..31afdd0 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -23,6 +23,7 @@
 #include <asm/config.h>
 #include <asm/page.h>
 #include <asm/asm_defns.h>
+#include <asm/early_printk.h>
 
 #define PT_PT     0xe7f /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=1 P=1 */
 #define PT_MEM    0xe7d /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=0 P=1 */
@@ -368,7 +369,7 @@ paging:
         str   x2, [x4, x1]           /* Map it in the fixmap''s slot */
 
         /* Use a virtual address to access the UART. */
-        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
 #endif
 
         /* Map the DTB in the boot misc slot */
diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
index 5f7c529..759d59a 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -12,6 +12,14 @@
 
 #include <xen/config.h>
 
+#ifdef __ASSEMBLY__
+
+/* need to add the uart address offset in page to the fixmap address */
+#define EARLY_UART_VIRTUAL_ADDRESS \
+    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
+
+#else
+
 #ifdef EARLY_PRINTK
 
 void early_printk(const char *fmt, ...)
@@ -31,4 +39,6 @@ __attribute__((format (printf, 1, 2))) early_panic(const char
*fmt, ...)
 
 #endif
 
+#endif  /* __ASSEMBLY__ */
+
 #endif
-- 
1.7.11.3

Andrew Cooper

2013-Oct-30 21:25 UTC

head link

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

On 30/10/13 21:18, Jon Fraser wrote:> When calculating the virtual address of the UART,
> add the page offset of the UART to the mapped address.
>
> Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
> of FIXMAP_ADDR(FIXMAP_CONSOLE)
>
> Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> ---
>  xen/arch/arm/arm32/debug.S         |  5 +++--
>  xen/arch/arm/arm32/head.S          |  3 ++-
>  xen/arch/arm/arm64/debug.S         |  5 +++--
>  xen/arch/arm/arm64/head.S          |  3 ++-
>  xen/include/asm-arm/early_printk.h | 10 ++++++++++
>  5 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
> index ec774cd..a95d9da 100644
> --- a/xen/arch/arm/arm32/debug.S
> +++ b/xen/arch/arm/arm32/debug.S
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <asm/config.h>
> +#include <asm/early_printk.h>
>  
>  #ifdef EARLY_PRINTK_INC
>  #include EARLY_PRINTK_INC
> @@ -26,14 +27,14 @@
>  /* Print a character on the UART - this function is called by C
>   * r0: character to print */
>  GLOBAL(early_putch)
> -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base
address */
> +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base
address */
>          early_uart_ready r1, r2
>          early_uart_transmit r1, r0
>          mov   pc, lr
>  
>  /* Flush the UART - this function is called by C */
>  GLOBAL(early_flush)
> -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base
address */
> +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base
address */
>          early_uart_ready r1, r2
>          mov   pc, lr
>  
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index ce1d21a..96230ac 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -22,6 +22,7 @@
>  #include <asm/processor-ca15.h>
>  #include <asm/processor-ca7.h>
>  #include <asm/asm_defns.h>
> +#include <asm/early_printk.h>
>  
>  #define ZIMAGE_MAGIC_NUMBER 0x016f2818
>  
> @@ -356,7 +357,7 @@ paging:
>          strd  r2, r3, [r1, r4]       /* Map it in the fixmap''s
slot */
>  
>          /* Use a virtual address to access the UART. */
> -        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> +        ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
>          /* Map the DTB in the boot misc slot */
>          teq   r12, #0                /* Only on boot CPU */
> diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
> index 472c157..a855358 100644
> --- a/xen/arch/arm/arm64/debug.S
> +++ b/xen/arch/arm/arm64/debug.S
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <asm/config.h>
> +#include <asm/early_printk.h>
>  
>  #ifdef EARLY_PRINTK_INC
>  #include EARLY_PRINTK_INC
> @@ -26,14 +27,14 @@
>  /* Print a character on the UART - this function is called by C
>   * x0: character to print */
>  GLOBAL(early_putch)
> -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
>          early_uart_ready x15, 1
>          early_uart_transmit x15, w0
>          ret
>  
>  /* Flush the UART - this function is called by C */
>  GLOBAL(early_flush)
> -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* x15 := VA UART base
address */
> +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA UART base
address */
>          early_uart_ready x15, 1
>          ret
>  
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index b8b5902..31afdd0 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -23,6 +23,7 @@
>  #include <asm/config.h>
>  #include <asm/page.h>
>  #include <asm/asm_defns.h>
> +#include <asm/early_printk.h>
>  
>  #define PT_PT     0xe7f /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=1 P=1 */
>  #define PT_MEM    0xe7d /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=0 P=1 */
> @@ -368,7 +369,7 @@ paging:
>          str   x2, [x4, x1]           /* Map it in the fixmap''s
slot */
>  
>          /* Use a virtual address to access the UART. */
> -        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> +        ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
>  #endif
>  
>          /* Map the DTB in the boot misc slot */
> diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> index 5f7c529..759d59a 100644
> --- a/xen/include/asm-arm/early_printk.h
> +++ b/xen/include/asm-arm/early_printk.h
> @@ -12,6 +12,14 @@
>  
>  #include <xen/config.h>
>  
> +#ifdef __ASSEMBLY__
> +
> +/* need to add the uart address offset in page to the fixmap address */
> +#define EARLY_UART_VIRTUAL_ADDRESS \
> +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
Why should this be limited to assembly code?  It would seem valid for
early C code to see and use as well.

With that change, you can drop all the #ifdef''ary

~Andrew
> +
> +#else
> +
>  #ifdef EARLY_PRINTK
>  
>  void early_printk(const char *fmt, ...)
> @@ -31,4 +39,6 @@ __attribute__((format (printf, 1, 2))) early_panic(const
char *fmt, ...)
>  
>  #endif
>  
> +#endif  /* __ASSEMBLY__ */
> +
>  #endif

Jon Fraser

2013-Oct-30 22:53 UTC

head link

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

On Wed, 2013-10-30 at 21:25 +0000, Andrew Cooper wrote:> On 30/10/13 21:18, Jon Fraser wrote:
> > When calculating the virtual address of the UART,
> > add the page offset of the UART to the mapped address.
> >
> > Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
> > of FIXMAP_ADDR(FIXMAP_CONSOLE)
> >
> > Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> > ---
> >  xen/arch/arm/arm32/debug.S         |  5 +++--
> >  xen/arch/arm/arm32/head.S          |  3 ++-
> >  xen/arch/arm/arm64/debug.S         |  5 +++--
> >  xen/arch/arm/arm64/head.S          |  3 ++-
> >  xen/include/asm-arm/early_printk.h | 10 ++++++++++
> >  5 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
> > index ec774cd..a95d9da 100644
> > --- a/xen/arch/arm/arm32/debug.S
> > +++ b/xen/arch/arm/arm32/debug.S
> > @@ -18,6 +18,7 @@
> >   */
> >  
> >  #include <asm/config.h>
> > +#include <asm/early_printk.h>
> >  
> >  #ifdef EARLY_PRINTK_INC
> >  #include EARLY_PRINTK_INC
> > @@ -26,14 +27,14 @@
> >  /* Print a character on the UART - this function is called by C
> >   * r0: character to print */
> >  GLOBAL(early_putch)
> > -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base
address */
> > +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base
address */
> >          early_uart_ready r1, r2
> >          early_uart_transmit r1, r0
> >          mov   pc, lr
> >  
> >  /* Flush the UART - this function is called by C */
> >  GLOBAL(early_flush)
> > -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base
address */
> > +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base
address */
> >          early_uart_ready r1, r2
> >          mov   pc, lr
> >  
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index ce1d21a..96230ac 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -22,6 +22,7 @@
> >  #include <asm/processor-ca15.h>
> >  #include <asm/processor-ca7.h>
> >  #include <asm/asm_defns.h>
> > +#include <asm/early_printk.h>
> >  
> >  #define ZIMAGE_MAGIC_NUMBER 0x016f2818
> >  
> > @@ -356,7 +357,7 @@ paging:
> >          strd  r2, r3, [r1, r4]       /* Map it in the
fixmap''s slot */
> >  
> >          /* Use a virtual address to access the UART. */
> > -        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > +        ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
> >  #endif
> >          /* Map the DTB in the boot misc slot */
> >          teq   r12, #0                /* Only on boot CPU */
> > diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
> > index 472c157..a855358 100644
> > --- a/xen/arch/arm/arm64/debug.S
> > +++ b/xen/arch/arm/arm64/debug.S
> > @@ -18,6 +18,7 @@
> >   */
> >  
> >  #include <asm/config.h>
> > +#include <asm/early_printk.h>
> >  
> >  #ifdef EARLY_PRINTK_INC
> >  #include EARLY_PRINTK_INC
> > @@ -26,14 +27,14 @@
> >  /* Print a character on the UART - this function is called by C
> >   * x0: character to print */
> >  GLOBAL(early_putch)
> > -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
> >          early_uart_ready x15, 1
> >          early_uart_transmit x15, w0
> >          ret
> >  
> >  /* Flush the UART - this function is called by C */
> >  GLOBAL(early_flush)
> > -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* x15 := VA UART
base address */
> > +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA UART
base address */
> >          early_uart_ready x15, 1
> >          ret
> >  
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index b8b5902..31afdd0 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -23,6 +23,7 @@
> >  #include <asm/config.h>
> >  #include <asm/page.h>
> >  #include <asm/asm_defns.h>
> > +#include <asm/early_printk.h>
> >  
> >  #define PT_PT     0xe7f /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=1
P=1 */
> >  #define PT_MEM    0xe7d /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=0
P=1 */
> > @@ -368,7 +369,7 @@ paging:
> >          str   x2, [x4, x1]           /* Map it in the
fixmap''s slot */
> >  
> >          /* Use a virtual address to access the UART. */
> > -        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > +        ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
> >  #endif
> >  
> >          /* Map the DTB in the boot misc slot */
> > diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> > index 5f7c529..759d59a 100644
> > --- a/xen/include/asm-arm/early_printk.h
> > +++ b/xen/include/asm-arm/early_printk.h
> > @@ -12,6 +12,14 @@
> >  
> >  #include <xen/config.h>
> >  
> > +#ifdef __ASSEMBLY__
> > +
> > +/* need to add the uart address offset in page to the fixmap address
*/
> > +#define EARLY_UART_VIRTUAL_ADDRESS \
> > +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> 
> Why should this be limited to assembly code?  It would seem valid for
> early C code to see and use as well.
> 
> With that change, you can drop all the #ifdef''ary
> 
> ~Andrew
> 
The problem is that the C code in the include can''t be included in the
assembler files.  I suppose that EARLY_UART_VIRTUAL_ADDRESS could be
used in C code.  I''d still have to ifdef out the C code in the include
file for the assembler files.
Jon
> > +
> > +#else
> > +
> >  #ifdef EARLY_PRINTK
> >  
> >  void early_printk(const char *fmt, ...)
> > @@ -31,4 +39,6 @@ __attribute__((format (printf, 1, 2)))
early_panic(const char *fmt, ...)
> >  
> >  #endif
> >  
> > +#endif  /* __ASSEMBLY__ */
> > +
> >  #endif
> 
>

Julien Grall

2013-Oct-30 23:09 UTC

head link

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

On 10/30/2013 02:18 PM, Jon Fraser wrote:> When calculating the virtual address of the UART,
> add the page offset of the UART to the mapped address.
>
> Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
> of FIXMAP_ADDR(FIXMAP_CONSOLE)
>
> Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> ---
[...]
> diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> index 5f7c529..759d59a 100644
> --- a/xen/include/asm-arm/early_printk.h
> +++ b/xen/include/asm-arm/early_printk.h
> @@ -12,6 +12,14 @@
>
>   #include <xen/config.h>
>
> +#ifdef __ASSEMBLY__
> +
> +/* need to add the uart address offset in page to the fixmap address */
> +#define EARLY_UART_VIRTUAL_ADDRESS \
> +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> +
Aside Andrew''s comment, you should only define 
EARLY_UART_VIRTUAL_ADDRESS when EARLY_PRINTK is enabled.

-- 
Julien Grall

Ian Campbell

2013-Oct-31 16:27 UTC

head link

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

On Wed, 2013-10-30 at 18:53 -0400, Jon Fraser wrote:> On Wed, 2013-10-30 at 21:25 +0000, Andrew Cooper wrote:
> > On 30/10/13 21:18, Jon Fraser wrote:
> > > When calculating the virtual address of the UART,
> > > add the page offset of the UART to the mapped address.
> > >
> > > Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
> > > of FIXMAP_ADDR(FIXMAP_CONSOLE)
> > >
> > > Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> > > ---
> > >  xen/arch/arm/arm32/debug.S         |  5 +++--
> > >  xen/arch/arm/arm32/head.S          |  3 ++-
> > >  xen/arch/arm/arm64/debug.S         |  5 +++--
> > >  xen/arch/arm/arm64/head.S          |  3 ++-
> > >  xen/include/asm-arm/early_printk.h | 10 ++++++++++
> > >  5 files changed, 20 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/arm32/debug.S
b/xen/arch/arm/arm32/debug.S
> > > index ec774cd..a95d9da 100644
> > > --- a/xen/arch/arm/arm32/debug.S
> > > +++ b/xen/arch/arm/arm32/debug.S
> > > @@ -18,6 +18,7 @@
> > >   */
> > >  
> > >  #include <asm/config.h>
> > > +#include <asm/early_printk.h>
> > >  
> > >  #ifdef EARLY_PRINTK_INC
> > >  #include EARLY_PRINTK_INC
> > > @@ -26,14 +27,14 @@
> > >  /* Print a character on the UART - this function is called by C
> > >   * r0: character to print */
> > >  GLOBAL(early_putch)
> > > -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART
base address */
> > > +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART
base address */
> > >          early_uart_ready r1, r2
> > >          early_uart_transmit r1, r0
> > >          mov   pc, lr
> > >  
> > >  /* Flush the UART - this function is called by C */
> > >  GLOBAL(early_flush)
> > > -        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART
base address */
> > > +        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART
base address */
> > >          early_uart_ready r1, r2
> > >          mov   pc, lr
> > >  
> > > diff --git a/xen/arch/arm/arm32/head.S
b/xen/arch/arm/arm32/head.S
> > > index ce1d21a..96230ac 100644
> > > --- a/xen/arch/arm/arm32/head.S
> > > +++ b/xen/arch/arm/arm32/head.S
> > > @@ -22,6 +22,7 @@
> > >  #include <asm/processor-ca15.h>
> > >  #include <asm/processor-ca7.h>
> > >  #include <asm/asm_defns.h>
> > > +#include <asm/early_printk.h>
> > >  
> > >  #define ZIMAGE_MAGIC_NUMBER 0x016f2818
> > >  
> > > @@ -356,7 +357,7 @@ paging:
> > >          strd  r2, r3, [r1, r4]       /* Map it in the
fixmap''s slot */
> > >  
> > >          /* Use a virtual address to access the UART. */
> > > -        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > > +        ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
> > >  #endif
> > >          /* Map the DTB in the boot misc slot */
> > >          teq   r12, #0                /* Only on boot CPU */
> > > diff --git a/xen/arch/arm/arm64/debug.S
b/xen/arch/arm/arm64/debug.S
> > > index 472c157..a855358 100644
> > > --- a/xen/arch/arm/arm64/debug.S
> > > +++ b/xen/arch/arm/arm64/debug.S
> > > @@ -18,6 +18,7 @@
> > >   */
> > >  
> > >  #include <asm/config.h>
> > > +#include <asm/early_printk.h>
> > >  
> > >  #ifdef EARLY_PRINTK_INC
> > >  #include EARLY_PRINTK_INC
> > > @@ -26,14 +27,14 @@
> > >  /* Print a character on the UART - this function is called by C
> > >   * x0: character to print */
> > >  GLOBAL(early_putch)
> > > -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > > +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
> > >          early_uart_ready x15, 1
> > >          early_uart_transmit x15, w0
> > >          ret
> > >  
> > >  /* Flush the UART - this function is called by C */
> > >  GLOBAL(early_flush)
> > > -        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* x15 := VA
UART base address */
> > > +        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA
UART base address */
> > >          early_uart_ready x15, 1
> > >          ret
> > >  
> > > diff --git a/xen/arch/arm/arm64/head.S
b/xen/arch/arm/arm64/head.S
> > > index b8b5902..31afdd0 100644
> > > --- a/xen/arch/arm/arm64/head.S
> > > +++ b/xen/arch/arm/arm64/head.S
> > > @@ -23,6 +23,7 @@
> > >  #include <asm/config.h>
> > >  #include <asm/page.h>
> > >  #include <asm/asm_defns.h>
> > > +#include <asm/early_printk.h>
> > >  
> > >  #define PT_PT     0xe7f /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111
T=1 P=1 */
> > >  #define PT_MEM    0xe7d /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111
T=0 P=1 */
> > > @@ -368,7 +369,7 @@ paging:
> > >          str   x2, [x4, x1]           /* Map it in the
fixmap''s slot */
> > >  
> > >          /* Use a virtual address to access the UART. */
> > > -        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
> > > +        ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
> > >  #endif
> > >  
> > >          /* Map the DTB in the boot misc slot */
> > > diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> > > index 5f7c529..759d59a 100644
> > > --- a/xen/include/asm-arm/early_printk.h
> > > +++ b/xen/include/asm-arm/early_printk.h
> > > @@ -12,6 +12,14 @@
> > >  
> > >  #include <xen/config.h>
> > >  
> > > +#ifdef __ASSEMBLY__
> > > +
> > > +/* need to add the uart address offset in page to the fixmap
address */
> > > +#define EARLY_UART_VIRTUAL_ADDRESS \
> > > +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> > 
> > Why should this be limited to assembly code?  It would seem valid for
> > early C code to see and use as well.
> > 
> > With that change, you can drop all the #ifdef''ary
> > 
> > ~Andrew
> > 
> 
> The problem is that the C code in the include can''t be included in
the
> assembler files.  I suppose that EARLY_UART_VIRTUAL_ADDRESS could be
> used in C code.
C code should be using early_printk, not anything which would need to
touch EARLY_UART_VIRTUAL_ADDRESS. Ultimately early_printk uses
early_putch which is implemented in ASM and is where
EARLY_UART_VIRTUAL_ADDRESS should be used (and I suppose is by this
patch, I didn''t check). (this doesn''t mean the #define should
be hidden
from C, just that it shouldn''t be used).

I think your __ASSEMBLY__ #ifdef is ok, but shouldn''t it be inside the
#idef EARLY_PRINTK?
> I''d still have to ifdef out the C code in the include
> file for the assembler files.
#ifndef __ASSEMBLY__ and drop the #else if you like, so C sees
everything and ASM sees only the relevant define that would be fine too.

Ian.

Jon Fraser

2013-Oct-31 19:37 UTC

head link

[PATCH V3] xen/arm: UART addresses are not always aligned to a page.

When calculating the virtual address of the UART,
add the page offset of the UART to the mapped address.

Create define EARLY_UART_VIRTUAL_ADDRESS and use instead
of FIXMAP_ADDR(FIXMAP_CONSOLE)

Signed-off-by: Jon Fraser <jfraser@broadcom.com>
---
 xen/arch/arm/arm32/debug.S         |  5 +++--
 xen/arch/arm/arm32/head.S          |  3 ++-
 xen/arch/arm/arm64/debug.S         |  5 +++--
 xen/arch/arm/arm64/head.S          |  3 ++-
 xen/include/asm-arm/early_printk.h | 12 ++++++++++++
 5 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
index ec774cd..a95d9da 100644
--- a/xen/arch/arm/arm32/debug.S
+++ b/xen/arch/arm/arm32/debug.S
@@ -18,6 +18,7 @@
  */
 
 #include <asm/config.h>
+#include <asm/early_printk.h>
 
 #ifdef EARLY_PRINTK_INC
 #include EARLY_PRINTK_INC
@@ -26,14 +27,14 @@
 /* Print a character on the UART - this function is called by C
  * r0: character to print */
 GLOBAL(early_putch)
-        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base address
*/
+        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base address */
         early_uart_ready r1, r2
         early_uart_transmit r1, r0
         mov   pc, lr
 
 /* Flush the UART - this function is called by C */
 GLOBAL(early_flush)
-        ldr   r1, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* r1 := VA UART base address
*/
+        ldr   r1, =EARLY_UART_VIRTUAL_ADDRESS  /* r1 := VA UART base address */
         early_uart_ready r1, r2
         mov   pc, lr
 
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index ce1d21a..96230ac 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -22,6 +22,7 @@
 #include <asm/processor-ca15.h>
 #include <asm/processor-ca7.h>
 #include <asm/asm_defns.h>
+#include <asm/early_printk.h>
 
 #define ZIMAGE_MAGIC_NUMBER 0x016f2818
 
@@ -356,7 +357,7 @@ paging:
         strd  r2, r3, [r1, r4]       /* Map it in the fixmap''s slot */
 
         /* Use a virtual address to access the UART. */
-        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   r11, =EARLY_UART_VIRTUAL_ADDRESS
 #endif
         /* Map the DTB in the boot misc slot */
         teq   r12, #0                /* Only on boot CPU */
diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
index 472c157..a855358 100644
--- a/xen/arch/arm/arm64/debug.S
+++ b/xen/arch/arm/arm64/debug.S
@@ -18,6 +18,7 @@
  */
 
 #include <asm/config.h>
+#include <asm/early_printk.h>
 
 #ifdef EARLY_PRINTK_INC
 #include EARLY_PRINTK_INC
@@ -26,14 +27,14 @@
 /* Print a character on the UART - this function is called by C
  * x0: character to print */
 GLOBAL(early_putch)
-        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
         early_uart_ready x15, 1
         early_uart_transmit x15, w0
         ret
 
 /* Flush the UART - this function is called by C */
 GLOBAL(early_flush)
-        ldr   x15, =FIXMAP_ADDR(FIXMAP_CONSOLE)  /* x15 := VA UART base address
*/
+        ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA UART base address
*/
         early_uart_ready x15, 1
         ret
 
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index b8b5902..31afdd0 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -23,6 +23,7 @@
 #include <asm/config.h>
 #include <asm/page.h>
 #include <asm/asm_defns.h>
+#include <asm/early_printk.h>
 
 #define PT_PT     0xe7f /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=1 P=1 */
 #define PT_MEM    0xe7d /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=111 T=0 P=1 */
@@ -368,7 +369,7 @@ paging:
         str   x2, [x4, x1]           /* Map it in the fixmap''s slot */
 
         /* Use a virtual address to access the UART. */
-        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        ldr   x23, =EARLY_UART_VIRTUAL_ADDRESS
 #endif
 
         /* Map the DTB in the boot misc slot */
diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
index 5f7c529..707bbf7 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -14,6 +14,16 @@
 
 #ifdef EARLY_PRINTK
 
+/* need to add the uart address offset in page to the fixmap address */
+#define EARLY_UART_VIRTUAL_ADDRESS \
+    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
+
+#endif
+
+#ifndef __ASSEMBLY__
+
+#ifdef EARLY_PRINTK
+
 void early_printk(const char *fmt, ...)
     __attribute__((format (printf, 1, 2)));
 void early_panic(const char *fmt, ...) __attribute__((noreturn))
@@ -31,4 +41,6 @@ __attribute__((format (printf, 1, 2))) early_panic(const char
*fmt, ...)
 
 #endif
 
+#endif	/* __ASSEMBLY__ */
+
 #endif
-- 
1.7.11.3

Ian Campbell

2013-Oct-31 19:52 UTC

head link

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

On Thu, 2013-10-31 at 15:37 -0400, Jon Fraser wrote:> diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> index 5f7c529..707bbf7 100644
> --- a/xen/include/asm-arm/early_printk.h
> +++ b/xen/include/asm-arm/early_printk.h
> @@ -14,6 +14,16 @@
>  
>  #ifdef EARLY_PRINTK
>  
> +/* need to add the uart address offset in page to the fixmap address */
> +#define EARLY_UART_VIRTUAL_ADDRESS \
> +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> +
> +#endif
> +
> +#ifndef __ASSEMBLY__
> +
> +#ifdef EARLY_PRINTK
> +
Is there something wrong with:
         #ifdef EARLY_PRINT
        
        +#define EARLY_UART_VIETRUAL_ADDRESS ...
        +
        +#ifndef __ASSEMBLY__
        +
         [... existing C code...]
        +
        +#endif /* !__ASSEMBLY__ */
        +
         #endif /* EARLY_PRINTK */

Rather than two lots of early_printk ifdefery.

Ian.

Jon Fraser

2013-Oct-31 20:11 UTC

head link

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

On Thu, 2013-10-31 at 19:52 +0000, Ian Campbell wrote:> On Thu, 2013-10-31 at 15:37 -0400, Jon Fraser wrote:
> > diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> > index 5f7c529..707bbf7 100644
> > --- a/xen/include/asm-arm/early_printk.h
> > +++ b/xen/include/asm-arm/early_printk.h
> > @@ -14,6 +14,16 @@
> >  
> >  #ifdef EARLY_PRINTK
> >  
> > +/* need to add the uart address offset in page to the fixmap address
*/
> > +#define EARLY_UART_VIRTUAL_ADDRESS \
> > +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> > +
> > +#endif
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#ifdef EARLY_PRINTK
> > +
> 
> Is there something wrong with:
>          #ifdef EARLY_PRINT
>         
>         +#define EARLY_UART_VIETRUAL_ADDRESS ...
>         +
>         +#ifndef __ASSEMBLY__
>         +
>          [... existing C code...]
>         +
>         +#endif /* !__ASSEMBLY__ */
>         +
>          #endif /* EARLY_PRINTK */
> 
> Rather than two lots of early_printk ifdefery.
> 
> Ian.
> 
The dummy functions for early_printk and early_panic
need to be visible for not EARLY_PRINTK, but not visible
for __ASSEMBLY__.

Ian Campbell

2013-Oct-31 20:47 UTC

head link

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

On Thu, 2013-10-31 at 16:11 -0400, Jon Fraser wrote:> On Thu, 2013-10-31 at 19:52 +0000, Ian Campbell wrote:
> > On Thu, 2013-10-31 at 15:37 -0400, Jon Fraser wrote:
> > > diff --git a/xen/include/asm-arm/early_printk.h
b/xen/include/asm-arm/early_printk.h
> > > index 5f7c529..707bbf7 100644
> > > --- a/xen/include/asm-arm/early_printk.h
> > > +++ b/xen/include/asm-arm/early_printk.h
> > > @@ -14,6 +14,16 @@
> > >  
> > >  #ifdef EARLY_PRINTK
> > >  
> > > +/* need to add the uart address offset in page to the fixmap
address */
> > > +#define EARLY_UART_VIRTUAL_ADDRESS \
> > > +    (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS &
~PAGE_MASK))
> > > +
> > > +#endif
> > > +
> > > +#ifndef __ASSEMBLY__
> > > +
> > > +#ifdef EARLY_PRINTK
> > > +
> > 
> > Is there something wrong with:
> >          #ifdef EARLY_PRINT
> >         
> >         +#define EARLY_UART_VIETRUAL_ADDRESS ...
> >         +
> >         +#ifndef __ASSEMBLY__
> >         +
> >          [... existing C code...]
> >         +
> >         +#endif /* !__ASSEMBLY__ */
> >         +
> >          #endif /* EARLY_PRINTK */
> > 
> > Rather than two lots of early_printk ifdefery.
> > 
> > Ian.
> > 
> 
> The dummy functions for early_printk and early_panic
> need to be visible for not EARLY_PRINTK, but not visible
> for __ASSEMBLY__.
Ah, thanks, was missing that context from the patch, I should have
looked at the original file.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Ian Campbell

2013-Nov-01 13:24 UTC

head link

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

On Thu, 2013-10-31 at 20:47 +0000, Ian Campbell wrote:> Acked-by: Ian Campbell <ian.campbell@citrix.com>
And applied, thanks.

Xen devel - Oct 2013 - [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

[PATCH V2] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V2] xen/arm: UART addresses are not always aligned to a page.

[PATCH V3] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.

Re: [PATCH V3] xen/arm: UART addresses are not always aligned to a page.