Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support

[Alexander Graf]

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