raw/ioat: extend python script functionality

  Currently the user needs to find the DSA instance number for any DSA
device they would like to configure using this script, which can be
cumbersome and error-prone since the instance numbering changes when
changing the binding of the devices between vfio-pci and idxd. In
addition to this, once the device is configured, there is no option to
reset the device using the script.

This patch contains two additions to the script:
1. Add the ability to specify the DSA device to configure using the
   device's PCI address instead of the DSA instance number. For example,
   "$dpdk_idxd_cfg.py 0" and "$dpdk_idxd_cfg.py 6a:01.0" are both valid
   references to the same device (assuming the numbering).
2. An option to reset the device via the script. For example,
   "$dpdk_idxd_cfg.py 6a:01.0 --reset"

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/raw/ioat/dpdk_idxd_cfg.py | 38 +++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
  

On Thu, May 27, 2021 at 02:26:46PM +0100, Kevin Laatz wrote:
> Currently the user needs to find the DSA instance number for any DSA
> device they would like to configure using this script, which can be
> cumbersome and error-prone since the instance numbering changes when
> changing the binding of the devices between vfio-pci and idxd. In
> addition to this, once the device is configured, there is no option to
> reset the device using the script.
> 
> This patch contains two additions to the script:
> 1. Add the ability to specify the DSA device to configure using the
>    device's PCI address instead of the DSA instance number. For example,
>    "$dpdk_idxd_cfg.py 0" and "$dpdk_idxd_cfg.py 6a:01.0" are both valid
>    references to the same device (assuming the numbering).
> 2. An option to reset the device via the script. For example,
>    "$dpdk_idxd_cfg.py 6a:01.0 --reset"
> 
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
>  drivers/raw/ioat/dpdk_idxd_cfg.py | 38 +++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 

Hi Kevin,

since you list out two changes here, it's a good indication that this might
be better as two separate patches. Can you please split it, thanks.

Couple of minor comments inline below too.

Regards,
/Bruce

> diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py b/drivers/raw/ioat/dpdk_idxd_cfg.py
> index ff06d9e240..7bc33b6ddb 100755
> --- a/drivers/raw/ioat/dpdk_idxd_cfg.py
> +++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
> @@ -29,6 +29,29 @@ def write_values(self, values):
>                  f.write(str(contents))
>  
>  
> +def reset_device(dsa_id):
> +    "Reset the DSA device and all its queues"
> +    drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
> +    drv_dir.write_values({"unbind": f"dsa{dsa_id}"})
> +
> +
> +def get_pci_dir(pci):
> +    "Search for the sysfs directory of the PCI device"
> +    full_pci = pci if pci.startswith("0000:") else f"0000:{pci}"

This will limit the script to only working on domains starting with 0000.
While I'm not aware of any specific cases where this won't work, for
generality sake, can we detect the presence/absense of the 0000: in some
other way, e.g. length, or count of ":" characters?

> +    if os.path.exists(f'/sys/bus/pci/devices/{full_pci}'):
> +        return f'/sys/bus/pci/devices/{full_pci}'
> +    return None
> +
> +
> +def get_dsa_id(pci):
> +    "Get the DSA instance ID using the PCI address of the device"
> +    pci_dir = get_pci_dir(pci)
> +    for path, dirs, files in os.walk(pci_dir):

What happens if pci_dir is None?

> +        for dir in dirs:
> +            if dir.startswith('dsa') and 'wq' not in dir:
> +                return int(dir[3:])
> +
> +
>  def configure_dsa(dsa_id, queues, prefix):
>      "Configure the DSA instance with appropriate number of queues"
>      dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
> @@ -68,14 +91,25 @@ def main(args):
>      "Main function, does arg parsing and calls config function"
>      arg_p = argparse.ArgumentParser(
>          description="Configure whole DSA device instance for DPDK use")
> -    arg_p.add_argument('dsa_id', type=int, help="DSA instance number")
> +    arg_p.add_argument('dsa_id',
> +                       help="Specify DSA instance either via DSA instance number or PCI address")
>      arg_p.add_argument('-q', metavar='queues', type=int, default=255,
>                         help="Number of queues to set up")
>      arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
>                         default="dpdk",
>                         help="Prefix for workqueue name to mark for DPDK use [default: 'dpdk']")
> +    arg_p.add_argument('--reset', action='store_true',
> +                       help="Reset DSA device and its queues")
>      parsed_args = arg_p.parse_args(args[1:])
> -    configure_dsa(parsed_args.dsa_id, parsed_args.q, parsed_args.prefix)
> +
> +    dsa_id = parsed_args.dsa_id
> +    dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
> +    if parsed_args.reset:
> +        print(f"Resetting DSA instance {dsa_id}")
> +        reset_device(dsa_id)
> +    else:
> +        print(f"Configuring DSA instance {dsa_id}")
> +        configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
>  
>  
>  if __name__ == "__main__":
> -- 
> 2.30.2
>
  

