Re: [dpdk-dev] Q on Support for I217 and I218 Intel chipsets.

["Zhang, Helin" <helin.zhang@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ravi Kerur
> Sent: Monday, January 5, 2015 7:28 AM
> To: dev@dpdk.org; Neil Horman; Thomas Monjalon
> Subject: [dpdk-dev] Q on Support for I217 and I218 Intel chipsets.
> 
> Hi,
> 
> We have a Gigabyte H97N motherboard which has I217 Intel chipset which
> uses e100e drivers. I looked into lib/librte_pmd_e1000 directory and I do see
> that e1000e code is integrated but missing some support for read/write from
> flash_address and other minor things. I have made changes shown below and
> have done some testing with testpmd utility and now have following questions
> 
> 1. What amount of testing is required to qualify patch as successfully tested on
> new chipsets
> 
> 2. FreeBSD testing, currently we have Ubuntu 14.04 installed on existing H97N
> motherboard and testing is done solely on Linux. We plan to get another
> motherboard which will have I218 chipset and still deciding whether to go with
> FreeBSD or Ubuntu. So the question I have is what amount of testing should be
> done on FreeBSD? I don't think setup.sh/dpdk_nic_bind.py works on FreeBSD
> yet hence the question on testing.
> 
> Thanks,
> Ravi
> 
> 
> 
> On Sun, Jan 4, 2015 at 3:15 PM, Ravi Kerur <rkerur@gmail.com> wrote:
> 
> > This patch adds support for I217 and I218 Intel chipsets.
> > Gigabyte H97N motherboard has I217 Intel chipsets and changes include
> >
> > 1. Add relevant device-ids via RTE_PCI_DEV_ID_DECL_EM 2. Add support
> > for memory mapped flash address read/write
> >
> > Basic testing on Ubuntu with testpmd utility.
> >
> > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> > ---
> >  lib/librte_eal/common/include/rte_pci_dev_ids.h |  4 ++++
> >  lib/librte_pmd_e1000/e1000/e1000_api.c          | 21
> +++++++++++++++++++++
> >  lib/librte_pmd_e1000/e1000/e1000_api.h          |  1 +
> >  lib/librte_pmd_e1000/e1000/e1000_osdep.h        | 24
> > +++++++++++++++++++-----
> >  lib/librte_pmd_e1000/em_ethdev.c                |  7 +++++++
> >  5 files changed, 52 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > b/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > index c922de9..712793a 100644
> > --- a/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > +++ b/lib/librte_eal/common/include/rte_pci_dev_ids.h
> > @@ -274,6 +274,10 @@
> RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > E1000_DEV_ID_82572EI)
> >  RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> E1000_DEV_ID_82573L)
> > RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> E1000_DEV_ID_82574L)
> > RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> E1000_DEV_ID_82574LA)
> > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > +E1000_DEV_ID_PCH_LPT_I217_LM)
> > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > +E1000_DEV_ID_PCH_LPT_I217_V)
> > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > E1000_DEV_ID_PCH_LPTLP_I218_LM)
> > +RTE_PCI_DEV_ID_DECL_EM(PCI_VENDOR_ID_INTEL,
> > +E1000_DEV_ID_PCH_LPTLP_I218_V)
So, you are enabling new Intel(r) NICs. That's great, and thanks!

