1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] virt-host-validate: Fix warning for IOMMU detection on PPC

Nitesh Konkar posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170817134805.27720-1-niteshkonkar.libvirt@gmail.com
tools/virt-host-validate-common.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[libvirt] [PATCH v2] virt-host-validate: Fix warning for IOMMU detection on PPC
Posted by Nitesh Konkar 6 years, 8 months ago 
Fix the warning generated on PPC by virt-host-validate
for IOMMU

Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
---
 tools/virt-host-validate-common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index 6faed04..51fa8c0 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -35,6 +35,7 @@
 #include "virfile.h"
 #include "virt-host-validate-common.h"
 #include "virstring.h"
+#include "virarch.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -442,8 +443,7 @@ int virHostValidateIOMMU(const char *hvname,
     virBitmapPtr flags;
     struct stat sb;
     const char *bootarg = NULL;
-    bool isAMD = false, isIntel = false;
-
+    bool isAMD = false, isIntel = false, isPPC = false;
     flags = virHostValidateGetCPUFlags();
 
     if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
@@ -453,9 +453,10 @@ int virHostValidateIOMMU(const char *hvname,
 
     virBitmapFree(flags);
 
-    virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
+    isPPC = ARCH_IS_PPC64(virArchFromHost());
 
     if (isIntel) {
+        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
         if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
             virHostMsgPass();
             bootarg = "intel_iommu=on";
@@ -467,6 +468,7 @@ int virHostValidateIOMMU(const char *hvname,
             return -1;
         }
     } else if (isAMD) {
+        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
         if (access("/sys/firmware/acpi/tables/IVRS", F_OK) == 0) {
             virHostMsgPass();
             bootarg = "iommu=pt iommu=1";
@@ -477,6 +479,8 @@ int virHostValidateIOMMU(const char *hvname,
                            "hardware platform");
             return -1;
         }
+    } else if (isPPC) {
+        /* Empty Block */
     } else {
         virHostMsgFail(level,
                        "Unknown if this platform has IOMMU support");
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virt-host-validate: Fix warning for IOMMU detection on PPC
Posted by John Ferlan 6 years, 8 months ago 

On 08/17/2017 09:48 AM, Nitesh Konkar wrote:
> Fix the warning generated on PPC by virt-host-validate
> for IOMMU
> 
> Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
> ---
>  tools/virt-host-validate-common.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
> index 6faed04..51fa8c0 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c
> @@ -35,6 +35,7 @@
>  #include "virfile.h"
>  #include "virt-host-validate-common.h"
>  #include "virstring.h"
> +#include "virarch.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
> @@ -442,8 +443,7 @@ int virHostValidateIOMMU(const char *hvname,
>      virBitmapPtr flags;
>      struct stat sb;
>      const char *bootarg = NULL;
> -    bool isAMD = false, isIntel = false;
> -
> +    bool isAMD = false, isIntel = false, isPPC = false;
>      flags = virHostValidateGetCPUFlags();
>  
>      if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
> @@ -453,9 +453,10 @@ int virHostValidateIOMMU(const char *hvname,
>  
>      virBitmapFree(flags);
>  
> -    virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
> +    isPPC = ARCH_IS_PPC64(virArchFromHost());
>  
>      if (isIntel) {
> +        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
>          if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
>              virHostMsgPass();
>              bootarg = "intel_iommu=on";
> @@ -467,6 +468,7 @@ int virHostValidateIOMMU(const char *hvname,
>              return -1;
>          }
>      } else if (isAMD) {
> +        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
>          if (access("/sys/firmware/acpi/tables/IVRS", F_OK) == 0) {
>              virHostMsgPass();
>              bootarg = "iommu=pt iommu=1";
> @@ -477,6 +479,8 @@ int virHostValidateIOMMU(const char *hvname,
>                             "hardware platform");
>              return -1;
>          }
> +    } else if (isPPC) {
> +        /* Empty Block */

So there's nothing to check at all? Perhaps elaborate in the commit
message...


Still what happens when @bootarg isn't populated and by chance we fall
into the:

if (sb.st_nlink <= 2) {
}

condition below here?  You'll get "Add <nil>..."

Or can we not get to that code?  I'm not PPC and IOMMU aware, so I'm
asking...

Tks,

John

>      } else {
>          virHostMsgFail(level,
>                         "Unknown if this platform has IOMMU support");
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virt-host-validate: Fix warning for IOMMU detection on PPC
Posted by Nitesh Konkar 6 years, 8 months ago 
Hello John,

In case of PPC, IOMMU in the host kernel either has it or not compiled in.
The  /sys/kernel/iommu_groups check is good enough to verify if it was
compiled with the kernel or not.

If not, then we can have a ppc specific message there:


    virHostMsgCheck(hvname, "%s", _("if IOMMU is enabled by kernel"));
    if (sb.st_nlink <= 2) {
        if(!isPPC) {
            virHostMsgFail(level,
                           "IOMMU appears to be disabled in kernel. "
                           "Add %s to kernel cmdline arguments", bootarg);
        } else {
            virHostMsgFail(level,
                           "IOMMU capability not compiled into kernel. ");

        }
        return -1;
    }
    virHostMsgPass();
    return 0;



On Wed, Aug 23, 2017 at 7:06 PM, John Ferlan <jferlan@redhat.com> wrote:

>
>
> On 08/17/2017 09:48 AM, Nitesh Konkar wrote:
> > Fix the warning generated on PPC by virt-host-validate
> > for IOMMU
> >
> > Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com>
> > ---
> >  tools/virt-host-validate-common.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/virt-host-validate-common.c
> b/tools/virt-host-validate-common.c
> > index 6faed04..51fa8c0 100644
> > --- a/tools/virt-host-validate-common.c
> > +++ b/tools/virt-host-validate-common.c
> > @@ -35,6 +35,7 @@
> >  #include "virfile.h"
> >  #include "virt-host-validate-common.h"
> >  #include "virstring.h"
> > +#include "virarch.h"
> >
> >  #define VIR_FROM_THIS VIR_FROM_NONE
> >
> > @@ -442,8 +443,7 @@ int virHostValidateIOMMU(const char *hvname,
> >      virBitmapPtr flags;
> >      struct stat sb;
> >      const char *bootarg = NULL;
> > -    bool isAMD = false, isIntel = false;
> > -
> > +    bool isAMD = false, isIntel = false, isPPC = false;
> >      flags = virHostValidateGetCPUFlags();
> >
> >      if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_
> VMX))
> > @@ -453,9 +453,10 @@ int virHostValidateIOMMU(const char *hvname,
> >
> >      virBitmapFree(flags);
> >
> > -    virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
> support"));
> > +    isPPC = ARCH_IS_PPC64(virArchFromHost());
> >
> >      if (isIntel) {
> > +        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
> support"));
> >          if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
> >              virHostMsgPass();
> >              bootarg = "intel_iommu=on";
> > @@ -467,6 +468,7 @@ int virHostValidateIOMMU(const char *hvname,
> >              return -1;
> >          }
> >      } else if (isAMD) {
> > +        virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
> support"));
> >          if (access("/sys/firmware/acpi/tables/IVRS", F_OK) == 0) {
> >              virHostMsgPass();
> >              bootarg = "iommu=pt iommu=1";
> > @@ -477,6 +479,8 @@ int virHostValidateIOMMU(const char *hvname,
> >                             "hardware platform");
> >              return -1;
> >          }
> > +    } else if (isPPC) {
> > +        /* Empty Block */
>
> So there's nothing to check at all? Perhaps elaborate in the commit
> message...
>
>
> Still what happens when @bootarg isn't populated and by chance we fall
> into the:
>
> if (sb.st_nlink <= 2) {
> }
>
> condition below here?  You'll get "Add <nil>..."
>
> Or can we not get to that code?  I'm not PPC and IOMMU aware, so I'm
> asking...
>
> Tks,
>
> John
>
> >      } else {
> >          virHostMsgFail(level,
> >                         "Unknown if this platform has IOMMU support");
> >
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virt-host-validate: Fix warning for IOMMU detection on PPC
Posted by John Ferlan 6 years, 8 months ago 

On 08/24/2017 02:52 AM, Nitesh Konkar wrote:
> Hello John,
> 
> In case of PPC, IOMMU in the host kernel either has it or not compiled in.
> The  /sys/kernel/iommu_groups check is good enough to verify if it was
> compiled with the kernel or not.
> 
> If not, then we can have a ppc specific message there:   
> 
> 
>     virHostMsgCheck(hvname, "%s", _("if IOMMU is enabled by kernel"));
>     if (sb.st_nlink <= 2) {
>         if(!isPPC) {
>             virHostMsgFail(level,
>                            "IOMMU appears to be disabled in kernel. "
>                            "Add %s to kernel cmdline arguments", bootarg);
>         } else {
>             virHostMsgFail(level,
>                            "IOMMU capability not compiled into kernel. ");
> 
>         }
>         return -1;
>     }
>     virHostMsgPass();
>     return 0;
> 

Should I say don't top post please...

Anyway, I've updated the commit message, added the extra check/error
message and pushed the patch upstream

Tks,

John

> 
> 
> On Wed, Aug 23, 2017 at 7:06 PM, John Ferlan <jferlan@redhat.com
> <mailto:jferlan@redhat.com>> wrote:
> 
> 
> 
>     On 08/17/2017 09:48 AM, Nitesh Konkar wrote:
>     > Fix the warning generated on PPC by virt-host-validate
>     > for IOMMU
>     >
>     > Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com
>     <mailto:nitkon12@linux.vnet.ibm.com>>
>     > ---
>     >  tools/virt-host-validate-common.c | 10 +++++++---
>     >  1 file changed, 7 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/tools/virt-host-validate-common.c
>     b/tools/virt-host-validate-common.c
>     > index 6faed04..51fa8c0 100644
>     > --- a/tools/virt-host-validate-common.c
>     > +++ b/tools/virt-host-validate-common.c
>     > @@ -35,6 +35,7 @@
>     >  #include "virfile.h"
>     >  #include "virt-host-validate-common.h"
>     >  #include "virstring.h"
>     > +#include "virarch.h"
>     >
>     >  #define VIR_FROM_THIS VIR_FROM_NONE
>     >
>     > @@ -442,8 +443,7 @@ int virHostValidateIOMMU(const char *hvname,
>     >      virBitmapPtr flags;
>     >      struct stat sb;
>     >      const char *bootarg = NULL;
>     > -    bool isAMD = false, isIntel = false;
>     > -
>     > +    bool isAMD = false, isIntel = false, isPPC = false;
>     >      flags = virHostValidateGetCPUFlags();
>     >
>     >      if (flags && virBitmapIsBitSet(flags,
>     VIR_HOST_VALIDATE_CPU_FLAG_VMX))
>     > @@ -453,9 +453,10 @@ int virHostValidateIOMMU(const char *hvname,
>     >
>     >      virBitmapFree(flags);
>     >
>     > -    virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
>     support"));
>     > +    isPPC = ARCH_IS_PPC64(virArchFromHost());
>     >
>     >      if (isIntel) {
>     > +        virHostMsgCheck(hvname, "%s", _("for device assignment
>     IOMMU support"));
>     >          if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
>     >              virHostMsgPass();
>     >              bootarg = "intel_iommu=on";
>     > @@ -467,6 +468,7 @@ int virHostValidateIOMMU(const char *hvname,
>     >              return -1;
>     >          }
>     >      } else if (isAMD) {
>     > +        virHostMsgCheck(hvname, "%s", _("for device assignment
>     IOMMU support"));
>     >          if (access("/sys/firmware/acpi/tables/IVRS", F_OK) == 0) {
>     >              virHostMsgPass();
>     >              bootarg = "iommu=pt iommu=1";
>     > @@ -477,6 +479,8 @@ int virHostValidateIOMMU(const char *hvname,
>     >                             "hardware platform");
>     >              return -1;
>     >          }
>     > +    } else if (isPPC) {
>     > +        /* Empty Block */
> 
>     So there's nothing to check at all? Perhaps elaborate in the commit
>     message...
> 
> 
>     Still what happens when @bootarg isn't populated and by chance we fall
>     into the:
> 
>     if (sb.st_nlink <= 2) {
>     }
> 
>     condition below here?  You'll get "Add <nil>..."
> 
>     Or can we not get to that code?  I'm not PPC and IOMMU aware, so I'm
>     asking...
> 
>     Tks,
> 
>     John
> 
>     >      } else {
>     >          virHostMsgFail(level,
>     >                         "Unknown if this platform has IOMMU support");
>     >
> 
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.