diff mbox

[2/2] xen/arm: don't enable VFP on XEN during the boot

Message ID 1369924733-18701-3-git-send-email-julien.grall@linaro.org
State Superseded
Headers show

Commit Message

Julien Grall May 30, 2013, 2:38 p.m. UTC
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(-)

Comments

Ian Campbell May 30, 2013, 3:15 p.m. UTC | #1
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.
Julien Grall May 30, 2013, 3:38 p.m. UTC | #2
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.
Ian Campbell May 30, 2013, 3:43 p.m. UTC | #3
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.
diff mbox

Patch

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);