From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH 1 of 3] IOMMU: Add command line param to disable sharing of IOMMU and hap tables Date: Thu, 22 Mar 2012 10:44:35 +0000 Message-ID: <20120322104435.GC37468@ocelot.phlegethon.org> References: <642c0e6a01c2e30e6043.1332357778@xdev.gridcentric.ca> <4F6AFC2D020000780007A1CC@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <4F6AFC2D020000780007A1CC@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: olaf@aepfle.de, keir@xen.org, andres@gridcentric.ca, xen-devel@lists.xen.org, wei.wang2@amd.com, Andres Lagar-Cavilla , adin@gridcentric.ca, hongkaixing@huawei.com List-Id: xen-devel@lists.xenproject.org At 09:17 +0000 on 22 Mar (1332407837), Jan Beulich wrote: > >>> On 21.03.12 at 20:22, Andres Lagar-Cavilla wrote: > > The default is 1, and the command line parameter sets to 1. Disabling may be > > desired if the host will contain VMs that do paging, sharing or mem access, > > and > > won't be doing passthrough. These two features are mutually exclusive for > > AMD > > processors. > > > > Signed-off-by: Andres Lagar-Cavilla > > > > diff -r d7e417afcbe4 -r 642c0e6a01c2 xen/drivers/passthrough/iommu.c > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -87,6 +87,8 @@ static void __init parse_iommu_param(cha > > iommu_dom0_strict = 1; > > else if ( !strcmp(s, "sharept") ) > > iommu_hap_pt_share = 1; > > + else if ( !strcmp(s, "no-sharept") ) > > + iommu_hap_pt_share = 0; > > Taking the description above into consideration - why not simply > replace the pointless enabling option with the disabling one? Let's just fix the whole function to handle 'no-' the way the main parser does: ----- IOMMU: clean up handling of 'foo' and 'no-foo' command-line options Signed-off-by: Tim Deegan diff -r 8180cb3895af xen/drivers/passthrough/iommu.c --- a/xen/drivers/passthrough/iommu.c Thu Mar 22 10:26:45 2012 +0000 +++ b/xen/drivers/passthrough/iommu.c Thu Mar 22 10:38:48 2012 +0000 @@ -57,36 +57,41 @@ 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'; if ( !parse_bool(s) ) - iommu_enabled = 0; + iommu_enabled = val; 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 );