[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [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@xxxxxxxxxxxx>
> > > ---
> > >  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.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.