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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel