Re: [PATCH] iommu/amd: Reserve exclusion range in iova-domain

[Gary R Hook <ghook@amd.com>]

On 3/29/19 9:51 AM, Joerg Roedel wrote:
> Hi Gary,
> 
> On Thu, Mar 28, 2019 at 02:52:19PM +0000, Gary R Hook wrote:
>> On 3/28/19 5:44 AM, Joerg Roedel wrote:
>>> +		if (entry->prot & (1 << 2))
>>
>> Could we add #define IOMMU_WRITE_EXCL (1 << 2) to amd_iommu_types.h?
> 
> Yes, I replace that magic number with a descriptive name.

Super; thanks.

>> The problem I see here is that if, for some untold reason, there is more
>> than one exclusion range emitted, where only the last one wins in the
>> hardware, we'd still end up with more than one range reserved in the
>> IOVA tree. Clearly, this is extremely unlikely, but wouldn't we want to
>> protect against that sort of misuse/mistake?
>>
>> I could be missing something.
> 
> No, you are not, this could still be a problem. Until now it isn't,
> because this week was the first time I have every seen an AMD IOMMU
> system making use of exclusion ranges, and it doesn't have this problem.
> 
> But this problem has been in the code even before this patch and needs
> to be addressed separatly. I think it is the best option to cancel IOMMU
> initialization when the IVRS table defines conflicting exclusion ranges
> for a single IOMMU instance.

Really? Interesting. May I ask who, as I've not seen it yet, either?

This change accomplishes the goal of setting exclusion + reserving the 
IOVA range, and is verified with testing? Cool. One wonders if they've 
considered a unity range?

Agree that addressing the uniqueness issue separately is fine. I'd 
probably prefer "first one wins" until IOVA tree clean-up could be 
implemented for "last one wins".... but I'm seeing that there are at 
least two spots in the code that need attention. I may go experiment 
with this.

All told, this is really about handling a corner case, as the likelihood 
of a BIOS trying to set up >1 exclusion ranges seems improbable, if not 
impossible.


grh