Re: [Xen-devel] [PATCH 1/2] xen/arm32: implement VFP context switch

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-05-30 at 15:38 +0100, Julien Grall wrote:
> diff --git a/xen/arch/arm/arm32/vfp.c b/xen/arch/arm/arm32/vfp.c
> new file mode 100644
> index 0000000..16f635a
> --- /dev/null
> +++ b/xen/arch/arm/arm32/vfp.c
> @@ -0,0 +1,71 @@
> +#include <xen/sched.h>
> +#include <asm/processor.h>
> +#include <asm/vfp.h>
> +
> +void vfp_save_state(struct vcpu *v)
> +{
[...]
> +}
> +
> +void vfp_restore_state(struct vcpu *v)
> +{
[...]

Without having read the documentation this seem plausible enough.

> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/arm64/vfp.h b/xen/include/asm-arm/arm64/vfp.h
> new file mode 100644
> index 0000000..34cd202
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/vfp.h
> @@ -0,0 +1,16 @@
> +#ifndef _ARM_ARM64_VFP_H
> +#define _ARM_ARM32_VFP_H

Mismatch.

> +
> +struct vfp_state
> +{
> +};
> +
> +#endif /* _ARM_ARM32_VFP_H */

And again.

> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
> index f08d59a..d99ccfd 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -60,6 +60,12 @@
>   * arguments, which are cp,opc1,crn,crm,opc2.
>   */
>  

+ /* Coprocessor 10 */

Please.

> +#define FPSCR           p10,7,c1,c0,0   /* Floating-Point Status and Control 
> Register */
> +#define MVFR0           p10,7,c7,c0,0   /* Media and VFP Feature Register 0 
> */
> +#define FPEXC           p10,7,c8,c0,0   /* Floating-Point Exception Control 
> Register */
> +#define FPINST          p10,7,c9,c0,0   /* Floating-Point Instruction 
> Register */
> +#define FPINST2         p10,7,c10,c0,0  /* Floating-point Instruction 
> Register 2 */
> +
>  /* Coprocessor 14 */
>  
>  /* CP14 CR0: */
> @@ -106,6 +112,7 @@
>  #define NSACR           p15,0,c1,c1,2   /* Non-Secure Access Control 
> Register */
>  #define HSCTLR          p15,4,c1,c0,0   /* Hyp. System Control Register */
>  #define HCR             p15,4,c1,c1,0   /* Hyp. Configuration Register */
> +#define HCPTR           p15,4,c1,c1,2   /* Hyp. Coprocessor Trap Register */
>  
>  /* CP15 CR2: Translation Table Base and Control Registers */
>  #define TTBCR           p15,0,c2,c0,2   /* Translatation Table Base Control 
> Register */
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index cb251cc..6b52b5e 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -1,11 +1,13 @@
>  #ifndef __ASM_DOMAIN_H__
>  #define __ASM_DOMAIN_H__
>  
> +#include <asm/atomic.h>

What is this used for?

>  #include <xen/config.h>
>  #include <xen/cache.h>
>  #include <xen/sched.h>
>  #include <asm/page.h>
>  #include <asm/p2m.h>
> +#include <asm/vfp.h>
>  #include <public/hvm/params.h>
>  
>  /* Represents state corresponding to a block of 32 interrupts */
> diff --git a/xen/include/asm-arm/vfp.h b/xen/include/asm-arm/vfp.h
> index b800816..6ba3cd1 100644
> --- a/xen/include/asm-arm/vfp.h
> +++ b/xen/include/asm-arm/vfp.h
> @@ -2,7 +2,14 @@
>  #define __ARM_VFP_H_
>  
>  #include <xen/types.h>
> +#include <xen/sched.h>

> +#if defined(CONFIG_ARM_32)
> +# include <asm/arm32/vfp.h>
> +#elif defined(CONFIG_ARM_64)
> +# include <asm/arm64/vfp.h>

Did you mean to include a #else here?

> +# error "Unknown ARM variant"
> +#endif
>  
>  #ifdef CONFIG_ARM_32

I suppose the content of this #if now belongs in the subarch header.

>  
> @@ -32,6 +39,9 @@ static inline void enable_vfp(void)
>  }
>  #endif
>  
> +void vfp_save_state(struct vcpu *v);
> +void vfp_restore_state(struct vcpu *v);
> +
>  #endif
>  /*
>   * Local variables:



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