On 30.07.2012, at 16:46, Christian Borntraeger wrote:
> On 30/07/12 15:24, Alexander Graf wrote:
>
> Thanks for the review.
>
> Here is just a short answer regarding the interrupt, we will adress the other
> comments in a later
> mail.
>
>>> void sclp_service_interrupt(uint32_t sccb)
>>> {
>>> - if (!sccb) {
>>> + SCLPEventFacility *ef = sbus->event_facility->instance;
>>> + int event_pending = sbus->event_facility->event_pending(ef);
>>> +
>>> + if (!event_pending && !sccb) {
>>> return;
>>> }
>>
>> Uh. So when there's no event pending no sclp call may trigger an interrupt?
>
> No. If there is no event pending AND no sccb was specified, then we have
> nothing to report
> --> no interrupt.
Oh. My bad :).
>
> An service interrupt has a 32bit value as payload. Bits 0-28 (in IBM speak,
> 31-3 in Linux speak)
> contain the sccb address (if any), the other bits are special. The LSB
> describes if there are still
> events pending.
>
> So an unsolicited interrupt will only set the LSB.
> An answer to a service call will always contain the sccb and might have the
> lsb set if there are
> events pending.
Ok, so this really is a bit we're trying to set then, not an integer we want to
add.
>
>
>>> - s390_sclp_extint(sccb & ~3);
>>> + s390_sclp_extint((sccb & ~3) + event_pending);
>>
>> event_pending returns a bool, right? Please make this code a bit less
>> magical :).
>
> Something like "event_pending?1:0" ?
Something like
param = sccb & ~3;
/* Indicate whether an event is still pending */
param |= event_pending ? 1 : 0;
if (!param) {
/* No need to send an interrupt, there's nothing to be notified about */
return;
}
s390_sclp_extint(param);
Alex
