Re: [PATCH v2 04/12] KVM: s390: implement GISA IPM related primitives

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 01/25/2018 03:06 PM, David Hildenbrand wrote:
> On 25.01.2018 14:28, 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 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index b7b2865..d559776 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
>> @@ -203,6 +203,28 @@ static inline u8 int_word_to_isc(u32 int_word)
>>  	return (int_word & 0x38000000) >> 27;
>>  }
>>  
> 
> Can we add a comment like
> 
> /*
>  * To use atomic bitmap functions, we have to provide a bitmap address
>  * that is u64 aligned. However, the ipm might be u32 aligned.
>  * Therefore, we logically start the bitmap at the very beginning of the
>  * struct and fixup the bit number.
>  */

Yes, makes sense, will add.

> 
>> +#define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * BITS_PER_BYTE)
>> +
>> +static inline void kvm_s390_gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
>> +{
>> +	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
>> +}
>> +
>> +static inline u8 kvm_s390_gisa_get_ipm(struct kvm_s390_gisa *gisa)
>> +{
>> +	return (u8) READ_ONCE(gisa->ipm);
> 
> Is this cast necessary? (ipm is defined as u8)

I think it can go. will fix.

> 
>> +}
>> +
>> +static inline void kvm_s390_gisa_clear_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
> 
> wonder if an u8 for gisc would be better.

isc is a 32bit value throughout that code, so lets keep it here as well.

> 
>> +{
>> +	clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
>> +}
>> +
>> +static inline int kvm_s390_gisa_tac_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
>> +{
>> +	return test_and_clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
>> +}
>> +
>>  static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
>>  {
>>  	return vcpu->kvm->arch.float_int.pending_irqs |
>> @@ -2682,3 +2704,4 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
>>  
>>  	return n;
>>  }
>> +
>>
> 
> Unrelated whitespace change.

Will fixup. 




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux