Re: [Xen-devel] [PATCH v2 2/3] x86/pt: enable binding of GSIs to a PVH Dom0

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Tue, May 16, 2017 at 10:23:48AM -0600, Jan Beulich wrote:
> >>> On 16.05.17 at 17:55, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, May 12, 2017 at 07:42:51AM -0600, Jan Beulich wrote:
> >> >>> On 19.04.17 at 17:11, <roger.pau@xxxxxxxxxx> wrote:
> >> > Note that currently there's no support for unbinding this interrupts.
> >> 
> >> Do you plan to deal with that before this changes goes in? Aiui this
> >> not working means you can't pass through devices with pin based
> >> interrupts once Dom0 chose to bind to them. Otoh hand you modify
> >> pt_irq_destroy_bind(), so I'm a little puzzled ...
> > 
> > Yes, I modify pt_irq_destroy_bind to return EOPNOTSUPP when trying to unbind
> > such an interrupt. I can implement the unbind, but it's not going to be used
> > ATM.
> 
> Is it not? I can see the mentioned pass-through case to be of no
> interest, but wouldn't a well behaved kernel perhaps unmap IRQs
> while shutting down?

I guess I haven't explained myself correctly, what I meant is that right now I
don't have any use-case for this, I haven't started working on pci-passtrhough
for guests, and the Dom0 implementation I have doesn't unbind interrupts on
shutdown.

I could unbind them when Dom0 masks the vIO APIC pin, but I think that's
going to be awfully slow.

> >> > +    spin_lock(&d->arch.hvm_domain.irq_lock);
> >> > +    if ( !hvm_irq->gsi_assert_count[gsi] )
> >> > +    {
> >> > +        hvm_irq->gsi_assert_count[gsi]++;
> >> 
> >> Why is this an increment instead of a simple write of 1? Or the
> >> other way around - why is this not always incrementing, just like
> >> __hvm_pci_intx_assert() does? (Symmetric questions then for
> >> hvm_gsi_deassert()).
> > 
> > __hvm_pci_intx_{de}assert has an array that tracks the status of each 
> > interrupt
> > line, and Xen does the routing based on that (the __test_and_clear_bit at 
> > the
> > top of __hvm_pci_intx_assert). That prevents the same line from triggering
> > multiple times, which is not available here, and thus Xen needs to rely on
> > gsi_assert_count in order to know if the GSI is pending or not.
> > 
> > Switched to use a set to 1/0 instead of the increment, which I agree makes 
> > this
> > clearer.
> 
> And altogether this likely would benefit from a comment put
> somewhere.

Done.

Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel