Re: [Xen-devel] [PATCH 28/45] xen: arm: arm64 trap handling.

[Tim Deegan <tim@xxxxxxx>]

At 15:56 +0000 on 23 Jan (1358956594), Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>


> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 815f305..be41f43 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -1,5 +1,7 @@
>  subdir-y += lib
>  
> +obj-y += entry.o
>  obj-y += mode_switch.o
>  
> +obj-y += traps.o
>  obj-y += domain.o

Maybe in alphabetical order?

> +/* base-2 logarithm */
> +#define __L2(_x)  (((_x) & 0x00000002) ?   1 : 0)
> +#define __L4(_x)  (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x))
> +#define __L8(_x)  (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x))
> +#define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x))
> +#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x))

This is now replicated in three places.  Maybe it should live in, say,
xen/bitops.h?

> +hyp_sync:
> +        entry   hyp=1
> +        msr     daifclr, #2
> +        adr     lr, return_to_hypervisor
> +        mov     x0, sp
> +        b       do_trap_hypervisor

This pattern (call fnX with arg0 == sp and lr == fnY) is repeated
quite a few times.  Could we have another tidying macro for it?

> +ENTRY(return_to_new_vcpu)
> +ENTRY(return_to_guest)
> +ENTRY(return_to_hypervisor)
> +        ldp     x21, x22, [sp, #UREGS_PC]       // load ELR, SPSR
> +
> +        pop     x0, x1
> +        pop     x2, x3
> +        pop     x4, x5
> +        pop     x6, x7
> +        pop     x8, x9
> +
> +        /* XXX handle return to guest tasks, soft irqs etc */
> +        
> +        msr     elr_el2, x21                    // set up the return data
> +        msr     spsr_el2, x22

Done here becasue it's roughly half-way between the load and the
overwrite below?  Should we be using x28/x29 for this to give ourselves
even more pipeline space?

> +        pop     x10, x11
> +        pop     x12, x13
> +        pop     x14, x15
> +        pop     x16, x17
> +        pop     x18, x19
> +        pop     x20, x21
> +        pop     x22, x23
> +        pop     x24, x25
> +        pop     x26, x27
> +        pop     x28, x29
> +
> +        ldr     lr, [sp], #(UREGS_SPSR_el1 - UREGS_SP)
> +        eret
> +

> --- a/xen/arch/arm/io.h
> +++ b/xen/arch/arm/io.h
> @@ -26,7 +26,7 @@
>  typedef struct
>  {
>      struct hsr_dabt dabt;
> -    uint32_t gva;
> +    vaddr_t gva;
>      paddr_t gpa;
>  } mmio_info_t;
>  

This seems like it belongs in the earlier vaddr patch.

> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -238,7 +238,7 @@ union hsr {
>  #endif
>  
>  #ifndef __ASSEMBLY__
> -extern uint32_t hyp_traps_vector[8];
> +extern uint32_t hyp_traps_vector;

hyp_traps_vector[], and avoid adding '&' to the users?

Tim.

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