Re: [PATCH] intel/iommu: setup inclusive mappings before enabling iommu

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Sep 14, 2018 at 04:49:49AM -0600, Jan Beulich wrote:
> >>> On 14.09.18 at 11:54, <roger.pau@citrix.com> wrote:
> > On Fri, Sep 14, 2018 at 03:06:03AM -0600, Jan Beulich wrote:
> >> >>> On 14.09.18 at 10:02, <roger.pau@citrix.com> wrote:
> >> > This restores the behavior before commit 66a9274cc3435 that changed
> >> > the order and enabled the iommu without having the inclusive mappings
> >> > setup.
> >> > 
> >> > Note that in order to restore previous behavior a new enable hook is
> >> > added to the iommu_ops struct that's only used by VT-d.
> >> 
> >> But your earlier series also extends inclusive mapping support to AMD -
> >> why is there no similar change needed there in case someone overrides
> >> the default of off in that case?
> > 
> > I don't see any iommu enable related code in amd_iommu_hwdom_init, but
> > maybe I'm missing something (same applies to ARM SMMU). AFAICT for AMD
> > the iommu is initialized in iommu_setup which happens before Dom0
> > creation.
> 
> But isn't the problem here that the IOMMU gets enabled too early?

For Intel yes, my earlier series changed the order for Intel and the
IOMMU got initialized before setting the inclusive mappings.

> Who or what tells us this isn't a problem on AMD as well (which then
> would be another regression of your earlier series, not the patch
> here, but would presumably also want fixing here)?

My earlier series only changed the initialization order of the Intel
IOMMU, AMD IOMMU has always been initialized earlier in iommu_setup,
and that's not changed.

> >> While looking at this I also notice that dom0_construct_pvh()'s call to
> >> iommu_hwdom_init() is unconditional, while dom0_construct_pv()'s is
> >> conditional. Is this really intentional?
> > 
> > No, I don't think so. AFAICT it should have the same check also
> > present on the PV Dom0 builder.
> > 
> > But then other logic in the PVH Dom0 builder should also be moved
> > under such check. For example a PVH Dom0 that's not the hardware
> > domain shouldn't get a vIOAPIC, access to the native ACPI tables or
> > the low 1MB and it could even have a flat physmap, as a PVH DomU would
> > get.
> 
> Can I take it that you'll be looking into this, a part of your PVH Dom0
> work?

Sure, there's some work to be done there.

Thanks, Roger.

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