Re: [PATCH 1 of 3] IOMMU: Add command line param to disable sharing of IOMMU and hap tables

[Keir Fraser <keir@xen.org>]

On 29/03/2012 11:27, "Tim Deegan" <tim@xen.org> wrote:

> At 08:19 -0700 on 27 Mar (1332836366), Andres Lagar-Cavilla wrote:
>>> At 10:57 +0000 on 22 Mar (1332413830), Jan Beulich wrote:
>>>>>          if ( !parse_bool(s) )
>>>>> -            iommu_enabled = 0;
>>>>> +            iommu_enabled = val;
>>>> 
>>>> ... this one must remain unchanged, and the handling of the "no-"
>>>> prefix should come afterwards.
>>> 
>>> Meh.  I reserve the right to set 'iommu=no-yes'. :)
>>> That is to say, I don't think it makes any real difference.
>> 
>> Well, it'd be great if one version or the other goes in before 4.2 closes :)
> 
> OK, here's a version that keeps the 'iommu_enabled = 0' (so we never
> turn it _on_ in this function).  Jan, does that address your concern?
> 
> Any IOMMU maintainers have an opinion about this patch?
> 
> Keir, I think you're maintainer for this file - can I get an Ack?

Why does workaround_bios_bug not respect val like all the other options?
Apart from that:
Acked-by: Keir Fraser <keir@xen.org>

> Cheers,
> 
> Tim.
> 
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1333016609 -3600
> # Node ID 67418ef0095b93b0084c2fae5fd87bdae714a69e
> # Parent  74d2af0b56ea7e6072bdfd6e493be5f108808bb7
> IOMMU: clean up handling of 'foo' and 'no-foo' command-line options
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r 74d2af0b56ea -r 67418ef0095b xen/drivers/passthrough/iommu.c
> --- a/xen/drivers/passthrough/iommu.c Wed Mar 28 16:59:02 2012 +0200
> +++ b/xen/drivers/passthrough/iommu.c Thu Mar 29 11:23:29 2012 +0100
> @@ -57,8 +57,13 @@ DEFINE_PER_CPU(bool_t, iommu_dont_flush_
>  static void __init parse_iommu_param(char *s)
>  {
>      char *ss;
> +    int val;
>  
>      do {
> +        val = !!strncmp(s, "no-", 3);
> +        if ( !val )
> +            s += 3;
> +
>          ss = strchr(s, ',');
>          if ( ss )
>              *ss = '\0';
> @@ -66,27 +71,27 @@ static void __init parse_iommu_param(cha
>          if ( !parse_bool(s) )
>              iommu_enabled = 0;
>          else if ( !strcmp(s, "force") || !strcmp(s, "required") )
> -            force_iommu = 1;
> +            force_iommu = val;
>          else if ( !strcmp(s, "workaround_bios_bug") )
>              iommu_workaround_bios_bug = 1;
>          else if ( !strcmp(s, "verbose") )
> -            iommu_verbose = 1;
> -        else if ( !strcmp(s, "no-snoop") )
> -            iommu_snoop = 0;
> -        else if ( !strcmp(s, "no-qinval") )
> -            iommu_qinval = 0;
> -        else if ( !strcmp(s, "no-intremap") )
> -            iommu_intremap = 0;
> +            iommu_verbose = val;
> +        else if ( !strcmp(s, "snoop") )
> +            iommu_snoop = val;
> +        else if ( !strcmp(s, "qinval") )
> +            iommu_qinval = val;
> +        else if ( !strcmp(s, "intremap") )
> +            iommu_intremap = val;
>          else if ( !strcmp(s, "debug") )
> -            iommu_debug = 1;
> +            iommu_debug = val;
>          else if ( !strcmp(s, "amd-iommu-perdev-intremap") )
> -            amd_iommu_perdev_intremap = 1;
> +            amd_iommu_perdev_intremap = val;
>          else if ( !strcmp(s, "dom0-passthrough") )
> -            iommu_passthrough = 1;
> +            iommu_passthrough = val;
>          else if ( !strcmp(s, "dom0-strict") )
> -            iommu_dom0_strict = 1;
> +            iommu_dom0_strict = val;
>          else if ( !strcmp(s, "sharept") )
> -            iommu_hap_pt_share = 1;
> +            iommu_hap_pt_share = val;
>  
>          s = ss + 1;
>      } while ( ss );
> 
>