[PATCH qemu-kvm] device assignment: default requires IOMMU

[PATCH qemu-kvm] device assignment: default requires IOMMU

[Chris Wright]

[ resend, fixing email header, sorry for duplicate ]

The default mode for device assignment is to rely on an IOMMU for
proper translations and a functioning device in the guest.  The current
logic makes this requirement advisory, and simply disables the request
for IOMMU if one is not found on the host.  This makes for a confused
user when the device assignment appears to work, but the device in the
guest is not functioning  (I've seen about a half-dozen reports with
this failure mode).

Change the logic such that the default requires the IOMMU.  Period.
If the host does not have an IOMMU, device assignment will fail.

This is a user visible change, however I think the current situation is
simply broken.

And, of course, disabling the IOMMU requirement using the old:

   -pcidevice host=[addr],dma=none

or the newer:

   -device pci-assign,host=[addr],iommu=0

will do what it always did (not require an IOMMU, and fail to work
properly).

Cc: Alexander Graf <agraf@suse.de>
Cc: Dmitri Seletski <drjoms@gmail.com>
Cc: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Chris Wright <chrisw@redhat.com>
---
 hw/device-assignment.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 02d23d8..a314360 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -816,14 +816,15 @@ static int assign_device(AssignedDevice *dev)
     assigned_dev_data.devfn = dev->h_devfn;
 
 #ifdef KVM_CAP_IOMMU
-    /* We always enable the IOMMU if present
-     * (or when not disabled on the command line)
-     */
-    r = kvm_check_extension(kvm_state, KVM_CAP_IOMMU);
-    if (!r)
-        dev->use_iommu = 0;
-    if (dev->use_iommu)
-	assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
+    /* We always enable the IOMMU unless disabled on the command line */
+    if (dev->use_iommu) {
+        if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
+            fprintf(stderr, "No IOMMU found.  Unable to assign device \"%s\"\n",
+                    dev->dev.qdev.id);
+             return -ENODEV;
+        }
+        assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
+    }
 #else
     dev->use_iommu = 0;
 #endif

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Alexander Graf]

Am 23.12.2009 um 23:40 schrieb Chris Wright <chrisw@sous-sol.org>:

> [ resend, fixing email header, sorry for duplicate ]
>
> The default mode for device assignment is to rely on an IOMMU for
> proper translations and a functioning device in the guest.  The  
> current
> logic makes this requirement advisory, and simply disables the request
> for IOMMU if one is not found on the host.  This makes for a confused
> user when the device assignment appears to work, but the device in the
> guest is not functioning  (I've seen about a half-dozen reports with
> this failure mode).
>
> Change the logic such that the default requires the IOMMU.  Period.
> If the host does not have an IOMMU, device assignment will fail.
>
> This is a user visible change, however I think the current situation  
> is
> simply broken.
>
> And, of course, disabling the IOMMU requirement using the old:
>
>   -pcidevice host=[addr],dma=none
>
> or the newer:
>
>   -device pci-assign,host=[addr],iommu=0
>
> will do what it always did (not require an IOMMU, and fail to work
> properly).

Yay!

Alex

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Simon Horman]

On Thu, Dec 24, 2009 at 01:45:34AM +0100, Alexander Graf wrote:
> 
> Am 23.12.2009 um 23:40 schrieb Chris Wright <chrisw@sous-sol.org>:
> 
> >[ resend, fixing email header, sorry for duplicate ]
> >
> >The default mode for device assignment is to rely on an IOMMU for
> >proper translations and a functioning device in the guest.  The
> >current
> >logic makes this requirement advisory, and simply disables the request
> >for IOMMU if one is not found on the host.  This makes for a confused
> >user when the device assignment appears to work, but the device in the
> >guest is not functioning  (I've seen about a half-dozen reports with
> >this failure mode).
> >
> >Change the logic such that the default requires the IOMMU.  Period.
> >If the host does not have an IOMMU, device assignment will fail.
> >
> >This is a user visible change, however I think the current
> >situation is
> >simply broken.
> >
> >And, of course, disabling the IOMMU requirement using the old:
> >
> >  -pcidevice host=[addr],dma=none
> >
> >or the newer:
> >
> >  -device pci-assign,host=[addr],iommu=0
> >
> >will do what it always did (not require an IOMMU, and fail to work
> >properly).
> 
> Yay!

