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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 23 Jun 2014, Julien Grall wrote:
> 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.

Actually the spec says:

"5. The caller must ensure that appropriate wake-up events are enabled
to allow resumption from that state."

so maybe we could allow the guest kernel to shut itself in the foot and
avoiding clearing PSR_IRQ_MASK.


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