Re: [Xen-devel] [PATCH 02/18] xen/arm: Restore HCR_EL2 register

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

On 2017/3/15 19:12, Julien Grall wrote:
> Hi Wei,
>
> On 15/03/17 08:34, Wei Chen wrote:
>> On 2017/3/15 8:25, Stefano Stabellini wrote:
>>> On Mon, 13 Mar 2017, Wei Chen wrote:
>>>> Different domains may have different HCR_EL2 flags. For example, the
>>>> 64-bit domain needs HCR_RW flag but the 32-bit does not need it. So
>>>> we give each domain a default HCR_EL2 value and save it in the VCPU's
>>>> context.
>>>>
>>>> HCR_EL2 register has only one bit can be updated automatically without
>>>> explicit write (HCR_VSE). But we haven't used this bit currently, so
>>>> we can consider that the HCR_EL2 register will not be modified while
>>>> the guest is running. So save the HCR_EL2 while guest exiting to
>>>> hypervisor is not neccessary. We just have to restore this register for
>>>> each VCPU while leaving hypervisor.
>>>>
>>>> We prefer to restore HCR_EL2 in leave_hypervisor_tail rather than in
>>>> ctxt_switch_to. Because the leave_hypervisor_tail is the closest place
>>>> to the exception return. In this case, we don't need to warrant the
>>>> HCR_EL2 will not be changed between ctxt_switch_to and exception return.
>>>
>>> Please explain a bit better why it is good to restore HCR_EL2 in
>>> leave_hypervisor_tail. Why is it a good thing that is closer to the
>>> exception return? What can be the cause of a change in HCR_EL2?
>>>
>>
>> Ok, I will try to improve it in next version. In current Xen code, I
>> can't see any code would change the HCR_EL2 between ctxt_switch_to and
>> return_from_trap. But my concern is that, in the future, if someone
>> want to insert some HCR_EL2 change code between them, we can't guarantee
>> he will restore correct HCR_EL2 value for current vcpu.
>
> Well, the purpose of this series is to inject a virtual SError to the
> guest when a host SError is happening. The host SError will be received
> in the hypervisor whilst the vCPU is running and no context switch (e.g
> call to ctxt_switch) may happen if the scheduler decides to keep the
> vCPU running.
>
> Also, one could argue that HCR_EL2 could be modified on-fly in the
> function but we may have other places in the future which will modify
> HCR_EL2. For instance, this would be the case when the monitor app will
> gain support of inspecting some registers.
>
> So we want a single place to restore HCR_EL2. And leave_hypervisor_tail
> is the best place for that.
>

Thanks! I will reference this comment to the commit message in next
version.

> Cheers,
>


-- 
Regards,
Wei Chen

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