The aibvo of zpci device should be constant after issued mpcifc
registering irqs instruction. Each msix vector should offset from the
aibvo. But for flic adapter interrupt, we should use the absolute
offset within the aibv. So let's use the aibvo+vector to fixup msix
routing entry.

Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
---
 target/s390x/kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index e348bfb7cc..c08b7757e7 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2515,14 +2515,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
         return -ENODEV;
     }
 
-    pbdev->routes.adapter.ind_offset = vec;
-
     route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
     route->flags = 0;
     route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
     route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
     route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
-    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
+    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset + vec;
     route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
     return 0;
 }
-- 
2.11.0 (Apple Git-81)

On Mon, 28 Aug 2017 10:04:46 +0200
Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:

> The aibvo of zpci device should be constant after issued mpcifc
> registering irqs instruction. Each msix vector should offset from the
> aibvo. But for flic adapter interrupt, we should use the absolute
> offset within the aibv. So let's use the aibvo+vector to fixup msix
> routing entry.

This makes sense, but I would tweak the description a bit.

"The guest uses the mpcifc instruction to register the aibvo of a zpci
device, which is the starting offset of indicators in the indicator
area and thus remains constant. Each msix vector is an offset from the
aibvo. When we map a msix route to an adapter route, we should not
modify the starting offset, but instead add the vector to the starting
offset to get the absolute offset in the specific route."

I'm wondering how this was ever supposed to work?

> 
> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> ---
>  target/s390x/kvm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index e348bfb7cc..c08b7757e7 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2515,14 +2515,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>          return -ENODEV;
>      }
>  
> -    pbdev->routes.adapter.ind_offset = vec;
> -
>      route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
>      route->flags = 0;
>      route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
>      route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
>      route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
> -    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
> +    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset + vec;
>      route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
>      return 0;
>  }

在 2017/8/28 下午11:33, Cornelia Huck 写道:
> On Mon, 28 Aug 2017 10:04:46 +0200
> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>
>> The aibvo of zpci device should be constant after issued mpcifc
>> registering irqs instruction. Each msix vector should offset from the
>> aibvo. But for flic adapter interrupt, we should use the absolute
>> offset within the aibv. So let's use the aibvo+vector to fixup msix
>> routing entry.
> This makes sense, but I would tweak the description a bit.
>
> "The guest uses the mpcifc instruction to register the aibvo of a zpci
> device, which is the starting offset of indicators in the indicator
> area and thus remains constant. Each msix vector is an offset from the
> aibvo. When we map a msix route to an adapter route, we should not
> modify the starting offset, but instead add the vector to the starting
> offset to get the absolute offset in the specific route."
Much better. Thanks!
>
> I'm wondering how this was ever supposed to work?
I investigated this. Linux kernel always uses 0 as starting offset for
aibvo. And each msix entry is only registered one time. So we didn't
encounter any problem. But the logic here is not right obviously. It
is just a coincidence.

>
>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>> ---
>>   target/s390x/kvm.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index e348bfb7cc..c08b7757e7 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -2515,14 +2515,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>>           return -ENODEV;
>>       }
>>   
>> -    pbdev->routes.adapter.ind_offset = vec;
>> -
>>       route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
>>       route->flags = 0;
>>       route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
>>       route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
>>       route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
>> -    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
>> +    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset + vec;
>>       route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
>>       return 0;
>>   }
>