Sounds good to me. Though I am curious to know the reasoning
behind the current logic.

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Sheng Yang]

On Thursday 24 December 2009 14:51:23 Simon Horman wrote:
> On Thu, Dec 24, 2009 at 01:45:34AM +0100, Alexander Graf wrote:
> > Am 23.12.2009 um 23:40 schrieb Chris Wright <chrisw@sous-sol.org>:
> > >[ resend, fixing email header, sorry for duplicate ]
> > >
> > >The default mode for device assignment is to rely on an IOMMU for
> > >proper translations and a functioning device in the guest.  The
> > >current
> > >logic makes this requirement advisory, and simply disables the request
> > >for IOMMU if one is not found on the host.  This makes for a confused
> > >user when the device assignment appears to work, but the device in the
> > >guest is not functioning  (I've seen about a half-dozen reports with
> > >this failure mode).
> > >
> > >Change the logic such that the default requires the IOMMU.  Period.
> > >If the host does not have an IOMMU, device assignment will fail.
> > >
> > >This is a user visible change, however I think the current
> > >situation is
> > >simply broken.
> > >
> > >And, of course, disabling the IOMMU requirement using the old:
> > >
> > >  -pcidevice host=[addr],dma=none
> > >
> > >or the newer:
> > >
> > >  -device pci-assign,host=[addr],iommu=0
> > >
> > >will do what it always did (not require an IOMMU, and fail to work
> > >properly).
> >
> > Yay!
> 
> Sounds good to me. Though I am curious to know the reasoning
> behind the current logic.
> 
Sounds pretty good. :)

I think maybe it due to we are interested in implementing PV DMA?

-- 
regards
Yang, Sheng

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Muli Ben-Yehuda]

On Thu, Dec 24, 2009 at 02:56:00PM +0800, Sheng Yang wrote:

> > > >And, of course, disabling the IOMMU requirement using the old:
> > > >
> > > >  -pcidevice host=[addr],dma=none
> > > >
> > > >or the newer:
> > > >
> > > >  -device pci-assign,host=[addr],iommu=0
> > > >
> > > >will do what it always did (not require an IOMMU, and fail to work
> > > >properly).
> > >
> > > Yay!
> > 
> > Sounds good to me. Though I am curious to know the reasoning
> > behind the current logic.
> > 
> Sounds pretty good. :)
> 
> I think maybe it due to we are interested in implementing PV DMA?

Yes, we are. The current defaults came from the desire to support both
pvdma and also 1-1 host mappings. There were patches floating around
for both. Having said that, I fully agree with this change---the
default should be to require IOMMU. Anything else is "highly
experimental".

Cheers,
Muli

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Simon Horman]

On Thu, Dec 24, 2009 at 02:56:00PM +0800, Sheng Yang wrote:
> On Thursday 24 December 2009 14:51:23 Simon Horman wrote:
> > On Thu, Dec 24, 2009 at 01:45:34AM +0100, Alexander Graf wrote:
> > > Am 23.12.2009 um 23:40 schrieb Chris Wright <chrisw@sous-sol.org>:
> > > >[ resend, fixing email header, sorry for duplicate ]
> > > >
> > > >The default mode for device assignment is to rely on an IOMMU for
> > > >proper translations and a functioning device in the guest.  The
> > > >current
> > > >logic makes this requirement advisory, and simply disables the request
> > > >for IOMMU if one is not found on the host.  This makes for a confused
> > > >user when the device assignment appears to work, but the device in the
> > > >guest is not functioning  (I've seen about a half-dozen reports with
> > > >this failure mode).
> > > >
> > > >Change the logic such that the default requires the IOMMU.  Period.
> > > >If the host does not have an IOMMU, device assignment will fail.
> > > >
> > > >This is a user visible change, however I think the current
> > > >situation is
> > > >simply broken.
> > > >
> > > >And, of course, disabling the IOMMU requirement using the old:
> > > >
> > > >  -pcidevice host=[addr],dma=none
> > > >
> > > >or the newer:
> > > >
> > > >  -device pci-assign,host=[addr],iommu=0
> > > >
> > > >will do what it always did (not require an IOMMU, and fail to work
> > > >properly).
> > >
> > > Yay!
> > 
> > Sounds good to me. Though I am curious to know the reasoning
> > behind the current logic.
> > 
> Sounds pretty good. :)
> 
> I think maybe it due to we are interested in implementing PV DMA?

Ok, that would explain it.

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Marcelo Tosatti]

On Wed, Dec 23, 2009 at 02:40:20PM -0800, Chris Wright wrote:
> [ resend, fixing email header, sorry for duplicate ]
> 
> The default mode for device assignment is to rely on an IOMMU for
> proper translations and a functioning device in the guest.  The current
> logic makes this requirement advisory, and simply disables the request
> for IOMMU if one is not found on the host.  This makes for a confused
> user when the device assignment appears to work, but the device in the
> guest is not functioning  (I've seen about a half-dozen reports with
> this failure mode).
> 
> Change the logic such that the default requires the IOMMU.  Period.
> If the host does not have an IOMMU, device assignment will fail.
> 
> This is a user visible change, however I think the current situation is
> simply broken.
> 
> And, of course, disabling the IOMMU requirement using the old:
> 
>    -pcidevice host=[addr],dma=none
> 
> or the newer:
> 
>    -device pci-assign,host=[addr],iommu=0
> 
> will do what it always did (not require an IOMMU, and fail to work
> properly).

Applied, thanks.

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Alexander Graf]

Chris Wright wrote:
> [ resend, fixing email header, sorry for duplicate ]
>
> The default mode for device assignment is to rely on an IOMMU for
> proper translations and a functioning device in the guest.  The current
> logic makes this requirement advisory, and simply disables the request
> for IOMMU if one is not found on the host.  This makes for a confused
> user when the device assignment appears to work, but the device in the
> guest is not functioning  (I've seen about a half-dozen reports with
> this failure mode).
>
> Change the logic such that the default requires the IOMMU.  Period.
> If the host does not have an IOMMU, device assignment will fail.
>
> This is a user visible change, however I think the current situation is
> simply broken.
>
> And, of course, disabling the IOMMU requirement using the old:
>
>    -pcidevice host=[addr],dma=none
>
> or the newer:
>
>    -device pci-assign,host=[addr],iommu=0
>
> will do what it always did (not require an IOMMU, and fail to work
> properly).
>   

Avi,

could you please cherry-pick this into 0.12-stable?
I don't feel too eager getting 500 bug reports from people who try out
device passthrough with disabled or no IOMMU.

Alex

Re: [PATCH qemu-kvm] device assignment: default requires IOMMU

[Avi Kivity]

On 01/18/2010 04:18 PM, Alexander Graf wrote:
> Chris Wright wrote:
>    
>> [ resend, fixing email header, sorry for duplicate ]
>>
>> The default mode for device assignment is to rely on an IOMMU for
>> proper translations and a functioning device in the guest.  The current
>> logic makes this requirement advisory, and simply disables the request
>> for IOMMU if one is not found on the host.  This makes for a confused
>> user when the device assignment appears to work, but the device in the
>> guest is not functioning  (I've seen about a half-dozen reports with
>> this failure mode).
>>
>> Change the logic such that the default requires the IOMMU.  Period.
>> If the host does not have an IOMMU, device assignment will fail.
>>
>> This is a user visible change, however I think the current situation is
>> simply broken.
>>
>> And, of course, disabling the IOMMU requirement using the old:
>>
>>     -pcidevice host=[addr],dma=none
>>
>> or the newer:
>>
>>     -device pci-assign,host=[addr],iommu=0
>>
>> will do what it always did (not require an IOMMU, and fail to work
>> properly).
>>
>>      
> Avi,
>
> could you please cherry-pick this into 0.12-stable?
> I don't feel too eager getting 500 bug reports from people who try out
> device passthrough with disabled or no IOMMU.
>
>    

Makes sense.  Marcelo is committing this week, though.

-- 
error compiling committee.c: too many arguments to function