> -----Original Message----- > From: Jerin Jacob Kollanukkaran <jer...@marvell.com> > Sent: Wednesday, July 17, 2019 7:44 PM > To: Hyong Youb Kim (hyonkim) <hyon...@cisco.com>; Nithin Kumar > Dabilpuram <ndabilpu...@marvell.com>; David Marchand > <david.march...@redhat.com>; Thomas Monjalon > <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>; Bruce > Richardson <bruce.richard...@intel.com> > Cc: John Daley (johndale) <johnd...@cisco.com>; Shahed Shaikh > <shsha...@marvell.com>; dev@dpdk.org > Subject: RE: [RFC PATCH v3 2/3] eal: add mask and unmask interrupt APIs > > > > I think, it vary from the perspective of IRQ Chip(or controller) vs > > > NIC > > > register(Source) PoV. > > > Since the API starts from rte_intr_* it is more of controller so _ack_ > > > make sense Other reason for ack: > > > 1) It will enforce that it needs to be called form ISR > > > 2) It would be have been really correct to unmask if VFIO+MSIx+Linux > > > supports it > > > 3) if it is ack, no need to add unmask counterpart, the _mask_ API > > > > > > > Just curious, what you mean by irq controller? Ack/mask/unmask PIOs all > go > > Programmable Interrupt Controller. Like Intel 8259A, GIC from ARM etc > The drivers in linux/drivers/irqchip/ > > > to the NIC. It is the NIC that asserts/de-asserts irq.. > > > > > > > > > > Besides the name, are we agreeing that we want these? > > > > - Unmask if INTx > > > > > > Yes > > > > > > > - Nothing if MSI/MSI-X > > > Yes for MSI over VFIO > > > No for MSI over UIO/igb_uio > > > > > > > I guess I was not clear. For MSI/MSI-X, we do not want to do mask/unmask > > regardless of vfio-pci/igb_uio. Below is my comment about > > linux/windows/freebsd from an earlier email. Do you disagree? I am sure > > there are plenty of kernel NIC driver guys here. Please correct me if I am > > mistaken... > > > For some reason, igb_uio kernel driver mask the interrupt for MSIx. > We need to ack or unmask if needs to work with MSIX + IGB_UIO. > > See > pci_uio_alloc_resource() > if (dev->kdrv == RTE_KDRV_IGB_UIO) > dev->intr_handle.type = RTE_INTR_HANDLE_UIO; > else { > dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX; > > igbuio_pci_irqcontrol() for masking in kernel. >
igb_uio does not auto-mask MSI/MSI-X. static irqreturn_t igbuio_pci_irqhandler(int irq, void *dev_id) { struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id; struct uio_info *info = &udev->info; /* Legacy mode need to mask in hardware */ if (udev->mode == RTE_INTR_MODE_LEGACY && !pci_check_and_mask_intx(udev->pdev)) return IRQ_NONE; uio_event_notify(info); /* Message signal mode, no share IRQ and automasked */ return IRQ_HANDLED; } Also tested just now with igb_uio. The driver does not need to call rte_intr_enable(), and it keeps getting interrupts without any issues. Am I missing something? -Hyong > So it is more of making inline with igb_uio kernel driver AND not break > The existing drivers which was using rte_intr_enable in ISR with > MSIX+IGB_UIO > > I do agree with that for edge trigged interrupt mask may not require from > kernel. > But I am not sure why it is added in igb_uio kernel driver. May be it is just > legacy. > Anyway this wont change schematics, when igb_uio kenrel fixed then the > counter > Part can be changed in rte_intr_ack(). Ie. it is transparent to drivers. > > > > > > I don't have very strong opinion unmask vs ack. I prefer to have ack > > > due the reasons stated above. > > > If you really have strong opinion on using unmask, we will stick with > > > that to make forward progress. > > > Let us know. > > > > > > > I have no strong opinion either. > > OK. Lets stick with rte_intr_ack(). > > > > > Thanks.. > > -Hyong