Re: [kvm-unit-tests PATCH 4/7] s390x: css: registering IRQ

[Pierre Morel <pmorel@linux.ibm.com>]

On 11/3/21 09:01, Thomas Huth wrote:
> On 27/08/2021 12.17, Pierre Morel wrote:
>> Registering IRQ for the CSS level.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h     | 21 +++++++++++++++++++++
>>   lib/s390x/css_lib.c | 27 +++++++++++++++++++++++++--
>>   2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
>> index 2005f4d7..0422f2e7 100644
>> --- a/lib/s390x/css.h
>> +++ b/lib/s390x/css.h
>> @@ -402,4 +402,25 @@ struct measurement_block_format1 {
>>       uint32_t irq_prio_delay_time;
>>   };
>> +#include <asm/arch_def.h>
>> +static inline void disable_io_irq(void)
>> +{
>> +    uint64_t mask;
>> +
>> +    mask = extract_psw_mask();
>> +    mask &= ~PSW_MASK_IO;
>> +    load_psw_mask(mask);
>> +}
>> +
>> +static inline void enable_io_irq(void)
>> +{
>> +    uint64_t mask;
>> +
>> +    mask = extract_psw_mask();
>> +    mask |= PSW_MASK_IO;
>> +    load_psw_mask(mask);
>> +}
>> +
>> +int register_css_irq_func(void (*f)(void));
>> +int unregister_css_irq_func(void (*f)(void));
>>   #endif
>> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
>> index 484f9c41..a89fc93c 100644
>> --- a/lib/s390x/css_lib.c
>> +++ b/lib/s390x/css_lib.c
>> @@ -350,8 +350,29 @@ bool css_disable_mb(int schid)
>>       return retry_count > 0;
>>   }
>> -static struct irb irb;
>> +static void (*css_irq_func)(void);
>> +
>> +int register_css_irq_func(void (*f)(void))
>> +{
>> +    if (css_irq_func)
>> +        return -1;
>> +    css_irq_func = f;
>> +    assert(register_io_int_func(css_irq_io) == 0);
> 
> It's unlikely that we ever disable assert() in the k-u-t, but anyway, 
> I'd prefer not to put function calls within assert() statements, just in 
> case. So could you please replace this with:
> 
>      rc = register_io_int_func(css_irq_io);
>      assert(rc == 0);
>  > ?

OK

> 
>> +    enable_io_isc(0x80 >> IO_SCH_ISC);
>> +    return 0;
>> +}
>> +int unregister_css_irq_func(void (*f)(void))
>> +{
>> +    if (css_irq_func != f)
>> +        return -1;
>> +    enable_io_isc(0);
>> +    unregister_io_int_func(css_irq_io);
>> +    css_irq_func = NULL;
>> +    return 0;
>> +}
>> +
>> +static struct irb irb;
>>   void css_irq_io(void)
>>   {
>>       int ret = 0;
>> @@ -386,7 +407,9 @@ void css_irq_io(void)
>>           report(0, "tsch reporting sch %08x as not operational", sid);
>>           break;
>>       case 0:
>> -        /* Stay humble on success */
>> +        /* Call upper level IRQ routine */
>> +        if (css_irq_func)
>> +            css_irq_func();
>>           break;
>>       }
>>   pop:
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 

Thanks,
Pierre


-- 
Pierre Morel
IBM Lab Boeblingen