Re: [PATCH 4/8] arm: Synchronize CPU on PSCI on

[Alexander Graf <agraf@csgraf.de>]

On 26.11.20 22:47, Peter Maydell wrote:
> On Thu, 26 Nov 2020 at 21:36, Alexander Graf <agraf@csgraf.de> wrote:
>> We are going to reuse the TCG PSCI code for HVF. This however means that we
>> need to ensure that CPU register state is synchronized properly between the
>> two worlds.
>>
>> So let's make sure that at least on the PSCI on call, the secondary core gets
>> to sync its registers after reset, so that changes also propagate.
>>
>> Signed-off-by: Alexander Graf <agraf@csgraf.de>
>> ---
>>   target/arm/arm-powerctl.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
>> index b75f813b40..256f7cfdcd 100644
>> --- a/target/arm/arm-powerctl.c
>> +++ b/target/arm/arm-powerctl.c
>> @@ -15,6 +15,7 @@
>>   #include "arm-powerctl.h"
>>   #include "qemu/log.h"
>>   #include "qemu/main-loop.h"
>> +#include "sysemu/hw_accel.h"
>>
>>   #ifndef DEBUG_ARM_POWERCTL
>>   #define DEBUG_ARM_POWERCTL 0
>> @@ -66,6 +67,8 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
>>       cpu_reset(target_cpu_state);
>>       target_cpu_state->halted = 0;
>>
>> +    cpu_synchronize_state(target_cpu_state);
>> +
>>       if (info->target_aa64) {
>>           if ((info->target_el < 3) && arm_feature(&target_cpu->env,
>>                                                    ARM_FEATURE_EL3))
> This looks weird. The CPU was off, so not running anything.
> Why doesn't the state we set up here get synchronized to
> HVF as part of the normal enter-guest-code process that we
> do when we do whatever HVF's equivalent of KVM_RUN is ?
>
> Also, we change more bits of CPU state later in this function,
> so if we do need to manually sychronize in this function this
> doesn't seem like the right place...


cpu_synchronize_state() sets the CPU registers into "dirty" state which 
means that env now holds the current copy. On the next entry, we then 
sync them back into HVF.

Without the cpu_synchronize_state() call, HVF never knows that the CPU 
state is actually dirty. I guess it could as well live in cpu_reset() 
somewhere, but we have to get the state switched over to dirty one way 
or another.

One interesting thing to note here is that the CPU actually comes up in 
"dirty" after init. But init is done on realization already. I'm not 
sure why we lose the dirty state in between that and the reset.


Alex