lkml.org 
[lkml]   [2013]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/3] PCI: Refactor MSI/MSIX mask restore code to fix interrupt lost issue
Date
On Wednesday, October 16, 2013 3:33 PM, Zhenzhong Duan wrote:
>
> Driver init call graph under baremetal:
> driver_init->
> msix_capability_init->
> msix_program_entries->
> msix_mask_irq->
> entry->masked = 1
> request_irq->
> __setup_irq->
> irq_startup->
> unmask_msi_irq->
> msix_mask_irq->
> entry->masked = 0;
>
> So entry->masked is always updated with newest value and its value could be used
> to restore to mask register in device.
>
> But in initial domain (aka priviliged guest), it's different.
> Driver init call graph under initial domain:
> driver_init->
> msix_capability_init->
> msix_program_entries->
> msix_mask_irq->
> entry->masked = 1
> request_irq->
> __setup_irq->
> irq_startup->
> __startup_pirq->
> EVTCHNOP_bind_pirq hypercall (trap into Xen)
> [Xen:]
> pirq_guest_bind->
> startup_msi_irq->
> unmask_msi_irq->
> msi_set_mask_bit->
> entry->msi_attrib.masked = 0;
>
> So entry->msi_attrib.masked in xen side always has newest value. entry->masked
> in initial domain is untouched and is 1 after msix_capability_init.
>
> Based on above, it's Xen's duty to restore entry->msi_attrib.masked to device,
> but with current code, entry->masked is used and MSI-x interrupt is masked.
>
> Before patch, restore call graph under initial domain:
> pci_reset_function->
> pci_restore_state->
> __pci_restore_msix_state->
> arch_restore_msi_irqs->
> xen_initdom_restore_msi_irqs->
> PHYSDEVOP_restore_msi hypercall (first mask restore)
> msix_mask_irq(entry, entry->masked) (second mask restore)
>
> So msix_mask_irq call in initial domain call graph needs to be removed.
>
> Based on this we can move the restore of the mask in default_restore_msi_irqs
> which would avoid restoring the invalid mask under Xen. Furthermore this
> simplifies the API by making everything related to restoring an MSI be in the
> platform specific APIs instead of just parts of it.
>
> After patch, restore call graph under initial domain:
> pci_reset_function->
> pci_restore_state->
> __pci_restore_msix_state->
> arch_restore_msi_irqs->
> xen_initdom_restore_msi_irqs->
> PHYSDEVOP_restore_msi hypercall (first mask restore)
>
> Logic for baremetal is not changed.
> Before patch, restore call graph under baremetal:
> pci_reset_function->
> pci_restore_state->
> __pci_restore_msix_state->
> arch_restore_msi_irqs->
> default_restore_msi_irqs->
> msix_mask_irq(entry, entry->masked) (first mask restore)
>
> After patch, restore call graph under baremetal:
> pci_reset_function->
> pci_restore_state->
> __pci_restore_msix_state->
> arch_restore_msi_irqs->
> default_restore_msi_irqs->
> msix_mask_irq(entry, entry->masked) (first mask restore)
>
> The process for MSI code is similiar.
>
> Tested-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

It looks good. Also, I tested this patch on Exynos5440.

Best regards,
Jingoo Han



\
 
 \ /
  Last update: 2013-10-18 07:41    [W:0.048 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site