Re: [PATCH v2 RFC] KVM: s390/interrupt: do not pin adapter interrupt pages

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 12.02.20 13:39, Cornelia Huck wrote:
[...]

>> +	 */
>> +	return 0;
> 
> Given that this function now always returns 0, we basically get a
> completely useless roundtrip into the kernel when userspace is trying
> to setup the mappings.
> 
> Can we define a new IO_ADAPTER_MAPPING_NOT_NEEDED or so capability that
> userspace can check?

Nack. This is one system call per initial indicator ccw. This is so seldom
and cheap that I do not see a point in optimizing this. 


> This change in behaviour probably wants a change in the documentation
> as well.

Yep. 
[...]

>> @@ -2951,12 +2900,15 @@ int kvm_set_routing_entry(struct kvm *kvm,
>>  			  const struct kvm_irq_routing_entry *ue)
>>  {
>>  	int ret;
>> +	u64 uaddr;
>>  
>>  	switch (ue->type) {
>>  	case KVM_IRQ_ROUTING_S390_ADAPTER:
>>  		e->set = set_adapter_int;
>> -		e->adapter.summary_addr = ue->u.adapter.summary_addr;
>> -		e->adapter.ind_addr = ue->u.adapter.ind_addr;
>> +		uaddr =  gmap_translate(kvm->arch.gmap, ue->u.adapter.summary_addr);
> 
> Can gmap_translate() return -EFAULT here? The code above only seems to
> check for 0... do we want to return an error here?

Yes.

> 
>> +		e->adapter.summary_addr = uaddr;
>> +		uaddr =  gmap_translate(kvm->arch.gmap, ue->u.adapter.ind_addr);
>> +		e->adapter.ind_addr = uaddr;
>>  		e->adapter.summary_offset = ue->u.adapter.summary_offset;
>>  		e->adapter.ind_offset = ue->u.adapter.ind_offset;
>>  		e->adapter.adapter_id = ue->u.adapter.adapter_id;
>