Re: [PATCH RFC] xen/arm : emulation of arm's psci v0.2 standard

[Julien Grall <julien.grall@linaro.org>]

On 06/23/2014 11:40 AM, Stefano Stabellini wrote:
> On Mon, 23 Jun 2014, Parth Dixit wrote:
>> Next version of my patch is ready except for following things on which i need your suggestion
>> 1. Exposing PSCI v0.2 functions in device tree - This was not done because it gives the impression that you can modify the function id's
>> and kernel will call the function id's based on function id's exposed in device tree whereas kernel ignores it for PSCI v0.2 while it
>> follows it for PSCI v0.1 which can be confusing. Either way is fine with me.
>> 2.  Why do you clear the IRQ flag in psci_suspend - I am taking cue from the "vcpu_block_enable_events" in xen/common/schedule.c where
>> flag is cleared to enable interrupts before pausing the cpu.
> 
> Keep in mind that vcpu_block_enable_events is common code, while
> local_event_delivery_enable is the arm specific implementation.
> 
> In the arm case local_event_delivery_enable is implemented by clearing
> PSR_IRQ_MASK because effectively that's what is needed to enable event
> delivery. Events are just a Xen specific kind of interrupts.
> 
> vcpu_block_enable_events calls local_event_delivery_enable before
> blocking a vcpu, to make sure it can wake the vcpu up if an event needs
> to be delivered to it.
> 
> We need to clear PSR_IRQ_MASK because the CPU_SUSPEND call "is intended
> for use in idle subsystems where the core is expected to return to
> execution through a wake up event". The vcpu is never going to come up
> again if we don't clear PSR_IRQ_MASK, because events wouldn't be
> delivered to it.

With this solution Xen will return into the guest with IRQ enable
unconditionally.

I don't see anything in the specification that allow a such change. So
the guest may assume that the IRQs are still disabled. This would break it.

Couldn't we use the same trick as WFI ie:

vcpu_block();
if ( local_events_delivery_nomask() )
  vcpu_unblock(current);

It might be better to introduce a new helper for this purpose.

Regards,

-- 
Julien Grall