Re: [PATCH v3 6/7] xen/dom0: Drop iommu_hwdom_inclusive entirely

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Juergen Gross <jgross@suse.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Xen-devel <xen-devel@lists.xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH v3 6/7] xen/dom0: Drop iommu_hwdom_inclusive entirely
Date: Wed, 16 Jan 2019 12:24:07 +0100	[thread overview]
Message-ID: <20190116112407.rehzbvwpzvmoejfg@mac> (raw)
In-Reply-To: <1547629250-1405-7-git-send-email-andrew.cooper3@citrix.com>

On Wed, Jan 16, 2019 at 09:00:49AM +0000, Andrew Cooper wrote:
> This option is unique to x86 PV dom0's, but it is not sensible to have a
> catch-all which blindly maps all non-RAM regions into the IOMMU.
> 
> The map-reserved option remains, and covers all the buggy firmware issues that
> I am aware of.  The two common cases are legacy USB keyboard emulation, and
> the BMC mailbox used by vendor firmware in NICs/HBAs to report information
> back to the iLO/iDRAC/etc for remote remote management purposes.
> 
> A specific advantage of removing this option is that x86 dom0's IOMMU setup is
> now consistent between PV and PVH.
> 
> This removal is not expected to have any impact, due to map-reserved
> remaining.  In the unlikely case that it does cause an issue, we should
> introduce other map-$SPECIFIC options rather than re-introducing this
> catch-all.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
> index e40d7a7..9fe2329 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -172,10 +172,10 @@ static bool __hwdom_init hwdom_iommu_map(const struct domain *d,
>      default:
>          if ( type & RAM_TYPE_RESERVED )
>          {
> -            if ( !iommu_hwdom_inclusive && !iommu_hwdom_reserved )
> +            if ( !iommu_hwdom_reserved )
>                  return false;
>          }
> -        else if ( is_hvm_domain(d) || !iommu_hwdom_inclusive || pfn > max_pfn )
> +        else if ( is_hvm_domain(d) || pfn > max_pfn )
>              return false;
>      }

AFAICT the logic in the switch above can be simplified if
iommu_hwdom_inclusive is dropped, I will prepare a patch do this as
soon as this is committed, since I don't want to delay this series.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel