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

[Julien Grall <julien.grall@xxxxxxx>]

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?

> +
> +        /* Thumb set is allowed only for 32-bit domain */
> +        if ( epoint & 1 )
> +        {
> +            ctxt.user_regs.cpsr |= PSR_THUMB;
> +            ctxt.user_regs.pc64 &= ~(u64)1;
> +        }
> +    }
> +#ifdef CONFIG_ARM_64
> +    else
> +    {
> +        ctxt.user_regs.x0 = cid;
> +        ctxt.user_regs.cpsr = PSR_GUEST64_INIT;
Same here.

Cheers,

--
Julien Grall

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