usertools: add check for IOMMU support in dpdk-devbind

Message ID 20220308124901.9838-1-fidaullah.noonari@gmai.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series usertools: add check for IOMMU support in dpdk-devbind |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Fidaullah Noonari March 8, 2022, 12:49 p.m. UTC
binding with vfio driver, when IOMMU is disabled, causes program to crash.
this patch checks for IOMMU support, if it is disabled, changes vfio
into unsafe noiommu mode and prints a warning message.

Signed-off-by: Fidaullah Noonari <fidaullah.noonari@gmai.com>
---
 usertools/dpdk-devbind.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
  

Comments

Bruce Richardson March 8, 2022, 12:56 p.m. UTC | #1
On Tue, Mar 08, 2022 at 05:49:01PM +0500, Fidaullah Noonari wrote:
> binding with vfio driver, when IOMMU is disabled, causes program to crash.
> this patch checks for IOMMU support, if it is disabled, changes vfio
> into unsafe noiommu mode and prints a warning message.
> 
> Signed-off-by: Fidaullah Noonari <fidaullah.noonari@gmai.com>
> ---
>  usertools/dpdk-devbind.py | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> index ace4627218..f42eead0c4 100755
> --- a/usertools/dpdk-devbind.py
> +++ b/usertools/dpdk-devbind.py
> @@ -466,6 +466,26 @@ def unbind_all(dev_list, force=False):
>          unbind_one(d, force)
>  
>  
> +def check_iommu():
> +    """Check if IOMMU is enabled on system"""
> +    if len(os.listdir("/sys/class/iommu")) != 0:
> +        return True
> +    print("Warning: IOMMU support disabled")
> +    return False
> +
> +
> +def enable_noiommu_mode():
> +    """Enables the noiommu mode for vfio drivers"""
> +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> +    try:
> +        f = open(filename, "w")
> +    except:
> +        sys.exit("Error: unable to enable noiommu mode, could not open '%s'" % filename)
> +    f.write("1")
> +    f.close()
> +    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
> +
> +
>  def bind_all(dev_list, driver, force=False):
>      """Bind method, takes a list of device locations"""
>      global devices
> @@ -492,6 +512,11 @@ def bind_all(dev_list, driver, force=False):
>      except ValueError as ex:
>          sys.exit(ex)
>  
> +    # check for IOMMU support
> +    if not check_iommu():
> +        if driver == "vfio-pci":
> +            enable_noiommu_mode()
> +
>      for d in dev_list:
>          bind_one(d, driver, force)
>  
I like the idea of doing this from the script, but I would rather it not
happen by default, since no-iommu reduces the security of the system. In
the normal case, I think it would be best if the script printed an error
message asking the user to enable iommu. We can also then add a flag to
allow the user to explicitly ask for no-iommu mode and take the risk.

/Bruce
  
Fidaullah Noonari March 10, 2022, 4:08 a.m. UTC | #2
I agree with your suggestion. I would add new flag for noiommu and
submit v2 for the patch.


On Tue, Mar 8, 2022 at 5:57 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Mar 08, 2022 at 05:49:01PM +0500, Fidaullah Noonari wrote:
> > binding with vfio driver, when IOMMU is disabled, causes program to crash.
> > this patch checks for IOMMU support, if it is disabled, changes vfio
> > into unsafe noiommu mode and prints a warning message.
> >
> > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@gmai.com>
> > ---
> >  usertools/dpdk-devbind.py | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
> > index ace4627218..f42eead0c4 100755
> > --- a/usertools/dpdk-devbind.py
> > +++ b/usertools/dpdk-devbind.py
> > @@ -466,6 +466,26 @@ def unbind_all(dev_list, force=False):
> >          unbind_one(d, force)
> >
> >
> > +def check_iommu():
> > +    """Check if IOMMU is enabled on system"""
> > +    if len(os.listdir("/sys/class/iommu")) != 0:
> > +        return True
> > +    print("Warning: IOMMU support disabled")
> > +    return False
> > +
> > +
> > +def enable_noiommu_mode():
> > +    """Enables the noiommu mode for vfio drivers"""
> > +    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
> > +    try:
> > +        f = open(filename, "w")
> > +    except:
> > +        sys.exit("Error: unable to enable noiommu mode, could not open '%s'" % filename)
> > +    f.write("1")
> > +    f.close()
> > +    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
> > +
> > +
> >  def bind_all(dev_list, driver, force=False):
> >      """Bind method, takes a list of device locations"""
> >      global devices
> > @@ -492,6 +512,11 @@ def bind_all(dev_list, driver, force=False):
> >      except ValueError as ex:
> >          sys.exit(ex)
> >
> > +    # check for IOMMU support
> > +    if not check_iommu():
> > +        if driver == "vfio-pci":
> > +            enable_noiommu_mode()
> > +
> >      for d in dev_list:
> >          bind_one(d, driver, force)
> >
> I like the idea of doing this from the script, but I would rather it not
> happen by default, since no-iommu reduces the security of the system. In
> the normal case, I think it would be best if the script printed an error
> message asking the user to enable iommu. We can also then add a flag to
> allow the user to explicitly ask for no-iommu mode and take the risk.
>
> /Bruce
  

Patch

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index ace4627218..f42eead0c4 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -466,6 +466,26 @@  def unbind_all(dev_list, force=False):
         unbind_one(d, force)
 
 
+def check_iommu():
+    """Check if IOMMU is enabled on system"""
+    if len(os.listdir("/sys/class/iommu")) != 0:
+        return True
+    print("Warning: IOMMU support disabled")
+    return False
+
+
+def enable_noiommu_mode():
+    """Enables the noiommu mode for vfio drivers"""
+    filename = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode"
+    try:
+        f = open(filename, "w")
+    except:
+        sys.exit("Error: unable to enable noiommu mode, could not open '%s'" % filename)
+    f.write("1")
+    f.close()
+    print("Warning: enabling unsafe no IOMMU mode for vfio drivers")
+
+
 def bind_all(dev_list, driver, force=False):
     """Bind method, takes a list of device locations"""
     global devices
@@ -492,6 +512,11 @@  def bind_all(dev_list, driver, force=False):
     except ValueError as ex:
         sys.exit(ex)
 
+    # check for IOMMU support
+    if not check_iommu():
+        if driver == "vfio-pci":
+            enable_noiommu_mode()
+
     for d in dev_list:
         bind_one(d, driver, force)