[v2,03/10] target-arm: allow modifying vfp fpexc en bit only

From: Juha Riihimäki <juha.riihimaki@nokia.com>

All other bits except for the EN in the VFP FPEXC register are defined
as subarchitecture specific and real functionality for any of the
other bits has not been implemented in QEMU. However, current code
allows modifying all bits in the VFP FPEXC register leading to
problems when guest code is writing 1's to the subarchitecture
specific bits and checking whether the bits stay up to verify the
existence of functionality which in fact does not exist in QEMU.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
---
 target-arm/translate.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

On Sat, Oct 24, 2009 at 1:19 PM,  <juha.riihimaki@nokia.com> wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>
> All other bits except for the EN in the VFP FPEXC register are defined
> as subarchitecture specific and real functionality for any of the
> other bits has not been implemented in QEMU. However, current code
> allows modifying all bits in the VFP FPEXC register leading to
> problems when guest code is writing 1's to the subarchitecture
> specific bits and checking whether the bits stay up to verify the
> existence of functionality which in fact does not exist in QEMU.

Shouldn't writes to FPEXC from gdb be protected in the same
way?  Except for that I agree with your patch.


Laurent

> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
> ---
>  target-arm/translate.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 09c996d..8cb1c0f 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -2788,6 +2788,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                         case ARM_VFP_FPEXC:
>                             if (IS_USER(s))
>                                 return 1;
> +                            /* TODO: VFP subarchitecture support.
> +                             * For now, keep the EN bit only */
> +                            tcg_gen_andi_i32(tmp, tmp, 1 << 30);
>                             store_cpu_field(tmp, vfp.xregs[rn]);
>                             gen_lookup_tb(s);
>                             break;
> --
> 1.6.5
>
>
>
>

On Oct 25, 2009, at 14:23, ext Laurent Desnogues wrote:

> On Sat, Oct 24, 2009 at 1:19 PM,  <juha.riihimaki@nokia.com> wrote:
>> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>>
>> All other bits except for the EN in the VFP FPEXC register are  
>> defined
>> as subarchitecture specific and real functionality for any of the
>> other bits has not been implemented in QEMU. However, current code
>> allows modifying all bits in the VFP FPEXC register leading to
>> problems when guest code is writing 1's to the subarchitecture
>> specific bits and checking whether the bits stay up to verify the
>> existence of functionality which in fact does not exist in QEMU.
>
> Shouldn't writes to FPEXC from gdb be protected in the same
> way?  Except for that I agree with your patch.

Please correct me if I'm wrong but it seems to me that the code in  
gdbstub.c never writes anything to the VFP registers, at least it  
seems to me the code in cpu_gdb_write_register and  
cpu_gdb_read_register ignore floating point registers.


Regards,
Juha

On Mon, Oct 26, 2009 at 8:32 AM,  <Juha.Riihimaki@nokia.com> wrote:
[...]
>>
>> Shouldn't writes to FPEXC from gdb be protected in the same
>> way?  Except for that I agree with your patch.
>
> Please correct me if I'm wrong but it seems to me that the code in
> gdbstub.c never writes anything to the VFP registers, at least it
> seems to me the code in cpu_gdb_write_register and
> cpu_gdb_read_register ignore floating point registers.

There's code in helper.c that implements writes to coprocessor
registers.  Look for vfp_gdb_set_reg which is registered by
cpu_arm_init.


Laurent