Re: [dpdk-dev] [PATCH] scripts: enable extended tag of PCIe

["Lu, Patrick" <patrick.lu@intel.com>]

Hi Helin and Thomas,

Rather than an additional patch. Can we always enable extended tag in i40e driver? There is no negative impact for enabling extended tag. (Even on devices that don't support extended tag, the PCIe packet will simply ignore the bit and go on.)

Best,

Patrick

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Liu, Yong
Sent: Tuesday, March 24, 2015 8:32 PM
To: Zhang, Helin; Thomas Monjalon
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] scripts: enable extended tag of PCIe

Hi Helin,
This patch look fine for me. Just need add some descriptions about the extended tag. 
If this script work only on FVL device, maybe it should be renamed like "set_fvl_extended_tag". 

> -----Original Message-----
> From: Zhang, Helin
> Sent: Tuesday, March 24, 2015 9:08 AM
> To: Thomas Monjalon; Liu, Yong
> Cc: dev@dpdk.org; Butler, Siobhan A
> Subject: RE: [dpdk-dev] [PATCH] scripts: enable extended tag of PCIe
> 
> Hi Thomas
> 
> Zhida is our intern who has already been back to university. I think 
> Yong might have reviewed it.
> It is good supplementation for setting extended tag on Linux, though 
> not necessary. I am OK to have it merged or not. Thanks!
> 
> Marvin, could you help to ack it, as I know you have reviewed it?
> 
> Regards,
> Helin
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Monday, March 23, 2015 7:53 PM
> > To: Zang, Zhida
> > Cc: dev@dpdk.org; Butler, Siobhan A; Zhang, Helin
> > Subject: Re: [dpdk-dev] [PATCH] scripts: enable extended tag of PCIe
> >
> > Hi,
> >
> > This patch needs review and documentation.
> > It's going to be dropped if nobody cares.
> >
> > There were some previous discussions about it:
> > 	http://dpdk.org/ml/archives/dev/2015-February/012708.html
> >
> >
> > 2015-01-30 12:57, zhida zang:
> > > As 'extended tag' of PCIe needs to be enabled for i40e high 
> > > performance, Linux command of 'setpci' can be used to check and 
> > > set the corresponding bit of 'extended tag' of PCIe configuration space.
> > > The script is to check and set the right bit in PCIe configuration
> space to
> > enable 'extended tag'.
> > >
> > > Signed-off-by: Zhida Zang <zhida.zang@intel.com>
> > > ---
> > >  tools/set_pci.py | 124
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 124 insertions(+)  create mode 100755 
> > > tools/set_pci.py
> > >
> > > diff --git a/tools/set_pci.py b/tools/set_pci.py new file mode 
> > > 100755 index 0000000..e242efb
> > > --- /dev/null
> > > +++ b/tools/set_pci.py
> > > @@ -0,0 +1,124 @@
> > > +#! /usr/bin/python
> > > +import sys
> > > +import os
> > > +import subprocess
> > > +import getopt
> > > +from os.path import basename
> > > +
> > > +# The register to check if extended tag is supported or not.
> > > +PCI_DEV_CAP_REG = 0xA4
> > > +# The control register which contains the bit to enable/disable
> 'extended
> > tag'.
> > > +PCI_DEV_CTRL_REG = 0xA8
> > > +# The mask of 'extended tag' in capability register.
> > > +PCI_DEV_CAP_EXT_TAG_MASK = 0x20
> > > +# The mask of 'extended tag' in control register.
> > > +PCI_DEV_CTRL_EXT_TAG_MASK = 0x100
> > > +
> > > +dev_ids = {}
> > > +flag = "Set"
> > > +
> > > +
> > > +def usage():
> > > +    '''Print usage information for the program'''
> > > +    argv0 = basename(sys.argv[0])
> > > +    print """
> > > +Usage:
> > > +------
> > > +
> > > +    %(argv0)s [options] DEVICE1 DEVICE2 ....
> > > +
> > > +where DEVICE1, DEVICE2 etc, are specified via PCI 
> > > +"domain:bus:slot.func" syntax or "bus:slot.func" syntax. For 
> > > +devices bound to Linux kernel drivers, they may also be referred 
> > > +to by Linux
> interface
> > name e.g. eth0, eth1, em0, em1, etc.
> > > +
> > > +Options:
> > > +    --help, --usage:
> > > +        Display usage information and quit
> > > +
> > > +    -s --set:
> > > +        Set the following pci device
> > > +
> > > +    -u --Unset:
> > > +        Unset the following pci device
> > > +
> > > +Examples:
> > > +---------
> > > +To set pci 0a:00.0
> > > +        %(argv0)s -s 0a:00.0
> > > +        %(argv0)s --set 0a:00.0
> > > +
> > > +To unset 0000:01:00.0
> > > +        %(argv0)s -u 0000:01:00.0
> > > +        %(argv0)s --unset 0000:01:00.0
> > > +
> > > +To set 0000:02:00.0 and 0000:02:00.1
> > > +        %(argv0)s -s 02:00.0 02:00.1
> > > +
> > > +    """ % locals()  # replace items from local variables
> > > +
> > > +
> > > +def parse_args():
> > > +    global flag
> > > +    global dev_ids
> > > +    if len(sys.argv) <= 1:
> > > +        usage()
> > > +        sys.exit(0)
> > > +    try:
> > > +        opts, dev_ids = getopt.getopt(
> > > +            sys.argv[1:],
> > > +            "su",
> > > +            ["help", "usage", "set", "unset"]
> > > +            )
> > > +    except getopt.GetoptError, error:
> > > +        print str(error)
> > > +        print "Run '%s --usage' for further information" % sys.argv[0]
> > > +        sys.exit(1)
> > > +
> > > +    for opt, arg in opts:
> > > +        if opt == "--help" or opt == "--usage":
> > > +            usage()
> > > +            sys.exit(0)
> > > +        if opt == "-s" or opt == "--set":
> > > +            flag = "Set"
> > > +        if opt == "-u" or opt == "--unset":
> > > +            flag = "Unset"
> > > +
> > > +
> > > +def check_output(args, stderr=None):
> > > +    '''Run a command and capture its output'''
> > > +    return subprocess.Popen(
> > > +        args,
> > > +        stdout=subprocess.PIPE,
> > > +        stderr=stderr
> > > +        ).communicate()[0]
> > > +
> > > +
> > > +def set_pci():
> > > +    if len(dev_ids) == 0:
> > > +        print "Error: No devices specified."
> > > +        print "Run '%s --usage' for further information" % sys.argv[0]
> > > +        sys.exit(1)
> > > +    param_cap = "%x.W" % PCI_DEV_CAP_REG
> > > +    for k in range(len(dev_ids)):
> > > +        val = check_output(["setpci", "-s", dev_ids[k], param_cap])
> > > +        if (not (int(val, 16) & PCI_DEV_CAP_EXT_TAG_MASK)):
> > > +            print dev_ids[k], "Not supported"
> > > +            continue
> > > +        if (int(val, 16) & PCI_DEV_CTRL_EXT_TAG_MASK):
> > > +            continue
> > > +        param_ctrl = "%x.W" % PCI_DEV_CTRL_REG
> > > +        val = check_output(["setpci", "-s", dev_ids[k], param_ctrl])
> > > +        if flag == "Set":
> > > +            val = int(val, 16) | PCI_DEV_CTRL_EXT_TAG_MASK
> > > +        else:
> > > +            val = int(val, 16) & ~PCI_DEV_CTRL_EXT_TAG_MASK
> > > +        param_ctrl = "%x.W=%x" % (PCI_DEV_CTRL_REG, val)
> > > +        check_output(["setpci", "-s", dev_ids[k], param_ctrl])
> > > +
> > > +
> > > +def main():
> > > +    parse_args()
> > > +    set_pci()
> > > +
> > > +if __name__ == "__main__":
> > > +    main()
> > >
> >