Re: [PATCH v2 04/19] xen/arm: Restore HCR_EL2 register

[Julien Grall <julien.grall@arm.com>]

Hi Wei,

On 03/31/2017 03:10 AM, Wei Chen wrote:
> Hi Julien and Stefano,
>
> On 2017/3/31 6:03, Stefano Stabellini wrote:
>> On Thu, 30 Mar 2017, Julien Grall wrote:
>>> Hi Wei,
>>>
>>> On 30/03/17 10:13, Wei Chen wrote:
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index de59e5f..8af223e 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -2171,6 +2171,13 @@ int construct_dom0(struct domain *d)
>>>>          return rc;
>>>>
>>>>      /*
>>>> +     * The HCR_EL2 will temporarily switch to dom0's HCR_EL2 value
>>>> +     * by p2m_restore_state. We have to save HCR_EL2 to idle vCPU's
>>>> +     * context for restoring it in later.
>>>> +     */
>>>> +    current->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
>>>
>>> I don't understand why we care here. idle vCPU will never restore HCR_EL2 nor
>>> return from the hypervisor.
>>
>> I don't understand this either
>>
>
> Yes, idle vCPU will never return from hypervisor. But in construct_dom0,
> it has one chance to restore HCR_EL2.
> In in construct_dom0 we will call p2m_restore_state twice.
>      saved_current = current;
>      p2m_restore_state(v); --->> This is dom0's vCPU0
>      [...]
>      set_current(saved_current);
>      p2m_restore_state(saved_current); --->> this is idle vCPU0
>
> In p2m_restore_state, we will write vcpu->arch.hcr_el2 to HCR_EL2. in
> this case, we will write an unknown value to HCR_EL2. Even though this
> would not cause any problem from my current testing. But from code
> scope, I think it would be a drawback.

The p2m_restore_state will do nothing for idle vCPU (see check at the 
beginning of the function). So there are no issue.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel