diff mbox series

[RFC] virtio: Use DMA MAP API for devices without an IOMMU

Message ID 20180405105631.9514-1-khandual@linux.vnet.ibm.com (mailing list archive)
State RFC
Headers show
Series [RFC] virtio: Use DMA MAP API for devices without an IOMMU | expand

Commit Message

Anshuman Khandual April 5, 2018, 10:56 a.m. UTC
There are certian platforms which would like to use SWIOTLB based DMA API
for bouncing purpose without actually requiring an IOMMU back end. But the
virtio core does not allow such mechanism. Right now DMA MAP API is only
selected for devices which have an IOMMU and then the QEMU/host back end
will process all incoming SG buffer addresses as IOVA instead of simple
GPA which is the case for simple bounce buffers after being processed with
SWIOTLB API. To enable this usage, it introduces an architecture specific
function which will just make virtio core front end select DMA operations
structure.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
This RFC is just to get some feedback. Please ignore the function call
back into the architecture. It can be worked out properly later on. But
the question is can we have virtio devices in the guest which would like
to use SWIOTLB based (or any custom DMA API based) bounce buffering with
out actually being an IOMMU devices emulated by QEMU/host as been with
the current VIRTIO_F_IOMMU_PLATFORM virtio flag ?

 arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
 drivers/virtio/virtio_ring.c           | 4 ++++
 include/linux/virtio.h                 | 2 ++
 3 files changed, 12 insertions(+)

Comments

