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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Sep 14, 2018 at 05:13:48AM -0600, Jan Beulich wrote:
> >>> On 14.09.18 at 13:06, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, Sep 14, 2018 at 04:49:49AM -0600, Jan Beulich wrote:
> >> >>> On 14.09.18 at 11:54, <roger.pau@xxxxxxxxxx> wrote:
> >> > On Fri, Sep 14, 2018 at 03:06:03AM -0600, Jan Beulich wrote:
> >> >> >>> On 14.09.18 at 10:02, <roger.pau@xxxxxxxxxx> 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.
> 
> But prior to that series, the command line parameter was private to
> xen/drivers/passthrough/vtd/x86/vtd.c. You've insisted on making
> the option available on AMD, but if it is to be of any use, ordering
> there likely needs to match that for VT-d.

OK, I think the requirement here would be that inclusive mappings are
always setup before either the IOMMU is enabled (on the Intel case) or
devices are added (on the AMD case, since the IOMMU is enabled very
early).

So I think the best way to solve this is to move the call to
arch_iommu_hwdom_init inside of the hwdom_init handlers for each
implementation.

Roger.

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