From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 1 of 3] IOMMU: Add command line param to disable sharing of IOMMU and hap tables Date: Thu, 29 Mar 2012 11:57:36 +0100 Message-ID: References: <20120329102721.GB72859@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120329102721.GB72859@ocelot.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan , Andres Lagar-Cavilla Cc: olaf@aepfle.de, andres@gridcentric.ca, xen-devel@lists.xen.org, wei.wang2@amd.com, hongkaixing@huawei.com, Jan Beulich , adin@gridcentric.ca List-Id: xen-devel@lists.xenproject.org On 29/03/2012 11:27, "Tim Deegan" 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 > Cheers, > > Tim. > > # HG changeset patch > # User Tim Deegan > # 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 > > 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 ); > >