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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.11.18 at 16:48, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Nov 15, 2018 at 08:40:11AM -0700, Jan Beulich wrote:
>> >>> On 14.11.18 at 12:57, <roger.pau@xxxxxxxxxx> 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.

You want to say so in the description then.

>> 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?

No, because even if they were assigned to another domain they still
should be skipped.

Jan



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