Re: [Xen-devel] [PATCH] x86/pt: skip setup of posted format IRTE when gvec is 0

[Chao Gao <chao.gao@xxxxxxxxx>]

On Mon, May 06, 2019 at 03:39:40AM -0600, Jan Beulich wrote:
>>>> On 06.05.19 at 06:44, <chao.gao@xxxxxxxxx> wrote:
>> On Thu, May 02, 2019 at 10:20:09AM +0200, Roger Pau Monné wrote:
>>>Can you see about avoiding the XEN_DOMCTL_bind_pt_irq call in QEMU if
>>>the interrupt is going to be routed over an event channel?
>> 
>> Yes. It is doable. But it needs changes in both qemu and Xen and some tricks
>> to be compatible with old qemu.
>
>That would be ugly indeed.
>
>> I prefer not to touch qemu and keep qemu unware of MSI's "routing over 
>> evtchn",
>> like the patch below:
>
>Is this meant as a replacement to your original patch, or as an
>add-on? In any event it's not immediately clear to me how

A replacement.

>...
>
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -504,10 +504,7 @@ static long evtchn_bind_pirq(evtchn_bind_pirq_t *bind)
>>      if ( !info )
>>          ERROR_EXIT(-ENOMEM);
>>      info->evtchn = port;
>> -    rc = (!is_hvm_domain(d)
>> -          ? pirq_guest_bind(v, info,
>> -                            !!(bind->flags & BIND_PIRQ__WILL_SHARE))
>> -          : 0);
>> +    rc = pirq_guest_bind(v, info, !!(bind->flags & BIND_PIRQ__WILL_SHARE));
>
>... this becoming unconditional won't conflict with its other
>invocation ...

Yes. It conflicts with the call in pt_irq_create_bind() for non-MSI case.

>
>> --- a/xen/drivers/passthrough/io.c
>> +++ b/xen/drivers/passthrough/io.c
>> @@ -346,6 +346,12 @@ int pt_irq_create_bind(
>>          uint32_t gflags = pt_irq_bind->u.msi.gflags &
>>                            ~XEN_DOMCTL_VMSI_X86_UNMASKED;
>>  
>> +        if ( !pt_irq_bind->u.msi.gvec )
>> +        {
>> +            spin_unlock(&d->event_lock);
>> +            return 0;
>> +        }
>
>... further down in this function, for the non-MSI case.
>Similarly I wonder whether the respective unbind function
>invocations then won't go (or already are?) out of sync.

The "out of sync" issue seems hard to be solved. It is error-prone to
move pirq_guest_(un)bind from one hypercall to another.

On second thought, I plan to go back to my original patch. The only
issue for that patch is how to migrate irq properly to avoid IPI during
interrupt delivery.

Actually, current code has set irq affinity correctly:
1. pirq is bound to vcpu[0] in pt_irq_create_bind(). It also sets
corresponding physical irq's affinity to vcpu[0].
2. evtchn is bound to vcpu[0] in evtchn_bind_pirq(). During
delivery, we would send pirq to vcpu[0] and no IPI is required.
3. If evtchn is rebound to another vcpu in evtchn_bind_vcpu(), the
affinity of the physical irq is already reconfigured there.

Thanks
Chao

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel