* [PATCH 0/2] Implement VFP context switch for arm32
@ 2013-05-30 14:38 Julien Grall
2013-05-30 14:38 ` [PATCH 1/2] xen/arm32: implement VFP context switch Julien Grall
2013-05-30 14:38 ` [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot Julien Grall
0 siblings, 2 replies; 8+ messages in thread
From: Julien Grall @ 2013-05-30 14:38 UTC (permalink / raw)
To: xen-devel; +Cc: patches, tim, ian.campbell, Julien Grall, Stefano.Stabellini
Hello,
This small patch series implement the VFP context for arm32.
I didn't implement the arm64 support, and just add dummy function to avoid
compilation breakage on this platform.
The second patch remove VFP support on XEN because we can let the guest
to enable when it's required by a process.
Cheers,
Julien Grall (2):
xen/arm32: implement VFP context switch
xen/arm: don't enable VFP on XEN during the boot
xen/arch/arm/Rules.mk | 2 +-
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/vfp.c | 71 +++++++++++++++++++++++++++++++++++++++
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/vfp.c | 13 +++++++
xen/arch/arm/domain.c | 7 ++--
xen/arch/arm/setup.c | 2 --
xen/arch/arm/smpboot.c | 1 -
xen/include/asm-arm/arm32/vfp.h | 29 ++++++++++++++++
xen/include/asm-arm/arm64/vfp.h | 16 +++++++++
xen/include/asm-arm/cpregs.h | 7 ++++
xen/include/asm-arm/domain.h | 5 +++
xen/include/asm-arm/vfp.h | 36 +++++---------------
13 files changed, 158 insertions(+), 33 deletions(-)
create mode 100644 xen/arch/arm/arm32/vfp.c
create mode 100644 xen/arch/arm/arm64/vfp.c
create mode 100644 xen/include/asm-arm/arm32/vfp.h
create mode 100644 xen/include/asm-arm/arm64/vfp.h
--
1.7.10.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] xen/arm32: implement VFP context switch
2013-05-30 14:38 [PATCH 0/2] Implement VFP context switch for arm32 Julien Grall
@ 2013-05-30 14:38 ` Julien Grall
2013-05-30 15:11 ` Ian Campbell
2013-05-30 14:38 ` [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot Julien Grall
1 sibling, 1 reply; 8+ messages in thread
From: Julien Grall @ 2013-05-30 14:38 UTC (permalink / raw)
To: xen-devel; +Cc: patches, tim, ian.campbell, Julien Grall, Stefano.Stabellini
Add support for VFP context switch on arm32 and a dummy support for arm64
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/vfp.c | 71 +++++++++++++++++++++++++++++++++++++++
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/vfp.c | 13 +++++++
xen/arch/arm/domain.c | 7 ++--
xen/include/asm-arm/arm32/vfp.h | 29 ++++++++++++++++
xen/include/asm-arm/arm64/vfp.h | 16 +++++++++
xen/include/asm-arm/cpregs.h | 7 ++++
xen/include/asm-arm/domain.h | 5 +++
xen/include/asm-arm/vfp.h | 10 ++++++
10 files changed, 158 insertions(+), 2 deletions(-)
create mode 100644 xen/arch/arm/arm32/vfp.c
create mode 100644 xen/arch/arm/arm64/vfp.c
create mode 100644 xen/include/asm-arm/arm32/vfp.h
create mode 100644 xen/include/asm-arm/arm64/vfp.h
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index aaf277a..b903803 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -6,5 +6,6 @@ obj-y += proc-ca15.o
obj-y += traps.o
obj-y += domain.o
+obj-y += vfp.o
obj-$(EARLY_PRINTK) += debug.o
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)
+{
+ uint32_t tmp;
+
+ v->arch.vfp.fpexc = READ_CP32(FPEXC);
+
+ WRITE_CP32(v->arch.vfp.fpexc | FPEXC_EN, FPEXC);
+
+ v->arch.vfp.fpscr = READ_CP32(FPSCR);
+
+ if ( v->arch.vfp.fpexc & FPEXC_EX ) /* Check for sub-architecture */
+ {
+ v->arch.vfp.fpinst = READ_CP32(FPINST);
+
+ if ( v->arch.vfp.fpexc & FPEXC_FP2V )
+ v->arch.vfp.fpinst2 = READ_CP32(FPINST2);
+ /* Disable FPEXC_EX */
+ WRITE_CP32((v->arch.vfp.fpexc | FPEXC_EN) & ~FPEXC_EX, FPEXC);
+ }
+
+ /* Save {d0-d15} */
+ asm volatile("stc p11, cr0, [%0], #32*4" : : "r" (v->arch.vfp.fpregs1));
+
+ tmp = READ_CP32(MVFR0);
+ if ( (tmp & MVFR0_A_SIMD_MASK) == 2 ) /* 32 x 64 bits registers */
+ {
+ /* Save {d16-d31} */
+ asm volatile("stcl p11, cr0, [%0], #32*4" : : "r" (v->arch.vfp.fpregs2));
+ }
+
+ WRITE_CP32(v->arch.vfp.fpexc & ~(FPEXC_EN), FPEXC);
+}
+
+void vfp_restore_state(struct vcpu *v)
+{
+ uint32_t tmp = READ_CP32(FPEXC);
+
+ WRITE_CP32(tmp | FPEXC_EN, FPEXC);
+
+ /* Restore {d0-d15} */
+ asm volatile("ldc p11, cr0, [%0], #32*4" : : "r" (v->arch.vfp.fpregs1));
+
+ tmp = READ_CP32(MVFR0);
+ if ( (tmp & MVFR0_A_SIMD_MASK) == 2 ) /* 32 x 64 bits registers */
+ /* Restore {d16-d31} */
+ asm volatile("ldcl p11, cr0, [%0], #32*4" : : "r" (v->arch.vfp.fpregs2));
+
+ if ( v->arch.vfp.fpexc & FPEXC_EX )
+ {
+ WRITE_CP32(v->arch.vfp.fpinst, FPINST);
+ if ( v->arch.vfp.fpexc & FPEXC_FP2V )
+ WRITE_CP32(v->arch.vfp.fpinst2, FPINST2);
+ }
+
+ WRITE_CP32(v->arch.vfp.fpscr, FPSCR);
+
+ WRITE_CP32(v->arch.vfp.fpexc, FPEXC);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 9484548..e06a0a9 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -5,5 +5,6 @@ obj-y += mode_switch.o
obj-y += traps.o
obj-y += domain.o
+obj-y += vfp.o
obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
new file mode 100644
index 0000000..74e6a50
--- /dev/null
+++ b/xen/arch/arm/arm64/vfp.c
@@ -0,0 +1,13 @@
+#include <xen/sched.h>
+#include <asm/processor.h>
+#include <asm/vfp.h>
+
+void vfp_save_state(struct vcpu *v)
+{
+ /* TODO: implement it */
+}
+
+void vfp_restore_state(struct vcpu *v)
+{
+ /* TODO: implement it */
+}
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 4c434a1..f465ab7 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -27,6 +27,7 @@
#include <asm/p2m.h>
#include <asm/irq.h>
#include <asm/cpufeature.h>
+#include <asm/vfp.h>
#include <asm/gic.h>
#include "vtimer.h"
@@ -117,7 +118,8 @@ static void ctxt_switch_from(struct vcpu *p)
/* XXX MPU */
- /* XXX VFP */
+ /* VFP */
+ vfp_save_state(p);
/* VGIC */
gic_save_state(p);
@@ -143,7 +145,8 @@ static void ctxt_switch_to(struct vcpu *n)
/* VGIC */
gic_restore_state(n);
- /* XXX VFP */
+ /* VFP */
+ vfp_restore_state(n);
/* XXX MPU */
diff --git a/xen/include/asm-arm/arm32/vfp.h b/xen/include/asm-arm/arm32/vfp.h
new file mode 100644
index 0000000..c32296e
--- /dev/null
+++ b/xen/include/asm-arm/arm32/vfp.h
@@ -0,0 +1,29 @@
+#ifndef _ARM_ARM32_VFP_H
+#define _ARM_ARM32_VFP_H
+
+#define FPEXC_EX (1u << 31)
+#define FPEXC_EN (1u << 30)
+#define FPEXC_FP2V (1u << 28)
+
+#define MVFR0_A_SIMD_MASK (0xf << 0)
+
+struct vfp_state
+{
+ uint64_t fpregs1[16]; /* {d0-d15} */
+ uint64_t fpregs2[16]; /* {d16-d31} */
+ uint32_t fpexc;
+ uint32_t fpscr;
+ /* VFP implementation specific state */
+ uint32_t fpinst;
+ uint32_t fpinst2;
+};
+
+#endif /* _ARM_ARM32_VFP_H */
+/*
+ * 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
+
+struct vfp_state
+{
+};
+
+#endif /* _ARM_ARM32_VFP_H */
+/*
+ * 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.
*/
+#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>
#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 */
@@ -188,6 +190,9 @@ struct arch_vcpu
uint32_t joscr, jmcr;
#endif
+ /* Float-pointer */
+ struct vfp_state vfp;
+
/* CP 15 */
uint32_t csselr;
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>
+# error "Unknown ARM variant"
+#endif
#ifdef CONFIG_ARM_32
@@ -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:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot
2013-05-30 14:38 [PATCH 0/2] Implement VFP context switch for arm32 Julien Grall
2013-05-30 14:38 ` [PATCH 1/2] xen/arm32: implement VFP context switch Julien Grall
@ 2013-05-30 14:38 ` Julien Grall
2013-05-30 15:15 ` Ian Campbell
1 sibling, 1 reply; 8+ messages in thread
From: Julien Grall @ 2013-05-30 14:38 UTC (permalink / raw)
To: xen-devel; +Cc: patches, tim, ian.campbell, Julien Grall, Stefano.Stabellini
We can safely remove VFP support in XEN because:
- the guest will enable VFP support when a process requires it
- XEN doesn't use VFP
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
xen/arch/arm/Rules.mk | 2 +-
xen/arch/arm/setup.c | 2 --
xen/arch/arm/smpboot.c | 1 -
xen/include/asm-arm/vfp.h | 28 ----------------------------
4 files changed, 1 insertion(+), 32 deletions(-)
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 422ed04..a18e7fd 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -22,7 +22,7 @@ arm := y
ifeq ($(TARGET_SUBARCH),arm32)
# Prevent floating-point variables from creeping into Xen.
CFLAGS += -msoft-float
-CFLAGS += -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
+CFLAGS += -mcpu=cortex-a15
arm32 := y
arm64 := n
endif
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index da2a734..2df091e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -457,8 +457,6 @@ void __init start_xen(unsigned long boot_phys_offset,
setup_virt_paging();
- enable_vfp();
-
softirq_init();
tasklet_subsys_init();
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 8011987..67e20d0 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -153,7 +153,6 @@ void __cpuinit start_secondary(unsigned long boot_phys_offset,
setup_virt_paging();
mmu_init_secondary_cpu();
- enable_vfp();
gic_init_secondary_cpu();
diff --git a/xen/include/asm-arm/vfp.h b/xen/include/asm-arm/vfp.h
index 6ba3cd1..fcffdf2 100644
--- a/xen/include/asm-arm/vfp.h
+++ b/xen/include/asm-arm/vfp.h
@@ -11,34 +11,6 @@
# error "Unknown ARM variant"
#endif
-#ifdef CONFIG_ARM_32
-
-#define FPEXC_EN (1u << 30)
-
-/* Save and restore FP state.
- * Ought to be using the new vmrs/vmsr names, but older binutils has a
- * bug where it only allows them to target fpscr (and not, say, fpexc). */
-#define READ_FP(reg) ({ \
- uint32_t val; \
- asm volatile ("fmrx %0, fp" #reg : "=r" (val)); \
- val; })
-
-#define WRITE_FP(reg, val) do { \
- asm volatile ("fmxr fp" #reg ", %0" : : "r" (val)); \
-} while (0)
-
-/* Start-of-day: Turn on VFP */
-static inline void enable_vfp(void)
-{
- WRITE_FP(exc, READ_FP(exc) | FPEXC_EN);
-}
-#else
-static inline void enable_vfp(void)
-{
- /* Always enable on 64-bit */
-}
-#endif
-
void vfp_save_state(struct vcpu *v);
void vfp_restore_state(struct vcpu *v);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] xen/arm32: implement VFP context switch
2013-05-30 14:38 ` [PATCH 1/2] xen/arm32: implement VFP context switch Julien Grall
@ 2013-05-30 15:11 ` Ian Campbell
2013-05-30 15:25 ` Julien Grall
0 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2013-05-30 15:11 UTC (permalink / raw)
To: Julien Grall; +Cc: Stefano.Stabellini, tim, patches, xen-devel
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:
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot
2013-05-30 14:38 ` [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot Julien Grall
@ 2013-05-30 15:15 ` Ian Campbell
2013-05-30 15:38 ` Julien Grall
0 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2013-05-30 15:15 UTC (permalink / raw)
To: Julien Grall; +Cc: Stefano.Stabellini, tim, patches, xen-devel
On Thu, 2013-05-30 at 15:38 +0100, Julien Grall wrote:
> We can safely remove VFP support in XEN because:
> - the guest will enable VFP support when a process requires it
> - XEN doesn't use VFP
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
> xen/arch/arm/Rules.mk | 2 +-
> xen/arch/arm/setup.c | 2 --
> xen/arch/arm/smpboot.c | 1 -
> xen/include/asm-arm/vfp.h | 28 ----------------------------
> 4 files changed, 1 insertion(+), 32 deletions(-)
>
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 422ed04..a18e7fd 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -22,7 +22,7 @@ arm := y
> ifeq ($(TARGET_SUBARCH),arm32)
> # Prevent floating-point variables from creeping into Xen.
> CFLAGS += -msoft-float
> -CFLAGS += -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
> +CFLAGS += -mcpu=cortex-a15
Does removing this have the side effect that if we accidentally add a
floating point operation to the hypervisor (very easy to do) it will now
compile with h/w fp, unlike previously where it would generate a call to
a non-existent library call and fail the build?
Since using vfp inside the hypervisor would require more supporting code
failing to build is quite useful.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] xen/arm32: implement VFP context switch
2013-05-30 15:11 ` Ian Campbell
@ 2013-05-30 15:25 ` Julien Grall
0 siblings, 0 replies; 8+ messages in thread
From: Julien Grall @ 2013-05-30 15:25 UTC (permalink / raw)
To: Ian Campbell; +Cc: Stefano.Stabellini, tim, patches, xen-devel
On 05/30/2013 04:11 PM, Ian Campbell wrote:
> 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?
Lost line. I will remove it.
>
>> #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?
Yes. It was lost during the creation of the patch. I will pay more
attention in the next patch series.
>
>> +# error "Unknown ARM variant"
>> +#endif
>>
>> #ifdef CONFIG_ARM_32
>
> I suppose the content of this #if now belongs in the subarch header.
This part is removed in the next patch. I was thinking to invert the 2
patch as the second patch doesn't need the first one (except for the diff).
>
>>
>> @@ -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:
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot
2013-05-30 15:15 ` Ian Campbell
@ 2013-05-30 15:38 ` Julien Grall
2013-05-30 15:43 ` Ian Campbell
0 siblings, 1 reply; 8+ messages in thread
From: Julien Grall @ 2013-05-30 15:38 UTC (permalink / raw)
To: Ian Campbell; +Cc: Stefano.Stabellini, tim, patches, xen-devel
On 05/30/2013 04:15 PM, Ian Campbell wrote:
> On Thu, 2013-05-30 at 15:38 +0100, Julien Grall wrote:
>> We can safely remove VFP support in XEN because:
>> - the guest will enable VFP support when a process requires it
>> - XEN doesn't use VFP
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>> xen/arch/arm/Rules.mk | 2 +-
>> xen/arch/arm/setup.c | 2 --
>> xen/arch/arm/smpboot.c | 1 -
>> xen/include/asm-arm/vfp.h | 28 ----------------------------
>> 4 files changed, 1 insertion(+), 32 deletions(-)
>>
>> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
>> index 422ed04..a18e7fd 100644
>> --- a/xen/arch/arm/Rules.mk
>> +++ b/xen/arch/arm/Rules.mk
>> @@ -22,7 +22,7 @@ arm := y
>> ifeq ($(TARGET_SUBARCH),arm32)
>> # Prevent floating-point variables from creeping into Xen.
>> CFLAGS += -msoft-float
>> -CFLAGS += -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
>> +CFLAGS += -mcpu=cortex-a15
>
> Does removing this have the side effect that if we accidentally add a
> floating point operation to the hypervisor (very easy to do) it will now
> compile with h/w fp, unlike previously where it would generate a call to
> a non-existent library call and fail the build?
Yes. GCC will notify that VFP support is not enabled, except if we use
directly mrc and mcr.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot
2013-05-30 15:38 ` Julien Grall
@ 2013-05-30 15:43 ` Ian Campbell
0 siblings, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2013-05-30 15:43 UTC (permalink / raw)
To: Julien Grall; +Cc: Stefano.Stabellini, tim, patches, xen-devel
On Thu, 2013-05-30 at 16:38 +0100, Julien Grall wrote:
> On 05/30/2013 04:15 PM, Ian Campbell wrote:
>
> > On Thu, 2013-05-30 at 15:38 +0100, Julien Grall wrote:
> >> We can safely remove VFP support in XEN because:
> >> - the guest will enable VFP support when a process requires it
> >> - XEN doesn't use VFP
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> ---
> >> xen/arch/arm/Rules.mk | 2 +-
> >> xen/arch/arm/setup.c | 2 --
> >> xen/arch/arm/smpboot.c | 1 -
> >> xen/include/asm-arm/vfp.h | 28 ----------------------------
> >> 4 files changed, 1 insertion(+), 32 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> >> index 422ed04..a18e7fd 100644
> >> --- a/xen/arch/arm/Rules.mk
> >> +++ b/xen/arch/arm/Rules.mk
> >> @@ -22,7 +22,7 @@ arm := y
> >> ifeq ($(TARGET_SUBARCH),arm32)
> >> # Prevent floating-point variables from creeping into Xen.
> >> CFLAGS += -msoft-float
> >> -CFLAGS += -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
> >> +CFLAGS += -mcpu=cortex-a15
> >
> > Does removing this have the side effect that if we accidentally add a
> > floating point operation to the hypervisor (very easy to do) it will now
> > compile with h/w fp, unlike previously where it would generate a call to
> > a non-existent library call and fail the build?
>
>
> Yes. GCC will notify that VFP support is not enabled,
Great!
> except if we use directly mrc and mcr.
Lets not do that!
Ian.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-05-30 15:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-30 14:38 [PATCH 0/2] Implement VFP context switch for arm32 Julien Grall
2013-05-30 14:38 ` [PATCH 1/2] xen/arm32: implement VFP context switch Julien Grall
2013-05-30 15:11 ` Ian Campbell
2013-05-30 15:25 ` Julien Grall
2013-05-30 14:38 ` [PATCH 2/2] xen/arm: don't enable VFP on XEN during the boot Julien Grall
2013-05-30 15:15 ` Ian Campbell
2013-05-30 15:38 ` Julien Grall
2013-05-30 15:43 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.