> >
> >  /******************** Physical IGB devices from e1000_hw.h
> > ********************/
> >
> > diff --git a/lib/librte_pmd_e1000/e1000/e1000_api.c
> > b/lib/librte_pmd_e1000/e1000/e1000_api.c
> > index a064565..30ed1f3 100644
> > --- a/lib/librte_pmd_e1000/e1000/e1000_api.c
> > +++ b/lib/librte_pmd_e1000/e1000/e1000_api.c
> > @@ -1355,3 +1355,24 @@ void e1000_shutdown_fiber_serdes_link(struct
> > e1000_hw *hw)
> >                 hw->mac.ops.shutdown_serdes(hw);  }
> >
> > +/**
> > + *  e1000_device_is_ich8 - Check for ICH8 device
> > + *  @hw: pointer to the HW structure
> > + *
> > + *  return TRUE for ICH8, otherwise FALSE  **/ bool
> > +e1000_device_is_ich8(struct e1000_hw *hw) {
> > +       DEBUGFUNC("e1000_device_is_ich8");
> > +
> > +       switch (hw->device_id) {
> > +       case E1000_DEV_ID_PCH_LPT_I217_LM:
> > +       case E1000_DEV_ID_PCH_LPT_I217_V:
> > +       case E1000_DEV_ID_PCH_LPTLP_I218_LM:
> > +       case E1000_DEV_ID_PCH_LPTLP_I218_V:
> > +               return 1;
> > +
> > +       default:
> > +               return 0;
> > +       }
> > +}
As Konstantin indicated, please do not try to modify any code in any source files in e1000
sub-folder, except e1000_osdep.c and e1000_osdep.h. If this piece of code is needed, please
try to move it to em_ethdev.c or e1000_osdep.c files!

> > diff --git a/lib/librte_pmd_e1000/e1000/e1000_api.h
> > b/lib/librte_pmd_e1000/e1000/e1000_api.h
> > index 02b16da..f96a674 100644
> > --- a/lib/librte_pmd_e1000/e1000/e1000_api.h
> > +++ b/lib/librte_pmd_e1000/e1000/e1000_api.h
> > @@ -49,6 +49,7 @@ extern void e1000_init_function_pointers_vf(struct
> > e1000_hw *hw);
> >  extern void e1000_power_up_fiber_serdes_link(struct e1000_hw *hw);
> > extern void e1000_shutdown_fiber_serdes_link(struct e1000_hw *hw);
> > extern void e1000_init_function_pointers_i210(struct e1000_hw *hw);
> > +extern bool e1000_device_is_ich8(struct e1000_hw *hw);
> >
> >  s32 e1000_set_obff_timer(struct e1000_hw *hw, u32 itr);
> >  s32 e1000_set_mac_type(struct e1000_hw *hw); diff --git
> > a/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > b/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > index 438641e..19146a3 100644
> > --- a/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > +++ b/lib/librte_pmd_e1000/e1000/e1000_osdep.h
> > @@ -95,13 +95,22 @@ typedef int         bool;
> >
> >  #define E1000_PCI_REG(reg) (*((volatile uint32_t *)(reg)))
> >
> > +#define E1000_PCI_REG16(reg) (*((volatile uint16_t *)(reg)))
> > +
> >  #define E1000_PCI_REG_WRITE(reg, value) do { \
> >         E1000_PCI_REG((reg)) = (value); \  } while (0)
> >
> > +#define E1000_PCI_REG_WRITE16(reg, value) do { \
> > +       E1000_PCI_REG16((reg)) = (value); \ } while (0)
> > +
> >  #define E1000_PCI_REG_ADDR(hw, reg) \
> >         ((volatile uint32_t *)((char *)(hw)->hw_addr + (reg)))
> >
> > +#define E1000_PCI_REG_FLASH_ADDR(hw, reg) \
> > +       ((volatile uint32_t *)((char *)(hw)->flash_address + (reg)))
> > +

