Re: [Xen-devel] [PATCH 02/18] xen/arm: Implement PSCI system suspend call (virtual interface)

[Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>]

On 11/13/2018 04:09 PM, Julien Grall wrote:
> On 13/11/2018 10:23, Julien Grall wrote:
> > Hi,
> >
> > On 12/11/2018 11:30, Mirela Simonovic wrote:
> > > +/*
> > > + * This function sets the context of current VCPU to the state 
> > > which is expected
> > > + * by the guest on resume. The expected VCPU state is:
> > > + * 1) pc to contain resume entry point (1st argument of PSCI 
> > > SYSTEM_SUSPEND)
> > > + * 2) r0/x0 to contain context ID (2nd argument of PSCI 
> > > SYSTEM_SUSPEND)
> > > + * 3) All other general purpose and system registers should have 
> > > reset values
> > > + *
> > > + * Note: this function has to return void because it has to always 
> > > succeed. In
> > > + * other words, this function is called from virtual PSCI 
> > > SYSTEM_SUSPEND
> > > + * implementation, which can return only a limited number of 
> > > possible errors,
> > > + * none of which could represent the fact that an error occurred 
> > > when preparing
> > > + * the domain for suspend.
> > > + * Consequently, dynamic memory allocation cannot be done within 
> > > this function,
> > > + * because if malloc fails the error has nowhere to propagate.
> > > + */
> > > +static void vcpu_suspend(register_t epoint, register_t cid)
> > > +{
> > > +    /* Static allocation because dynamic would need a non-void 
> > > return */
> > > +    static struct vcpu_guest_context ctxt;
> > > +    struct vcpu *v = current;
> > > +
> > > +    /* Make sure that VCPU guest regs are zeroied */
> > > +    memset(&ctxt, 0, sizeof(ctxt));
> > > +
> > > +    /* Set non-zero values to the registers prior to copying */
> > > +    ctxt.user_regs.pc64 = (u64)epoint;
> > > +
> > > +    if ( is_32bit_domain(current->domain) )
> > > +    {
> > > +        ctxt.user_regs.r0_usr = cid;
> > > +        ctxt.user_regs.cpsr = PSR_GUEST32_INIT;
> > This is going to disable the MMU and Cache as requested by the PSCI 
> > spec. As the guest is not required to clean the cache when turning 
> > off the CPU/suspending, the data may not have reached the main memory.
> > So do you need to perform cache maintenance to avoid stale information?
> Answering to myself, I have discussed about the cache with others Arm 
> folks today. SYSTEM_SUSPEND may exit early due to a pending event (BTW 
> you don't seem to handle it) and could jump to the specified entry 
> point address.
I believe it is handled - with the vcpu_block_unless_event_pending in 
domain_suspend
> In that case, the only guarantee is the data would not be lost, yet 
> can still be in the cache. This means that any data used before the 
> MMU & cache are enabled in the entry point should have been cleaned to 
> PoC and if you modify data make sure the cache is invalidated.
> Linux is doing that properly. Looking at Xen, we don't really properly 
> handle the cache in the boot path. So you may randomly crash.
> Cheers,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel