Re: [PATCH v4 5/5] amd/iommu: skip bridge devices when updating IOMMU page tables

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

On Thu, Nov 15, 2018 at 08:40:11AM -0700, Jan Beulich wrote:
> >>> On 14.11.18 at 12:57, <roger.pau@citrix.com> wrote:
> > Bridges are not behind an IOMMU, and are already special cased and
> > silently skipped in amd_iommu_add_device. Apply the same special
> > casing when updating page tables.
> 
> But bridges also don't issue I/O on their own if I'm not mistaken. So
> what I'm missing here is a word on the benefit of this change. I also
> question the "silently" in your wording, seeing the AMD_IOMMU_DEBUG()
> there.

I see, by silently I meant without throwing an error, but I think just
using 'skipped' would be clearer.

The benefit is that update_paging_mode doesn't return an error when it
finds a bridge attached to Dom0, which would cause the caller of
update_paging_mode (amd_iommu_{un}map_page) to crash the domain.

Ie: without this change a PVH Dom0 running on AMD hardware crashes
when the IOMMU page table is expanded.

> The code change itself looks fine to me, albeit personally I'd prefer
> if it fully matched the other conditional (i.e. if you flipped the
> operands of && ). Of course the special casing of the hardware
> domain here is somewhat odd anyway.

Do you mean because bridges would only be ever assigned to the
hardware domain?

Thanks, Roger.

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