[PATCH] target-arm: Bug fix in filling the cp_regs hashtable

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] target-arm: Bug fix in filling the cp_regs hashtable

Abdallah Bouassida
Check if the CPU supports AARCH64 before adding  the 64bit view of
the coprocessor's register to the cp_regs hashtable.

Signed-off-by: Abdallah Bouassida <[hidden email]>
---
Bug description: if a register has the .state = ARM_CP_STATE_BOTH, its
64bit view will be added to the hashtable even if the CPU is not 64bit.

  target/arm/helper.c | 12 +++++++-----
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2594faa..7fa2889 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5607,11 +5607,13 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
                              break;
                          }
                      } else {
-                        /* AArch64 registers get mapped to non-secure
instance
-                         * of AArch32 */
-                        add_cpreg_to_hashtable(cpu, r, opaque, state,
-                                               ARM_CP_SECSTATE_NS,
-                                               crm, opc1, opc2);
+                        if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+                            /* AArch64 registers get mapped to
non-secure instance
+                             * of AArch32 */
+                            add_cpreg_to_hashtable(cpu, r, opaque, state,
+ ARM_CP_SECSTATE_NS,
+                                                   crm, opc1, opc2);
+                        }
                      }
                  }
              }
--
1.9.1


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] target-arm: Bug fix in filling the cp_regs hashtable

Peter Maydell-5
On 16 June 2017 at 15:42, Abdallah Bouassida
<[hidden email]> wrote:
> Check if the CPU supports AARCH64 before adding  the 64bit view of
> the coprocessor's register to the cp_regs hashtable.
>
> Signed-off-by: Abdallah Bouassida <[hidden email]>
> ---
> Bug description: if a register has the .state = ARM_CP_STATE_BOTH, its 64bit
> view will be added to the hashtable even if the CPU is not 64bit.

This is deliberate and required. Where the AArch64 and AArch32
states both have a register which shares underlying architectural
state, QEMU chooses to implement migration of that state usually
via the AArch64 version's ARMCPRegInfo struct. If the AArch64
version is not included in the hashtable for an AArch32-only
CPU then the state of the 32-bit register won't be migrated.
The AArch64 register is of course invisible to the guest because
it is only accessible via 64-bit instructions, but it's used
during migration.

thanks
-- PMM

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] target-arm: Bug fix in filling the cp_regs hashtable

Abdallah Bouassida
Oh, I see!
Thanks for the details!

Regards,
Abdallah

Le 6/16/2017 à 3:51 PM, Peter Maydell a écrit :

> On 16 June 2017 at 15:42, Abdallah Bouassida
> <[hidden email]> wrote:
>> Check if the CPU supports AARCH64 before adding  the 64bit view of
>> the coprocessor's register to the cp_regs hashtable.
>>
>> Signed-off-by: Abdallah Bouassida <[hidden email]>
>> ---
>> Bug description: if a register has the .state = ARM_CP_STATE_BOTH, its 64bit
>> view will be added to the hashtable even if the CPU is not 64bit.
> This is deliberate and required. Where the AArch64 and AArch32
> states both have a register which shares underlying architectural
> state, QEMU chooses to implement migration of that state usually
> via the AArch64 version's ARMCPRegInfo struct. If the AArch64
> version is not included in the hashtable for an AArch32-only
> CPU then the state of the 32-bit register won't be migrated.
> The AArch64 register is of course invisible to the guest because
> it is only accessible via 64-bit instructions, but it's used
> during migration.
>
> thanks
> -- PMM


Reply | Threaded
Open this post in threaded view
|

Re: [Qemu-arm] [PATCH] target-arm: Bug fix in filling the cp_regs hashtable

Philippe Mathieu-Daudé
In reply to this post by Peter Maydell-5
On 06/16/2017 11:51 AM, Peter Maydell wrote:

> On 16 June 2017 at 15:42, Abdallah Bouassida
> <[hidden email]> wrote:
>> Check if the CPU supports AARCH64 before adding  the 64bit view of
>> the coprocessor's register to the cp_regs hashtable.
>>
>> Signed-off-by: Abdallah Bouassida <[hidden email]>
>> ---
>> Bug description: if a register has the .state = ARM_CP_STATE_BOTH, its 64bit
>> view will be added to the hashtable even if the CPU is not 64bit.
>
> This is deliberate and required. Where the AArch64 and AArch32
> states both have a register which shares underlying architectural
> state, QEMU chooses to implement migration of that state usually
> via the AArch64 version's ARMCPRegInfo struct. If the AArch64
> version is not included in the hashtable for an AArch32-only
> CPU then the state of the 32-bit register won't be migrated.
> The AArch64 register is of course invisible to the guest because
> it is only accessible via 64-bit instructions, but it's used
> during migration.

explanation worth to go as comment in the source!