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

[John Ferlan]

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 at redhat.com
> <mailto:jferlan at 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 at linux.vnet.ibm.com
>     <mailto:nitkon12 at 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");
>     >
> 
>