Subject: Re: [PATCH v7 00/10] KVM PCIe/MSI passthrough on ARM/ARM64: kernel
 part 1/3: iommu changes
To: Alex Williamson <alex.williamson@redhat.com>
References: <1461084994-2355-1-git-send-email-eric.auger@linaro.org>
 <5718C501.20906@linaro.org> <20160421133222.76e45596@t450s.home>
Cc: eric.auger@st.com, robin.murphy@arm.com, will.deacon@arm.com,
        joro@8bytes.org, tglx@linutronix.de, jason@lakedaemon.net,
        marc.zyngier@arm.com, christoffer.dall@linaro.org,
        linux-arm-kernel@lists.infradead.org, patches@linaro.org,
        linux-kernel@vger.kernel.org, Bharat.Bhushan@freescale.com,
        pranav.sawargaonkar@gmail.com, p.fedin@samsung.com,
        iommu@lists.linux-foundation.org, Jean-Philippe.Brucker@arm.com,
        julien.grall@arm.com
From: Eric Auger <eric.auger@linaro.org>
Message-ID: <571A1996.5010009@linaro.org>
Date: Fri, 22 Apr 2016 14:31:18 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.6.0
MIME-Version: 1.0
In-Reply-To: <20160421133222.76e45596@t450s.home>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2933
Lines: 73

Hi Alex,
On 04/21/2016 09:32 PM, Alex Williamson wrote:
> On Thu, 21 Apr 2016 14:18:09 +0200
> Eric Auger <eric.auger@linaro.org> wrote:
> 
>> Hi Alex, Robin,
>> On 04/19/2016 06:56 PM, Eric Auger wrote:
>>> This series introduces the dma-reserved-iommu api used to:
>>>
>>> - create/destroy an iova domain dedicated to reserved iova bindings
>>> - map/unmap physical addresses onto reserved IOVAs.
>>> - search for an existing reserved iova mapping matching a PA window
>>> - determine whether an msi needs to be iommu mapped
>>> - translate an msi_msg PA address into its IOVA counterpart  
>>
>> Following Robin's review, I understand one important point we have to
>> clarify is how much this API has to be generic.
>>
>> I agree with Robin on the fact there is quite a lot of duplication
>> between this dma-reserved-iommu implementation and dma-iommu
>> implementation. Maybe we could consider an msi-mapping API
>> implementation upon dma-iommu.c. This implementation would add MSI
>> doorbell binding list management, including, ref counting and locking.
>>
>> We would need to add a map/unmap function taking an iova/pa/size as
>> parameters in current dma-iommu.c
>>
>> An important assumption is that the dma-mapping API and the msi-mapping
>> API must not be used concurrently (be would be trying to use the same
>> cookie to store a different iova_domain).
>>
>> Any thought/suggestion?
> 
> Hi Eric,
> 
> I'm not attached to a generic interface, the important part for me is
> that if we have an iommu domain with space reserved for MSI, the MSI
> setup and allocation code should handle that so we don't need to play
> the remapping tricks between vfio-pci and a vfio iommu driver that we
> saw in early drafts of this.  My first inclination is always to try to
> make a generic, re-usable interface, but I apologize if that's led us
> astray here and we really do want the more simple, MSI specific
> interface.
> 
> For the IOMMU API, rather than just a DOMAIN_ATTR_MSI_MAPPING flag,
> what about DOMAIN_ATTR_MSI_GEOMETRY with both a get and set attribute?
> Maybe something like:
> 
> struct iommu_domain_msi_geometry {
> 	dma_addr_t	aperture_start;
> 	dma_addr_t	aperture_end;
> 	bool		fixed; /* or 'programmable' depending on your polarity preference */
> };
> 
> Calling \get\ on arm would return { 0, 0, false }, indicating it's
> programmable, \set\ would allocate the iovad as specified.  That would
> make it very easy to expand the API to x86 with reporting of the fixed
> MSI range and it operates within the existing IOMMU API interfaces.
> Thanks,
Yes I would be happy to handle this x86 query requirement. I would be
more inclined to define it at "MSI mapping API" level since the IOMMU
API implementation does not handle iova allocation, as Robin argued as
the beginning. When "MSI MAPPING API" CONFIG is unset I would return
default x86 aperture.

Does it make sense?

Best Regards

Eric
> 
> Alex
>