> -----Original Message----- > From: Roger Pau Monne > Sent: 11 June 2018 11:26 > To: Paul Durrant <[email protected]> > Cc: [email protected]; Kevin Tian <[email protected]>; > Stefano Stabellini <[email protected]>; Wei Liu <[email protected]>; > George Dunlap <[email protected]>; Andrew Cooper > <[email protected]>; Ian Jackson <[email protected]>; Tim > (Xen.org) <[email protected]>; Julien Grall <[email protected]>; Jan Beulich > <[email protected]> > Subject: Re: [Xen-devel] [PATCH 2/2] VT-d: reconcile > iommu_inclusive_mapping and iommu=dom0-strict > > On Fri, Jun 08, 2018 at 04:30:30PM +0100, Paul Durrant wrote: > > The documentation for the iommu_inclusive_mapping Xen command line > option > > states: > > > > "Use this to work around firmware issues providing incorrect RMRR > entries" > > > > Unfortunately this workaround does not function correctly if the dom0- > strict > > iommu option is also specified. > > > > The documentation goes on to say: > > > > "Rather than only mapping RAM pages for IOMMU accesses for Dom0, with > this > > option all pages up to and including 4GB, not marked as unusable in the > > E820 table, will get a mapping established." > > My version of xen-command-line.markdown says: > > "Use this to work around firmware issues providing incorrect RMRR entries. > Rather than only mapping RAM pages for IOMMU accesses for Dom0, with > this > option all pages not marked as unusable in the E820 table will get a mapping > established."
It will, without patch #1 applied. > > I think the documentation or the code needs fixing, because the > current code does indeed only map up to 4GB. > That's what patch #1 does :-) > > > > This patch modifies the VT-d hardware domain initialization code such that > > the workaround will continue to function in dom0-strict mode, by mapping > > all pages not marked as unusable *unless* they are RAM pages not > assigned > > to dom0. > > > > Signed-off-by: Paul Durrant <[email protected]> > > --- > > Cc: Andrew Cooper <[email protected]> > > Cc: George Dunlap <[email protected]> > > Cc: Ian Jackson <[email protected]> > > Cc: Jan Beulich <[email protected]> > > Cc: Julien Grall <[email protected]> > > Cc: Konrad Rzeszutek Wilk <[email protected]> > > Cc: Stefano Stabellini <[email protected]> > > Cc: Tim Deegan <[email protected]> > > Cc: Wei Liu <[email protected]> > > Cc: Kevin Tian <[email protected]> > > --- > > docs/misc/xen-command-line.markdown | 4 +++- > > xen/drivers/passthrough/iommu.c | 2 +- > > xen/drivers/passthrough/vtd/iommu.c | 2 +- > > xen/drivers/passthrough/vtd/x86/vtd.c | 8 ++++++++ > > xen/include/xen/iommu.h | 2 +- > > 5 files changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen- > command-line.markdown > > index 6beb28dada..97768f1633 100644 > > --- a/docs/misc/xen-command-line.markdown > > +++ b/docs/misc/xen-command-line.markdown > > @@ -1213,7 +1213,9 @@ wait descriptor timed out', try increasing this > value. > > Use this to work around firmware issues providing incorrect RMRR entries. > > Rather than only mapping RAM pages for IOMMU accesses for Dom0, with > this > > option all pages up to and including 4GB, not marked as unusable in the > > -E820 table, will get a mapping established. > > +E820 table, will get a mapping established. Note that if `dom0-strict` > > +mode is enabled then conventional RAM pages not assigned to dom0 will > not > > +be mapped. > > > > ### irq\_ratelimit (x86) > > > `= <integer>` > > diff --git a/xen/drivers/passthrough/iommu.c > b/xen/drivers/passthrough/iommu.c > > index 2c44fabf99..a483212356 100644 > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -52,7 +52,7 @@ custom_param("iommu", parse_iommu_param); > > bool_t __initdata iommu_enable = 1; > > bool_t __read_mostly iommu_enabled; > > bool_t __read_mostly force_iommu; > > -bool_t __hwdom_initdata iommu_dom0_strict; > > +bool_t __read_mostly iommu_dom0_strict; > > I'm not sure why you need to change the attributes of > iommu_dom0_strict, AFAICT it's still only used in hwdom_init (or init) > functions? > That's true at this point. I plan to change it as part of PV-IOMMU. TBH though I prefer the consistency but I can drop the change if there are any major objections. > > bool_t __read_mostly iommu_verbose; > > bool_t __read_mostly iommu_workaround_bios_bug; > > bool_t __read_mostly iommu_igfx = 1; > > diff --git a/xen/drivers/passthrough/vtd/iommu.c > b/xen/drivers/passthrough/vtd/iommu.c > > index 08bce92d40..e3f043288b 100644 > > --- a/xen/drivers/passthrough/vtd/iommu.c > > +++ b/xen/drivers/passthrough/vtd/iommu.c > > @@ -1304,7 +1304,7 @@ static void __hwdom_init > intel_iommu_hwdom_init(struct domain *d) > > { > > struct acpi_drhd_unit *drhd; > > > > - if ( !iommu_passthrough && !need_iommu(d) ) > > + if ( !iommu_passthrough ) > > I think you will have to add an is_pv_domain(d) check here, or else PVH > Dom0 will also get those mappings, which is wrong because in the PVH > case we don't want to identity map PFNs into guest p2m. > > PVH Dom0 wasn't calling vtd_set_hwdom_mapping because of the > !need_iommu check. Ah, ok. I'll add that. Cheers, Paul > > Thanks, Roger. _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