<...>

> Hi Kevin,
> 
> since you list out two changes here, it's a good indication that this might
> be better as two separate patches. Can you please split it, thanks.
> 
> Couple of minor comments inline below too.
> 
> Regards,
> /Bruce
>
Thanks for the feedback, Bruce. I'll split the changes as suggested and will
address the comments below for v2.

-Kevin
 
> > diff --git a/drivers/raw/ioat/dpdk_idxd_cfg.py
> b/drivers/raw/ioat/dpdk_idxd_cfg.py
> > index ff06d9e240..7bc33b6ddb 100755
> > --- a/drivers/raw/ioat/dpdk_idxd_cfg.py
> > +++ b/drivers/raw/ioat/dpdk_idxd_cfg.py
> > @@ -29,6 +29,29 @@ def write_values(self, values):
> >                  f.write(str(contents))
> >
> >
> > +def reset_device(dsa_id):
> > +    "Reset the DSA device and all its queues"
> > +    drv_dir = SysfsDir("/sys/bus/dsa/drivers/dsa")
> > +    drv_dir.write_values({"unbind": f"dsa{dsa_id}"})
> > +
> > +
> > +def get_pci_dir(pci):
> > +    "Search for the sysfs directory of the PCI device"
> > +    full_pci = pci if pci.startswith("0000:") else f"0000:{pci}"
> 
> This will limit the script to only working on domains starting with 0000.
> While I'm not aware of any specific cases where this won't work, for
> generality sake, can we detect the presence/absense of the 0000: in some
> other way, e.g. length, or count of ":" characters?
> 
> > +    if os.path.exists(f'/sys/bus/pci/devices/{full_pci}'):
> > +        return f'/sys/bus/pci/devices/{full_pci}'
> > +    return None
> > +
> > +
> > +def get_dsa_id(pci):
> > +    "Get the DSA instance ID using the PCI address of the device"
> > +    pci_dir = get_pci_dir(pci)
> > +    for path, dirs, files in os.walk(pci_dir):
> 
> What happens if pci_dir is None?
> 
> > +        for dir in dirs:
> > +            if dir.startswith('dsa') and 'wq' not in dir:
> > +                return int(dir[3:])
> > +
> > +
> >  def configure_dsa(dsa_id, queues, prefix):
> >      "Configure the DSA instance with appropriate number of queues"
> >      dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
> > @@ -68,14 +91,25 @@ def main(args):
> >      "Main function, does arg parsing and calls config function"
> >      arg_p = argparse.ArgumentParser(
> >          description="Configure whole DSA device instance for DPDK use")
> > -    arg_p.add_argument('dsa_id', type=int, help="DSA instance number")
> > +    arg_p.add_argument('dsa_id',
> > +                       help="Specify DSA instance either via DSA instance number or
> PCI address")
> >      arg_p.add_argument('-q', metavar='queues', type=int, default=255,
> >                         help="Number of queues to set up")
> >      arg_p.add_argument('--name-prefix', metavar='prefix', dest='prefix',
> >                         default="dpdk",
> >                         help="Prefix for workqueue name to mark for DPDK use
> [default: 'dpdk']")
> > +    arg_p.add_argument('--reset', action='store_true',
> > +                       help="Reset DSA device and its queues")
> >      parsed_args = arg_p.parse_args(args[1:])
> > -    configure_dsa(parsed_args.dsa_id, parsed_args.q, parsed_args.prefix)
> > +
> > +    dsa_id = parsed_args.dsa_id
> > +    dsa_id = get_dsa_id(dsa_id) if ':' in dsa_id else dsa_id
> > +    if parsed_args.reset:
> > +        print(f"Resetting DSA instance {dsa_id}")
> > +        reset_device(dsa_id)
> > +    else:
> > +        print(f"Configuring DSA instance {dsa_id}")
> > +        configure_dsa(dsa_id, parsed_args.q, parsed_args.prefix)
> >
> >
> >  if __name__ == "__main__":
> > --
> > 2.30.2
> >