Re: [Qemu-devel] [PATCH v5 15/18] intel_iommu: introduce IEC notifiers

[David Kiarie]

On Thu, Apr 28, 2016 at 11:49 AM, Peter Xu <address@hidden> wrote:
> On Thu, Apr 28, 2016 at 10:36:19AM +0200, Jan Kiszka wrote:
>> On 2016-04-28 10:29, Peter Xu wrote:
>> > On Thu, Apr 28, 2016 at 09:26:01AM +0200, Jan Kiszka wrote:
>> >> On 2016-04-28 09:05, Peter Xu wrote:
>> >>> This patch introduces Intel VT-d IEC (Interrupt Entry Cache)
>> >>> invalidation notifier list. When vIOMMU receives IEC invalidate request,
>> >>> all the registered units will be notified with specific invalidation
>> >>> requests.
>> >>
>> >> This should be designed to be IOMMU-agnostic, i.e. become reusable for
>> >> the AMD implementation. I suspect we will have the same need for route
>> >> invalidations there as well...
>> >
>> > Yes possibly...
>> >
>> > I feel like there are lots of things that can be shared between
>> > Intel and AMD IOMMUs. I just do not know what is the most suitable
>> > "extent" that we should abstract these shared functionalities
>> > between the two, and how.
>>
>> A rough indicator: if you add something that has "vtd" in its name to
>> non-vtd code, think twice about some reusable abstraction ;). So,
>> something like "vtd_get_iommu" could already be named and designed to
>> have two provides (e.g. allow an IOMMU to register itself as provider,
>> return that registered instance(s) when requested).
>
> Yes, thanks for the hints. :)
>
> Before that, I was considering that the AMD guy who is going to add
> its support will better consider this and finally make sure the two
> coops well (anyway, I know nothing about AMD IOMMU before reading
> recent patches, and there is still no amd_iommu.c yet for me to read
> at..). But you are right, best to start consider it from the very
> beginning.

I think AMD IOMMU could be a benefit greatly from the Intel IOMMU
cache implementation. There could be a few differences but I think
much of the code could be reused. The thing is, AMD IOMMU spec doesn't
mention anything to do with it's cache design except the available
interface (for system programmers) and I have a bit of a hard time
trying to design a cache from scratch. Trying to do this will however
require me to dig too much into Intel IOMMU and I would prefer to
reserve that for later, probably.

>
>>
>> >
>> > For example, AFAIU, a better solution for current IOMMU
>> > codes (including Intel and AMD) is to provide a common framework
>> > (like...  X86IOMMU?), abstract these shared things out into a
>> > framework, like per device name spaces, iotlb, IEC notifications,
>> > etc... However, that will need a lot of further work. Also, I still
>> > do not know whether this is a good idea even in the future.
>> >
>> > So, will this be a good point that we start to think about common
>> > code blocks for both Intel and AMD IOMMU?
>>
>> The core iommu code can still be refactored later on. I'm now more
>> concerned about the hooks you add to generic code, see above.
>
> Will try to make them look better in v6. Hopefully there will have
> no vtd_*() in common codes. ;)
>
> Thanks!
>
> -- peterx