On 18.01.2018 15:29, Michael Mueller wrote:
>
>
> On 17.01.18 15:35, David Hildenbrand wrote:
>> On 16.01.2018 21:02, Christian Borntraeger wrote:
>>> From: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx>
>>>
>>> The patch implements routines to access the GISA to test and modify
>>> its Interruption Pending Mask (IPM) from the host side.
>>>
>>> Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxxxxxxx>
>>> Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>>> ---
>>> arch/s390/kvm/interrupt.c | 23 +++++++++++++++++++++++
>>> arch/s390/kvm/kvm-s390.h | 4 ++++
>>> 2 files changed, 27 insertions(+)
>>>
>>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>>> index b94173560dcf..dfdecff302d2 100644
>>> --- a/arch/s390/kvm/interrupt.c
>>> +++ b/arch/s390/kvm/interrupt.c
>>> @@ -2682,3 +2682,26 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
>>>
>>> return n;
>>> }
>>> +
>>> +#define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * 8)
>>
>> 8 -> BITS_PER_BYTE, but ...
>>
>> Am I wrong or can we only modify the 8 ipm bits this way? But we
>> want/have to do it in an atomic fashion?
>>
>> Using an unsigned long seems wrong, because we "rewrite" more than we
>> should. Esp. everything beyond ipm. oi / ni and friends are not
>> available on older machines.
>>
>> What about something as simple as the following
>>
>>
>> +void kvm_s390_gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u8 gisc)
>> +{
>> + u8 value = (0x80 >> gisc);
>> +
>> + __sync_fetch_and_or(&gisa->ipm, value);
>> +}
>> +
>>
>
> Nobody is using this compiler build-in in the kernel and a quick compile
> shows that it produces an ORK and CS instead of a LAOG beside a lot of
> supporting instructions. Unfortunately it's not saving what you promise
> ... ;) Will not change.
>
Using unsigned long * bitmap operations to modify an u8 type atomically
just seems very very wrong.
--
Thanks,
David / dhildenb
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