> >  #define E1000_PCI_REG_ARRAY_ADDR(hw, reg, index) \
> >         E1000_PCI_REG_ADDR((hw), (reg) + ((index) << 2))
> >
> > @@ -110,6 +119,11 @@ static inline uint32_t e1000_read_addr(volatile
> > void*
> > addr)
> >         return E1000_PCI_REG(addr);
> >  }
> >
> > +static inline uint32_t e1000_read_addr16(volatile void* addr) {
> > +       return E1000_PCI_REG16(addr);
> > +}
> > +
> >  /* Necessary defines */
> >  #define E1000_MRQC_ENABLE_MASK                  0x00000007
> >  #define E1000_MRQC_RSS_FIELD_IPV6_EX           0x00080000
> > @@ -155,20 +169,20 @@ static inline uint32_t e1000_read_addr(volatile
> > void* addr)
> >         E1000_WRITE_REG(hw, reg, value)
> >
> >  /*
> > - * Not implemented.
> > + * Tested on I217 chipset.
> >   */
> >
> >  #define E1000_READ_FLASH_REG(hw, reg) \
> > -       (E1000_ACCESS_PANIC(E1000_READ_FLASH_REG, hw, reg, 0), 0)
> > +       e1000_read_addr(E1000_PCI_REG_FLASH_ADDR((hw), (reg)))
> >
> >  #define E1000_READ_FLASH_REG16(hw, reg)  \
> > -       (E1000_ACCESS_PANIC(E1000_READ_FLASH_REG16, hw, reg, 0), 0)
> > +       e1000_read_addr16(E1000_PCI_REG_FLASH_ADDR((hw), (reg)))
> >
> >  #define E1000_WRITE_FLASH_REG(hw, reg, value)  \
> > -       E1000_ACCESS_PANIC(E1000_WRITE_FLASH_REG, hw, reg, value)
> > +       E1000_PCI_REG_WRITE(E1000_PCI_REG_FLASH_ADDR((hw),
> (reg)),
> > + (value))
> >
> >  #define E1000_WRITE_FLASH_REG16(hw, reg, value) \
> > -       E1000_ACCESS_PANIC(E1000_WRITE_FLASH_REG16, hw, reg,
> value)
> > +       E1000_PCI_REG_WRITE16(E1000_PCI_REG_FLASH_ADDR((hw),
> (reg)),
> > (value))
> >
> >  #define STATIC static
> >
> > diff --git a/lib/librte_pmd_e1000/em_ethdev.c
> > b/lib/librte_pmd_e1000/em_ethdev.c
> > index 3f2897e..643f5cd 100644
> > --- a/lib/librte_pmd_e1000/em_ethdev.c
> > +++ b/lib/librte_pmd_e1000/em_ethdev.c
> > @@ -245,6 +245,9 @@ eth_em_dev_init(__attribute__((unused)) struct
> > eth_driver *eth_drv,
> >         hw->device_id = pci_dev->id.device_id;
> >
> >         /* For ICH8 support we'll need to map the flash memory BAR */
> > +       if (e1000_device_is_ich8(hw))
> > +               hw->flash_address = (void
> > + *)pci_dev->mem_resource[1].addr;
> > +
> >
> >         if (e1000_setup_init_funcs(hw, TRUE) != E1000_SUCCESS ||
> >                         em_hw_init(hw) != 0) { @@ -436,6 +439,7 @@
> > em_set_pba(struct e1000_hw *hw)
> >                         break;
> >                 case e1000_pchlan:
> >                 case e1000_pch2lan:
> > +               case e1000_pch_lpt:
> >                         pba = E1000_PBA_26K;
> >                         break;
> >                 default:
> > @@ -678,6 +682,8 @@ em_hardware_init(struct e1000_hw *hw)
> >         /* Workaround: no TX flow ctrl for PCH */
> >         if (hw->mac.type == e1000_pchlan)
> >                 hw->fc.requested_mode = e1000_fc_rx_pause;
> > +       else if (hw->mac.type == e1000_pch_lpt)
> > +               hw->fc.requested_mode = e1000_fc_full;
> >
> >         /* Override - settings for PCH2LAN, ya its magic :) */
> >         if (hw->mac.type == e1000_pch2lan) { @@ -845,6 +851,7 @@
> > em_get_max_pktlen(const struct e1000_hw *hw)
> >         case e1000_pch2lan:
> >         case e1000_82574:
> >         case e1000_80003es2lan: /* 9K Jumbo Frame size */
> > +       case e1000_pch_lpt:
> >                 return (0x2412);
> >         case e1000_pchlan:
> >                 return (0x1000);
> > --
> > 1.9.1
> >
> >