Re: [PATCH for-4.14 v2 2/2] x86/passthrough: introduce a flag for GSIs not requiring an EOI or unmask

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jun 16, 2020 at 10:45:51AM +0200, Jan Beulich wrote:
> On 16.06.2020 10:37, Roger Pau Monné wrote:
> > On Tue, Jun 16, 2020 at 08:27:54AM +0200, Jan Beulich wrote:
> >> On 10.06.2020 16:29, Roger Pau Monne wrote:
> >>> @@ -920,6 +923,8 @@ static void hvm_dirq_assist(struct domain *d, struct 
> >>> hvm_pirq_dpci *pirq_dpci)
> >>>          if ( pirq_dpci->flags & HVM_IRQ_DPCI_IDENTITY_GSI )
> >>>          {
> >>>              hvm_gsi_assert(d, pirq->pirq);
> >>> +            if ( pirq_dpci->flags & HVM_IRQ_DPCI_NO_EOI )
> >>> +                goto out;
> >>
> >> Immediately ahead of this there's a similar piece of code
> >> dealing with PCI INTx. They're commonly level triggered, but
> >> I don't think there's a strict need for this to be the case.
> >> At least hvm_pci_intx_assert() -> assert_gsi() ->
> >> vioapic_irq_positive_edge() also cover the edge triggered one.
> > 
> > Hm, I'm not sure it's safe to passthrough edge triggered IO-APIC
> > interrupts, as Xen will mark those as 'shared' always, and sharing
> > edge interrupts cannot reliably work. In any case the EOI timer is
> > definitely set for those, and needs to be disabled before exiting
> > hvm_dirq_assist.
> 
> That's the
> 
>                 if ( !is_hardware_domain(d) )
>                     share = BIND_PIRQ__WILL_SHARE;
> 
> in pt_irq_create_bind() aiui? I wonder why we have that ... At a
> guess it's to accommodate pciback in Dom0 also registering a handler.
> But wasn't it XenoLinux'es pciback only that does so, and upstream's
> doesn't?

I'm not that familiar with pciback in Linux. I've taken a look and
AFAICT modern Linux kernels will register a handler for the PCI
interrupts before doing a device reset and when dealing with PV guests
that use the pciif protocol (note that such IRQ is also
unconditionally marked as shared in Linux).

It might be safe for HVM domU passed through interrupts to set the
share bit based on the triggering mode.

Roger.