[dpdk-dev] [PATCH 1/7] [PATCH 4/8] igb: workaround errata with wthresh on 82576
On 30/05/2013 19:12, Stephen Hemminger wrote: > The 82576 has known issues which require the write threshold to be set to 1. > See: > http://download.intel.com/design/network/specupdt/82576_SPECUPDATE.pdf > > If not then single packets will hang in transmit ring until more arrive. > Simple tests like ping will fail. > > Signed-off-by: Stephen Hemminger Reviewed-by: Vincent Jardin It looks good. Thank you.
[dpdk-dev] [PATCH 2/7] rte_timer: optimize for empty case
On 30/05/2013 19:12, Stephen Hemminger wrote: > In many application there are no timers queued, and the call to > rte_timer_managecan be optimized in that case avoid reading HPET and > lock overhead. > > Signed-off-by: Stephen Hemminger Reviewed-by: Vincent Jardin It is a nice to have for performance since HPET is quite slow.
[dpdk-dev] [PATCH 3/7] optimize log/panic
On 30/05/2013 19:12, Stephen Hemminger wrote: > Both logging and calls to panic are never in the critical path. > Use the GCC attribute cold to mark these functions as cold, > which generates more optimised code. > > Signed-off-by: Stephen Hemminger Reviewed-by: Vincent Jardin It does not hurt to move both to a cold section. TODO: some other init and setup functions should be cold too.
[dpdk-dev] [PATCH 7/7] eal: add ability to override DPDK syslog parameters
On 30/05/2013 19:12, Stephen Hemminger wrote: > By default, DPDK based applications would only allow logging > to syslog as "rte", DAEMON; but for any production application more > control is desired to allow using actual application name and > overriding the facility. > > Signed-off-by: Stephen Hemminger Reviewed-by: Vincent Jardin
[dpdk-dev] [PATCH 5/7] pci: support multiple PCI regions per device
Hi Stephen, Overall this patch is very nice. My only comment on this one is why do you limit the max number of memory resources to 5 ? The PCI configuration space permits to store up to 6 base addresses. > +#define PCI_MEM_RESOURCE 5 Please, can you add a log/comment with your patch, too ? Cheers, -- Damien Millescamps
[dpdk-dev] [PATCH 5/7] pci: support multiple PCI regions per device
On Wed, 05 Jun 2013 16:50:03 +0200 Damien Millescamps wrote: > Hi Stephen, > > Overall this patch is very nice. My only comment on this one is why do > you limit the max number of memory resources to 5 ? > The PCI configuration space permits to store up to 6 base addresses. > > > +#define PCI_MEM_RESOURCE 5 > > Please, can you add a log/comment with your patch, too ? > > > Cheers, Only because I was trying to save some space, and I didn't see any hardware with that many useful regions. Also the kernel UIO driver has some control over which regions get exposed.
[dpdk-dev] [PATCH 5/7] pci: support multiple PCI regions per device
On 06/05/2013 05:49 PM, Stephen Hemminger wrote: > On Wed, 05 Jun 2013 16:50:03 +0200 > Damien Millescamps wrote: > >> Hi Stephen, >> >> Overall this patch is very nice. My only comment on this one is why do >> you limit the max number of memory resources to 5 ? >> The PCI configuration space permits to store up to 6 base addresses. >> >>> +#define PCI_MEM_RESOURCE 5 >> Please, can you add a log/comment with your patch, too ? >> >> >> Cheers, > Only because I was trying to save some space, and I didn't see any hardware > with that many useful regions. Also the kernel UIO driver has some control > over which regions get exposed. I agree that hardware generally don't use that much BAR for the PCIe. However, this is only a matter of 20 to 24 Bytes, so I don't see any reason not defining this macro as per the PCI standard value. Could you add a commit log and change that so it can be ack'd and pushed in the DPDK repository ? Thanks, -- Damien Millescamps
[dpdk-dev] [PATCH 5/7] pci: support multiple PCI regions per device
On Wed, 05 Jun 2013 20:05:15 +0200 Damien Millescamps wrote: > On 06/05/2013 05:49 PM, Stephen Hemminger wrote: > > On Wed, 05 Jun 2013 16:50:03 +0200 > > Damien Millescamps wrote: > > > >> Hi Stephen, > >> > >> Overall this patch is very nice. My only comment on this one is why do > >> you limit the max number of memory resources to 5 ? > >> The PCI configuration space permits to store up to 6 base addresses. > >> > >>> +#define PCI_MEM_RESOURCE 5 > >> Please, can you add a log/comment with your patch, too ? > >> > >> > >> Cheers, > > Only because I was trying to save some space, and I didn't see any hardware > > with that many useful regions. Also the kernel UIO driver has some control > > over which regions get exposed. > > I agree that hardware generally don't use that much BAR for the PCIe. > However, this is only a matter of 20 to 24 Bytes, so I don't see any > reason not defining this macro as per the PCI standard value. > > Could you add a commit log and change that so it can be ack'd and pushed > in the DPDK repository ? > > Thanks, Go ahead and change the 5 to a 7. No point in another resend of whole pile.
[dpdk-dev] [RFC] igb_uio: rework interrupt using pci_intx_mask
The code to do interrupts in the igb_uio is broken on later kernel versions
because pci_cfg_access_lock must not be used from interrupt context. But there
is
a much better way of doing all this now that a generic access to INTX
is available. Use pci_intx routines in later kernels, and provide backwards
compatible version for older kernels.
For enabling/disabling interrupts, better to program interrupt controller
via disable/enable irq rather than messing with PCI directly.
The driver was also broken for any mode other MSI-X. It did not do the
proper fallback to MSI then legacy mode if the platform does not support
MSI. This happens under virtualization environments where MSI is not
available. And the unwind code needed to handle the case of what ever
interrupt mode is used.
This version of the code is backported from our kernel modules, and
compile tested only. The pci_intx compatibility code is new and someone
who runs with older kernel should test it.
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c 2013-06-05 14:41:46.588241089
-0700
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c 2013-06-05 15:46:01.299710839
-0700
@@ -28,14 +28,6 @@
#include
#include
-/* Some function names changes between 3.2.0 and 3.3.0... */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0)
-#define PCI_LOCK pci_block_user_cfg_access
-#define PCI_UNLOCK pci_unblock_user_cfg_access
-#else
-#define PCI_LOCK pci_cfg_access_lock
-#define PCI_UNLOCK pci_cfg_access_unlock
-#endif
/**
* MSI-X related macros, copy from linux/pci_regs.h in kernel 2.6.39,
@@ -56,8 +48,7 @@
enum igbuio_intr_mode {
IGBUIO_LEGACY_INTR_MODE = 0,
IGBUIO_MSI_INTR_MODE,
- IGBUIO_MSIX_INTR_MODE,
- IGBUIO_INTR_MODE_MAX
+ IGBUIO_MSIX_INTR_MODE
};
/**
@@ -66,8 +57,9 @@ enum igbuio_intr_mode {
struct rte_uio_pci_dev {
struct uio_info info;
struct pci_dev *pdev;
- spinlock_t lock; /* spinlock for accessing PCI config space or msix
data in multi tasks/isr */
+ spinlock_t lock;
enum igbuio_intr_mode mode;
+ unsigned long flags;
struct msix_entry \
msix_entries[IGBUIO_NUM_MSI_VECTORS]; /* pointer to the msix
vectors to be allocated later */
};
@@ -87,70 +79,35 @@ igbuio_get_uio_pci_dev(struct uio_info *
return container_of(info, struct rte_uio_pci_dev, info);
}
-/**
- * It masks the msix on/off of generating MSI-X messages.
- */
-static int
-igbuio_msix_mask_irq(struct msi_desc *desc, int32_t state)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0)
+/* backport of intx mask support */
+static bool pci_intx_mask_supported(struct pci_dev *dev)
{
- uint32_t mask_bits = desc->masked;
- unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
- PCI_MSIX_ENTRY_VECTOR_CTRL;
-
- if (state != 0)
- mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
- else
- mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
-
- if (mask_bits != desc->masked) {
- writel(mask_bits, desc->mask_base + offset);
- readl(desc->mask_base);
- desc->masked = mask_bits;
- }
-
- return 0;
+ /* assume since this is known HW, that intx works. */
+ return true;
}
-/**
- * This function sets/clears the masks for generating LSC interrupts.
- *
- * @param info
- * The pointer to struct uio_info.
- * @param on
- * The on/off flag of masking LSC.
- * @return
- * -On success, zero value.
- * -On failure, a negative value.
- */
-static int
-igbuio_set_interrupt_mask(struct rte_uio_pci_dev *udev, int32_t state)
+static bool pci_check_and_mask_intx(struct pci_dev *pdev)
{
- struct pci_dev *pdev = udev->pdev;
-
- if (udev->mode == IGBUIO_MSIX_INTR_MODE) {
- struct msi_desc *desc;
-
- list_for_each_entry(desc, &pdev->msi_list, list) {
- igbuio_msix_mask_irq(desc, state);
- }
- }
- else if (udev->mode == IGBUIO_LEGACY_INTR_MODE) {
- uint32_t status;
- uint16_t old, new;
-
- pci_read_config_dword(pdev, PCI_COMMAND, &status);
+ u32 status;
+ u16 old, new;
+ bool ret = false;
+
+ pci_block_user_cfg_access(pdev);
+ pci_read_config_dword(pdev, PCI_COMMAND, &status);
+ if (status & PCI_STATUS_INTERRUPT) {
old = status;
- if (state != 0)
- new = old & (~PCI_COMMAND_INTX_DISABLE);
- else
- new = old | PCI_COMMAND_INTX_DISABLE;
+ new = old | PCI_COMMAND_INTX_DISABLE;
if (old != new)
pci_write_config_word(pdev, PCI_COMMAND, new);
+ ret = true;
}
+ pci_unblock_user_cfg_access(pdev);
- return 0;
+ return ret;
}
+#endif
/**
* This is the irqcontrol callback to be registered to uio_info.
@@ -1
