[Nouveau] [PATCH 1/6] platform: specify the IOMMU physical translation bit

Alexandre Courbot gnurou at gmail.com
Mon Apr 20 00:37:09 PDT 2015


On Sat, Apr 18, 2015 at 12:10 AM, Terje Bergstrom <tbergstrom at nvidia.com> wrote:
>
> On 04/16/2015 11:26 PM, Alexandre Courbot wrote:
>>
>> Instead of this function, you should probably use the data field of
>> struct of_device_id. Define a local struct called, say,
>> nouveau_platform_params containing (for now) a single iommu_addr_bit
>> field and instanciate one for each entry of nouveau_platform_match.
>> Then you can cast match->data and retrieve the field directly instead
>> of using strcmp.
>>
>> I'd say this is then simple enough to do directly in
>> nouveau_platform_probe_iommu() instead of having a function dedicated
>> to it. It is also safer because when we add a new chip, we have to
>> update nouveau_platform_match but might very well forget about your
>> function, and will end up with bit 0 being set on all our GPU
>> addresses and endless hours of fun figuring out how this happened. :)
>>
>> While I am at it: how about defining this as a u64 mask to set/unset
>> on GPU addresses instead of just a bit? This is more flexible and
>> again safer in case someone "omits" to specify the correct bit for a
>> chip.
>
> Wouldn't u64 be as dangerous? We'd be just messing up with address in
> another way.
>
> Another way of expressing this information would be defining number of
> physical(*) address bits. IOMMU bit is the bit after physical address bits.

I don't have a strong opinion on this - the advantage of a bit mask
over a bit index is that it at least gives us the option of not having
a MMU translation bit at all (mask == 0) whereas a bit index of 0 will
set/unset the first bit.

Not that we need this for now, but keeping that possibility open
doesn't sound bad for the future (nothing guarantees the GPU will
always be behind the IOMMU).


More information about the Nouveau mailing list