Balbir Singh April 5, 2018, 11:14 a.m. UTC | #1
On Thu, Apr 5, 2018 at 8:56 PM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
> There are certian platforms which would like to use SWIOTLB based DMA API
> for bouncing purpose without actually requiring an IOMMU back end. But the
> virtio core does not allow such mechanism. Right now DMA MAP API is only
> selected for devices which have an IOMMU and then the QEMU/host back end
> will process all incoming SG buffer addresses as IOVA instead of simple
> GPA which is the case for simple bounce buffers after being processed with
> SWIOTLB API. To enable this usage, it introduces an architecture specific
> function which will just make virtio core front end select DMA operations
> structure.
>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> This RFC is just to get some feedback. Please ignore the function call
> back into the architecture. It can be worked out properly later on. But
> the question is can we have virtio devices in the guest which would like
> to use SWIOTLB based (or any custom DMA API based) bounce buffering with
> out actually being an IOMMU devices emulated by QEMU/host as been with
> the current VIRTIO_F_IOMMU_PLATFORM virtio flag ?
>
>  arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
>  drivers/virtio/virtio_ring.c           | 4 ++++
>  include/linux/virtio.h                 | 2 ++
>  3 files changed, 12 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 06f02960b439..dd15fbddbe89 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1396,3 +1396,9 @@ static int __init disable_multitce(char *str)
>  __setup("multitce=", disable_multitce);
>
>  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> +
> +bool is_virtio_dma_platform(void)
> +{
> +       return true;
> +}
> +EXPORT_SYMBOL(is_virtio_dma_platform);
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 71458f493cf8..9f205a79d378 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -144,6 +144,10 @@ struct vring_virtqueue {
>
>  static bool vring_use_dma_api(struct virtio_device *vdev)
>  {
> +       /* Use DMA API even for virtio devices without an IOMMU */
> +       if (is_virtio_dma_platform())
> +               return true;
> +
>         if (!virtio_has_iommu_quirk(vdev))
>                 return true;
>
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 988c7355bc22..d8bb83d753ea 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -200,6 +200,8 @@ static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
>  int register_virtio_driver(struct virtio_driver *drv);
>  void unregister_virtio_driver(struct virtio_driver *drv);
>
> +extern bool is_virtio_dma_platform(void);
> +

Where is the default implementation for non-pseries platforms? Will they compile
after these changes?

Balbir
Anshuman Khandual April 5, 2018, 11:28 a.m. UTC | #2
On 04/05/2018 04:44 PM, Balbir Singh wrote:
> On Thu, Apr 5, 2018 at 8:56 PM, Anshuman Khandual
> <khandual@linux.vnet.ibm.com> wrote:
>> There are certian platforms which would like to use SWIOTLB based DMA API
>> for bouncing purpose without actually requiring an IOMMU back end. But the
>> virtio core does not allow such mechanism. Right now DMA MAP API is only
>> selected for devices which have an IOMMU and then the QEMU/host back end
>> will process all incoming SG buffer addresses as IOVA instead of simple
>> GPA which is the case for simple bounce buffers after being processed with
>> SWIOTLB API. To enable this usage, it introduces an architecture specific
>> function which will just make virtio core front end select DMA operations
>> structure.
>>
>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>> ---
>> This RFC is just to get some feedback. Please ignore the function call
>> back into the architecture. It can be worked out properly later on. But
>> the question is can we have virtio devices in the guest which would like
>> to use SWIOTLB based (or any custom DMA API based) bounce buffering with
>> out actually being an IOMMU devices emulated by QEMU/host as been with
>> the current VIRTIO_F_IOMMU_PLATFORM virtio flag ?
>>
>>  arch/powerpc/platforms/pseries/iommu.c | 6 ++++++
>>  drivers/virtio/virtio_ring.c           | 4 ++++
>>  include/linux/virtio.h                 | 2 ++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>> index 06f02960b439..dd15fbddbe89 100644
>> --- a/arch/powerpc/platforms/pseries/iommu.c
>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>> @@ -1396,3 +1396,9 @@ static int __init disable_multitce(char *str)
>>  __setup("multitce=", disable_multitce);
>>
>>  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
>> +
>> +bool is_virtio_dma_platform(void)
>> +{
>> +       return true;
>> +}
>> +EXPORT_SYMBOL(is_virtio_dma_platform);
>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> index 71458f493cf8..9f205a79d378 100644
>> --- a/drivers/virtio/virtio_ring.c
>> +++ b/drivers/virtio/virtio_ring.c
>> @@ -144,6 +144,10 @@ struct vring_virtqueue {
>>
>>  static bool vring_use_dma_api(struct virtio_device *vdev)
>>  {
>> +       /* Use DMA API even for virtio devices without an IOMMU */
>> +       if (is_virtio_dma_platform())
>> +               return true;
>> +
>>         if (!virtio_has_iommu_quirk(vdev))
>>                 return true;
>>
>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
>> index 988c7355bc22..d8bb83d753ea 100644
>> --- a/include/linux/virtio.h
>> +++ b/include/linux/virtio.h
>> @@ -200,6 +200,8 @@ static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
>>  int register_virtio_driver(struct virtio_driver *drv);
>>  void unregister_virtio_driver(struct virtio_driver *drv);
>>
>> +extern bool is_virtio_dma_platform(void);
>> +
> 
> Where is the default implementation for non-pseries platforms? Will they compile
> after these changes?

No they wont. This is just a RFC asking for suggestion/feedback on a
particular direction, will clean up the code later on once we agree
on this.
Anshuman Khandual April 5, 2018, 2:39 p.m. UTC | #3
On 04/05/2018 04:26 PM, Anshuman Khandual wrote:
> There are certian platforms which would like to use SWIOTLB based DMA API
> for bouncing purpose without actually requiring an IOMMU back end. But the
> virtio core does not allow such mechanism. Right now DMA MAP API is only
> selected for devices which have an IOMMU and then the QEMU/host back end
> will process all incoming SG buffer addresses as IOVA instead of simple
> GPA which is the case for simple bounce buffers after being processed with
> SWIOTLB API. To enable this usage, it introduces an architecture specific
> function which will just make virtio core front end select DMA operations
> structure.
> 
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>

+ "Michael S. Tsirkin" <mst@redhat.com>
Michael S. Tsirkin April 5, 2018, 2:54 p.m. UTC | #4
On Thu, Apr 05, 2018 at 08:09:30PM +0530, Anshuman Khandual wrote:
> On 04/05/2018 04:26 PM, Anshuman Khandual wrote:
> > There are certian platforms which would like to use SWIOTLB based DMA API
> > for bouncing purpose without actually requiring an IOMMU back end. But the
> > virtio core does not allow such mechanism. Right now DMA MAP API is only
> > selected for devices which have an IOMMU and then the QEMU/host back end
> > will process all incoming SG buffer addresses as IOVA instead of simple
> > GPA which is the case for simple bounce buffers after being processed with
> > SWIOTLB API. To enable this usage, it introduces an architecture specific
> > function which will just make virtio core front end select DMA operations
> > structure.
> > 
> > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> 
> + "Michael S. Tsirkin" <mst@redhat.com>

I'm confused by this.

static bool vring_use_dma_api(struct virtio_device *vdev)
{
        if (!virtio_has_iommu_quirk(vdev))
                return true;


Why doesn't setting VIRTIO_F_IOMMU_PLATFORM on the
hypervisor side sufficient?
Benjamin Herrenschmidt April 5, 2018, 3:09 p.m. UTC | #5
On Thu, 2018-04-05 at 17:54 +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 05, 2018 at 08:09:30PM +0530, Anshuman Khandual wrote:
> > On 04/05/2018 04:26 PM, Anshuman Khandual wrote:
> > > There are certian platforms which would like to use SWIOTLB based DMA API
> > > for bouncing purpose without actually requiring an IOMMU back end. But the
> > > virtio core does not allow such mechanism. Right now DMA MAP API is only
> > > selected for devices which have an IOMMU and then the QEMU/host back end
> > > will process all incoming SG buffer addresses as IOVA instead of simple
> > > GPA which is the case for simple bounce buffers after being processed with
> > > SWIOTLB API. To enable this usage, it introduces an architecture specific
> > > function which will just make virtio core front end select DMA operations
> > > structure.
> > > 
> > > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > 
> > + "Michael S. Tsirkin" <mst@redhat.com>
> 
> I'm confused by this.
> 
> static bool vring_use_dma_api(struct virtio_device *vdev)
> {
>         if (!virtio_has_iommu_quirk(vdev))
>                 return true;
> 
> 
> Why doesn't setting VIRTIO_F_IOMMU_PLATFORM on the
> hypervisor side sufficient?

In this specific case, because that would make qemu expect an iommu,
and there isn't one.

Anshuman, you need to provide more background here. I don't have time
right now it's late, but explain about the fact that this is for a
specific type of secure VM which has only a limited pool of (insecure)
memory that can be shared with qemu, so all IOs need to bounce via that
pool, which can be achieved by using swiotlb.

Note: this isn't urgent, we can discuss alternative approaches, this is
just to start the conversation.

Cheers,
Ben.
Michael S. Tsirkin April 5, 2018, 6:34 p.m. UTC | #6
On Fri, Apr 06, 2018 at 01:09:43AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2018-04-05 at 17:54 +0300, Michael S. Tsirkin wrote:
> > On Thu, Apr 05, 2018 at 08:09:30PM +0530, Anshuman Khandual wrote:
> > > On 04/05/2018 04:26 PM, Anshuman Khandual wrote:
> > > > There are certian platforms which would like to use SWIOTLB based DMA API
> > > > for bouncing purpose without actually requiring an IOMMU back end. But the
> > > > virtio core does not allow such mechanism. Right now DMA MAP API is only
> > > > selected for devices which have an IOMMU and then the QEMU/host back end
> > > > will process all incoming SG buffer addresses as IOVA instead of simple
> > > > GPA which is the case for simple bounce buffers after being processed with
> > > > SWIOTLB API. To enable this usage, it introduces an architecture specific
> > > > function which will just make virtio core front end select DMA operations
> > > > structure.
> > > > 
> > > > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> > > 
> > > + "Michael S. Tsirkin" <mst@redhat.com>
> > 
> > I'm confused by this.
> > 
> > static bool vring_use_dma_api(struct virtio_device *vdev)
> > {
> >         if (!virtio_has_iommu_quirk(vdev))
> >                 return true;
> > 
> > 
> > Why doesn't setting VIRTIO_F_IOMMU_PLATFORM on the
> > hypervisor side sufficient?
> 
> In this specific case, because that would make qemu expect an iommu,
> and there isn't one.


I think that you can set iommu_platform in qemu without an iommu.


> Anshuman, you need to provide more background here. I don't have time
> right now it's late, but explain about the fact that this is for a
> specific type of secure VM which has only a limited pool of (insecure)
> memory that can be shared with qemu, so all IOs need to bounce via that
> pool, which can be achieved by using swiotlb.
> 
> Note: this isn't urgent, we can discuss alternative approaches, this is
> just to start the conversation.
> 
> Cheers,
> Ben.
Benjamin Herrenschmidt April 5, 2018, 9:18 p.m. UTC | #7
On Thu, 2018-04-05 at 21:34 +0300, Michael S. Tsirkin wrote:
> > In this specific case, because that would make qemu expect an iommu,
> > and there isn't one.
> 
> 
> I think that you can set iommu_platform in qemu without an iommu.

No I mean the platform has one but it's not desirable for it to be used
due to the performance hit.

Cheers,
Ben.
> 
> > Anshuman, you need to provide more background here. I don't have time
> > right now it's late, but explain about the fact that this is for a
> > specific type of secure VM which has only a limited pool of (insecure)
> > memory that can be shared with qemu, so all IOs need to bounce via that
> > pool, which can be achieved by using swiotlb.
> > 
> > Note: this isn't urgent, we can discuss alternative approaches, this is
> > just to start the conversation.
> > 
> > Cheers,
> > Ben.
Anshuman Khandual April 6, 2018, 2:53 a.m. UTC | #8
On 04/06/2018 02:48 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2018-04-05 at 21:34 +0300, Michael S. Tsirkin wrote:
>>> In this specific case, because that would make qemu expect an iommu,
>>> and there isn't one.
>>
>>
>> I think that you can set iommu_platform in qemu without an iommu.
> 
> No I mean the platform has one but it's not desirable for it to be used
> due to the performance hit.

Also the only requirement is to bounce the I/O buffers through SWIOTLB
implemented as DMA API which the virtio core understands. There is no
need for an IOMMU to be involved for the device representation in this
case IMHO.
Christoph Hellwig April 6, 2018, 7:16 a.m. UTC | #9
On Fri, Apr 06, 2018 at 08:23:10AM +0530, Anshuman Khandual wrote:
> On 04/06/2018 02:48 AM, Benjamin Herrenschmidt wrote:
> > On Thu, 2018-04-05 at 21:34 +0300, Michael S. Tsirkin wrote:
> >>> In this specific case, because that would make qemu expect an iommu,
> >>> and there isn't one.
> >>
> >>
> >> I think that you can set iommu_platform in qemu without an iommu.
> > 
> > No I mean the platform has one but it's not desirable for it to be used
> > due to the performance hit.
> 
> Also the only requirement is to bounce the I/O buffers through SWIOTLB
> implemented as DMA API which the virtio core understands. There is no
> need for an IOMMU to be involved for the device representation in this
> case IMHO.

This whole virtio translation issue is a mess.  I think we need to
switch it to the dma API, and then quirk the legacy case to always
use the direct mapping inside the dma API.
Benjamin Herrenschmidt April 6, 2018, 8:37 a.m. UTC | #10
On Fri, 2018-04-06 at 00:16 -0700, Christoph Hellwig wrote:
> On Fri, Apr 06, 2018 at 08:23:10AM +0530, Anshuman Khandual wrote:
> > On 04/06/2018 02:48 AM, Benjamin Herrenschmidt wrote:
> > > On Thu, 2018-04-05 at 21:34 +0300, Michael S. Tsirkin wrote:
> > > > > In this specific case, because that would make qemu expect an iommu,
> > > > > and there isn't one.
> > > > 
> > > > 
> > > > I think that you can set iommu_platform in qemu without an iommu.
> > > 
> > > No I mean the platform has one but it's not desirable for it to be used
> > > due to the performance hit.
> > 
> > Also the only requirement is to bounce the I/O buffers through SWIOTLB
> > implemented as DMA API which the virtio core understands. There is no
> > need for an IOMMU to be involved for the device representation in this
> > case IMHO.
> 
> This whole virtio translation issue is a mess.  I think we need to
> switch it to the dma API, and then quirk the legacy case to always
> use the direct mapping inside the dma API.

Fine with using a dma API always on the Linux side, but we do want to
special case virtio still at the arch and qemu side to have a "direct
mapping" mode. Not sure how (special flags on PCI devices) to avoid
actually going through an emulated IOMMU on the qemu side, because that
slows things down, esp. with vhost.

IE, we can't I think just treat it the same as a physical device.

Cheers,
Ben.
Christoph Hellwig April 15, 2018, 12:11 p.m. UTC | #11
On Fri, Apr 06, 2018 at 06:37:18PM +1000, Benjamin Herrenschmidt wrote:
> > > implemented as DMA API which the virtio core understands. There is no
> > > need for an IOMMU to be involved for the device representation in this
> > > case IMHO.
> > 
> > This whole virtio translation issue is a mess.  I think we need to
> > switch it to the dma API, and then quirk the legacy case to always
> > use the direct mapping inside the dma API.
> 
> Fine with using a dma API always on the Linux side, but we do want to
> special case virtio still at the arch and qemu side to have a "direct
> mapping" mode. Not sure how (special flags on PCI devices) to avoid
> actually going through an emulated IOMMU on the qemu side, because that
> slows things down, esp. with vhost.
> 
> IE, we can't I think just treat it the same as a physical device.

We should have treated it like a physical device from the start, but
that device has unfortunately sailed.

But yes, we'll need a per-device quirk that says 'don't attach an
iommu'.
Anshuman Khandual April 18, 2018, 3:17 a.m. UTC | #12
On 04/15/2018 05:41 PM, Christoph Hellwig wrote:
> On Fri, Apr 06, 2018 at 06:37:18PM +1000, Benjamin Herrenschmidt wrote:
>>>> implemented as DMA API which the virtio core understands. There is no
>>>> need for an IOMMU to be involved for the device representation in this
>>>> case IMHO.
>>>
>>> This whole virtio translation issue is a mess.  I think we need to
>>> switch it to the dma API, and then quirk the legacy case to always
>>> use the direct mapping inside the dma API.
>>
>> Fine with using a dma API always on the Linux side, but we do want to
>> special case virtio still at the arch and qemu side to have a "direct
>> mapping" mode. Not sure how (special flags on PCI devices) to avoid
>> actually going through an emulated IOMMU on the qemu side, because that
>> slows things down, esp. with vhost.
>>
>> IE, we can't I think just treat it the same as a physical device.
> 
> We should have treated it like a physical device from the start, but
> that device has unfortunately sailed.
> 
> But yes, we'll need a per-device quirk that says 'don't attach an
> iommu'.

How about doing it per platform basis as suggested in this RFC through
an arch specific callback. Because all the virtio devices in the given
platform would require and exercise this option (to avail bounce buffer
mechanism for secure guests as an example). So the flag basically is a
platform specific one not a device specific one.
Michael S. Tsirkin April 18, 2018, 4:20 p.m. UTC | #13
On Wed, Apr 18, 2018 at 08:47:10AM +0530, Anshuman Khandual wrote:
> On 04/15/2018 05:41 PM, Christoph Hellwig wrote:
> > On Fri, Apr 06, 2018 at 06:37:18PM +1000, Benjamin Herrenschmidt wrote:
> >>>> implemented as DMA API which the virtio core understands. There is no
> >>>> need for an IOMMU to be involved for the device representation in this
> >>>> case IMHO.
> >>>
> >>> This whole virtio translation issue is a mess.  I think we need to
> >>> switch it to the dma API, and then quirk the legacy case to always
> >>> use the direct mapping inside the dma API.
> >>
> >> Fine with using a dma API always on the Linux side, but we do want to
> >> special case virtio still at the arch and qemu side to have a "direct
> >> mapping" mode. Not sure how (special flags on PCI devices) to avoid
> >> actually going through an emulated IOMMU on the qemu side, because that
> >> slows things down, esp. with vhost.
> >>
> >> IE, we can't I think just treat it the same as a physical device.
> > 
> > We should have treated it like a physical device from the start, but
> > that device has unfortunately sailed.
> > 
> > But yes, we'll need a per-device quirk that says 'don't attach an
> > iommu'.
> 
> How about doing it per platform basis as suggested in this RFC through
> an arch specific callback. Because all the virtio devices in the given
> platform would require and exercise this option (to avail bounce buffer
> mechanism for secure guests as an example). So the flag basically is a
> platform specific one not a device specific one.

That's not the case. A single platform can have a mix of virtio and
non-virtio devices. Same applies even within virtio, e.g. the balloon
device always bypasses an iommu.  Further, QEMU supports out of process
devices some of which might bypass the IOMMU.
Ram Pai May 1, 2018, 4:34 p.m. UTC | #14
On Wed, Apr 18, 2018 at 07:20:10PM +0300, Michael S. Tsirkin wrote:
> On Wed, Apr 18, 2018 at 08:47:10AM +0530, Anshuman Khandual wrote:
> > On 04/15/2018 05:41 PM, Christoph Hellwig wrote:
> > > On Fri, Apr 06, 2018 at 06:37:18PM +1000, Benjamin Herrenschmidt wrote:
> > >>>> implemented as DMA API which the virtio core understands. There is no
> > >>>> need for an IOMMU to be involved for the device representation in this
> > >>>> case IMHO.
> > >>>
> > >>> This whole virtio translation issue is a mess.  I think we need to
> > >>> switch it to the dma API, and then quirk the legacy case to always
> > >>> use the direct mapping inside the dma API.
> > >>
> > >> Fine with using a dma API always on the Linux side, but we do want to
> > >> special case virtio still at the arch and qemu side to have a "direct
> > >> mapping" mode. Not sure how (special flags on PCI devices) to avoid
> > >> actually going through an emulated IOMMU on the qemu side, because that
> > >> slows things down, esp. with vhost.
> > >>
> > >> IE, we can't I think just treat it the same as a physical device.
> > > 
> > > We should have treated it like a physical device from the start, but
> > > that device has unfortunately sailed.
> > > 
> > > But yes, we'll need a per-device quirk that says 'don't attach an
> > > iommu'.
> > 
> > How about doing it per platform basis as suggested in this RFC through
> > an arch specific callback. Because all the virtio devices in the given
> > platform would require and exercise this option (to avail bounce buffer
> > mechanism for secure guests as an example). So the flag basically is a
> > platform specific one not a device specific one.
> 
> That's not the case. A single platform can have a mix of virtio and
> non-virtio devices. Same applies even within virtio, e.g. the balloon
> device always bypasses an iommu.  Further, QEMU supports out of process
> devices some of which might bypass the IOMMU.

Given that each virtio device has to behave differently depending on 
(a) what it does?  (balloon, block, net etc ) 
(b) what platform it is on?  (pseries, x86, ....)
(c) what environment it is on?  (secure, insecure...)

I think, we should let the virtio device decide what it wants, instead
of forcing it to NOT use dma_ops when VIRTIO_F_IOMMU_PLATFORM is NOT
enabled.

Currently, virtio generic code, has an assumption that a device must NOT
use dma operations if the hypervisor has NOT enabled VIRTIO_F_IOMMU_PLATFORM.
This assumption is baked into vring_use_dma_api(); though there is a
special exception for xen_domain().

This assumption is restricting us from using the dma_ops abstraction for
virtio devices on secure VM. BTW: VIRTIO_F_IOMMU_PLATFORM may or may not
be set on this platform.

On our secure VM, virtio devices; by default, do not share pages with
hypervisor. In other words, hypervisor cannot access the secure VM
pages. The secure VM with the help of the hardware enables some pages
to be shared with the hypervisor.  Secure VM then uses these pages to
bounce virtio data with the hypervisor.

One elegant way to impliment this functionality is to abstract it
under our special dma_ops and wire it to the virtio devices.

However the restriction imposed by the generic virtio code, contrains us
from doing so.

If we can enrich vring_use_dma_api() to take multiple factors into
consideration and not just VIRTIO_F_IOMMU_PLATFORM; perferrably by
consulting a arch-dependent function, we could seemlessly integrate
into the existing virtio infrastructure.

RP
> 
> -- 
> MST
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 06f02960b439..dd15fbddbe89 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1396,3 +1396,9 @@  static int __init disable_multitce(char *str)
 __setup("multitce=", disable_multitce);
 
 machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
+
+bool is_virtio_dma_platform(void)
+{
+	return true;
+}
+EXPORT_SYMBOL(is_virtio_dma_platform);
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 71458f493cf8..9f205a79d378 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -144,6 +144,10 @@  struct vring_virtqueue {
 
 static bool vring_use_dma_api(struct virtio_device *vdev)
 {
+	/* Use DMA API even for virtio devices without an IOMMU */
+	if (is_virtio_dma_platform())
+		return true;
+
 	if (!virtio_has_iommu_quirk(vdev))
 		return true;
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 988c7355bc22..d8bb83d753ea 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -200,6 +200,8 @@  static inline struct virtio_driver *drv_to_virtio(struct device_driver *drv)
 int register_virtio_driver(struct virtio_driver *drv);
 void unregister_virtio_driver(struct virtio_driver *drv);
 
+extern bool is_virtio_dma_platform(void);
+
 /* module_virtio_driver() - Helper macro for drivers that don't do
  * anything special in module init/exit.  This eliminates a lot of
  * boilerplate.  Each module may only use this macro once, and