Xen devel - Jul 2013 - [PATCH] libxl: suppress device assignment to HVM guest when there is no IOMMU

This in effect copies similar logic from xend: While there''s no way to
check whether a device is assigned to a particular guest,
XEN_DOMCTL_test_assign_device at least allows checking whether an IOMMU
is there and whether a device has been assign to _some_ guest. For the
time being, this should be enough to cover for the missing error
checking/recovery in other parts of libxl''s device assignment paths.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1036,6 +1036,18 @@ int libxl__device_pci_add(libxl__gc *gc,
     int num_assigned, i, rc;
     int stubdomid = 0;
 
+    if (libxl__domain_type(gc, domid) == LIBXL_DOMAIN_TYPE_HVM) {
+        rc = xc_test_assign_device(ctx->xch, domid,
pcidev_encode_bdf(pcidev));
+        if (rc) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "PCI device %04x:%02x:%02x.%u %s?",
+                       pcidev->domain, pcidev->bus, pcidev->dev,
pcidev->func,
+                       errno == ENOSYS ? "cannot be assigned - no
IOMMU"
+                                       : "already assigned to a different
guest");
+            goto out;
+        }
+    }
+
     rc = libxl__device_pci_setdefault(gc, pcidev);
     if (rc) goto out;
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Mon, Jul 1, 2013 at 2:08 PM, Jan Beulich <JBeulich@suse.com>
wrote:
> This in effect copies similar logic from xend: While there''s no
way to
> check whether a device is assigned to a particular guest,
> XEN_DOMCTL_test_assign_device at least allows checking whether an IOMMU
> is there and whether a device has been assign to _some_ guest. For the
> time being, this should be enough to cover for the missing error
> checking/recovery in other parts of libxl''s device assignment
paths.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Something isn''t quite right about this patch:

# xl create h0
Parsing config from h0
xc: info: VIRTUAL MEMORY ARRANGEMENT:
  Loader:        0000000000100000->000000000019ee28
  Modules:       0000000000000000->0000000000000000
  TOTAL:         0000000000000000->00000001ff800000
  ENTRY ADDRESS: 0000000000100608
xc: info: PHYSICAL MEMORY ALLOCATION:
  4KB PAGES: 0x0000000000000200
  2MB PAGES: 0x00000000000007fb
  1GB PAGES: 0x0000000000000004
libxl: error: libxl_pci.c:1046:libxl__device_pci_add: PCI device
0000:07:00.0 cannot be assigned - no IOMMU?
Daemon running with PID 3468
# xl pci-list h0
Vdev Device
00.0 0000:07:00.0
# xl pci-assignable-list
#

In other words, the xenstore stuff is still happening.

 -George

On Mon, Jul 1, 2013 at 2:45 PM, George Dunlap
<George.Dunlap@eu.citrix.com> wrote:
> On Mon, Jul 1, 2013 at 2:08 PM, Jan Beulich <JBeulich@suse.com>
wrote:
>> This in effect copies similar logic from xend: While there''s
no way to
>> check whether a device is assigned to a particular guest,
>> XEN_DOMCTL_test_assign_device at least allows checking whether an IOMMU
>> is there and whether a device has been assign to _some_ guest. For the
>> time being, this should be enough to cover for the missing error
>> checking/recovery in other parts of libxl''s device assignment
paths.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Something isn''t quite right about this patch:
>
> # xl create h0
> Parsing config from h0
> xc: info: VIRTUAL MEMORY ARRANGEMENT:
>   Loader:        0000000000100000->000000000019ee28
>   Modules:       0000000000000000->0000000000000000
>   TOTAL:         0000000000000000->00000001ff800000
>   ENTRY ADDRESS: 0000000000100608
> xc: info: PHYSICAL MEMORY ALLOCATION:
>   4KB PAGES: 0x0000000000000200
>   2MB PAGES: 0x00000000000007fb
>   1GB PAGES: 0x0000000000000004
> libxl: error: libxl_pci.c:1046:libxl__device_pci_add: PCI device
> 0000:07:00.0 cannot be assigned - no IOMMU?
> Daemon running with PID 3468
> # xl pci-list h0
> Vdev Device
> 00.0 0000:07:00.0
> # xl pci-assignable-list
> #
>
> In other words, the xenstore stuff is still happening.
OK, I think this patch is probably still a good thing to take.  It
does result in the device getting "stuck" in this intermetidate state
until the domain shuts down (can''t assign it to someone else,
can''t
pci-detach either); but once the domain does shut down, everything
goes back to normal.  That''s better than the potential data corruption
/ security issue we had before.

Tested-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

George Dunlap wrote on 2013-07-01:
> On Mon, Jul 1, 2013 at 2:45 PM, George Dunlap
> <George.Dunlap@eu.citrix.com> wrote:
>> On Mon, Jul 1, 2013 at 2:08 PM, Jan Beulich <JBeulich@suse.com>
wrote:
>>> This in effect copies similar logic from xend: While
there''s no way to
>>> check whether a device is assigned to a particular guest,
>>> XEN_DOMCTL_test_assign_device at least allows checking whether an
>>> IOMMU is there and whether a device has been assign to _some_
guest.
>>> For the time being, this should be enough to cover for the missing
>>> error checking/recovery in other parts of libxl''s device
assignment
>>> paths.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Something isn''t quite right about this patch:
>> 
>> # xl create h0
>> Parsing config from h0
>> xc: info: VIRTUAL MEMORY ARRANGEMENT:
>>   Loader:        0000000000100000->000000000019ee28 Modules:      
>>   0000000000000000->0000000000000000 TOTAL:        
>>   0000000000000000->00000001ff800000 ENTRY ADDRESS: 0000000000100608
>>   xc: info: PHYSICAL MEMORY ALLOCATION: 4KB PAGES: 0x0000000000000200
>>   2MB PAGES: 0x00000000000007fb 1GB PAGES: 0x0000000000000004
>> libxl: error: libxl_pci.c:1046:libxl__device_pci_add: PCI device
>> 0000:07:00.0 cannot be assigned - no IOMMU?
>> Daemon running with PID 3468
>> # xl pci-list h0
>> Vdev Device
>> 00.0 0000:07:00.0
>> # xl pci-assignable-list
>> #
>> 
>> In other words, the xenstore stuff is still happening.
Still got confused. If no IOMMU, why there will have device assignable? But I
think there may no good way to solve it since pci-assignalbe-list don''t
need IOMMU''s intercept. Perhaps we should show whether IOMMU is enabled
with ''xl pci-assignable-list '' to let user know more earlier.
> 
> OK, I think this patch is probably still a good thing to take.  It
> does result in the device getting "stuck" in this intermetidate
state
> until the domain shuts down (can''t assign it to someone else,
can''t
> pci-detach either); but once the domain does shut down, everything
> goes back to normal.  That''s better than the potential data
corruption
> / security issue we had before.
> 
> Tested-by: George Dunlap <george.dunlap@eu.citrix.com>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Best regards,
Yang

>>> On 02.07.13 at 04:27, "Zhang, Yang Z"
<yang.z.zhang@intel.com> wrote:
> George Dunlap wrote on 2013-07-01:
>>> # xl pci-assignable-list
>>> #
>>> 
>>> In other words, the xenstore stuff is still happening.
> Still got confused. If no IOMMU, why there will have device assignable? But
> I think there may no good way to solve it since pci-assignalbe-list
don''t need
> IOMMU''s intercept. Perhaps we should show whether IOMMU is enabled
with ''xl
> pci-assignable-list '' to let user know more earlier.
Pass-through to PV guests doesn''t require an IOMMU (but of course
it''s not secure without one).

Jan

Jan Beulich wrote on 2013-07-02:
>>>> On 02.07.13 at 04:27, "Zhang, Yang Z"
<yang.z.zhang@intel.com> wrote:
>>>> George Dunlap wrote on 2013-07-01: # xl pci-assignable-list #
>>>> 
>>>> In other words, the xenstore stuff is still happening.
>> Still got confused. If no IOMMU, why there will have device assignable?
>> But I think there may no good way to solve it since pci-assignalbe-list
>> don''t need IOMMU''s intercept. Perhaps we should show
whether IOMMU is
>> enabled with ''xl pci-assignable-list '' to let user
know more earlier.
> 
> Pass-through to PV guests doesn''t require an IOMMU (but of course
> it''s not secure without one).
The message is only for HVM case, user can just ignore it if he want to try PV
guest pass through.

Best regards,
Yang

On Tue, Jul 2, 2013 at 3:27 AM, Zhang, Yang Z <yang.z.zhang@intel.com>
wrote:
> George Dunlap wrote on 2013-07-01:
>> On Mon, Jul 1, 2013 at 2:45 PM, George Dunlap
>> <George.Dunlap@eu.citrix.com> wrote:
>>> On Mon, Jul 1, 2013 at 2:08 PM, Jan Beulich
<JBeulich@suse.com> wrote:
>>>> This in effect copies similar logic from xend: While
there''s no way to
>>>> check whether a device is assigned to a particular guest,
>>>> XEN_DOMCTL_test_assign_device at least allows checking whether
an
>>>> IOMMU is there and whether a device has been assign to _some_
guest.
>>>> For the time being, this should be enough to cover for the
missing
>>>> error checking/recovery in other parts of libxl''s
device assignment
>>>> paths.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Something isn''t quite right about this patch:
>>>
>>> # xl create h0
>>> Parsing config from h0
>>> xc: info: VIRTUAL MEMORY ARRANGEMENT:
>>>   Loader:        0000000000100000->000000000019ee28 Modules:
>>>   0000000000000000->0000000000000000 TOTAL:
>>>   0000000000000000->00000001ff800000 ENTRY ADDRESS:
0000000000100608
>>>   xc: info: PHYSICAL MEMORY ALLOCATION: 4KB PAGES:
0x0000000000000200
>>>   2MB PAGES: 0x00000000000007fb 1GB PAGES: 0x0000000000000004
>>> libxl: error: libxl_pci.c:1046:libxl__device_pci_add: PCI device
>>> 0000:07:00.0 cannot be assigned - no IOMMU?
>>> Daemon running with PID 3468
>>> # xl pci-list h0
>>> Vdev Device
>>> 00.0 0000:07:00.0
>>> # xl pci-assignable-list
>>> #
>>>
>>> In other words, the xenstore stuff is still happening.
> Still got confused. If no IOMMU, why there will have device assignable? But
I think there may no good way to solve it since pci-assignalbe-list
don''t need IOMMU''s intercept. Perhaps we should show whether
IOMMU is enabled with ''xl pci-assignable-list '' to let user
know more earlier.
Remember that PV guests can be assigned a device even without an IOMMU
(although it''s less safe).

Expected behavior:
# xl pci-assignable-add 07:00.0
# xl pci-assignable-list
07:00.0
# xl pci-attach hvm-domain 07:00.0
[error]
# xl pci-assignable-list
07:00.0
# xl pci-attach pv-domain 07:00.0
[works]

Actual behavior:
# xl pci-assignable-add 07:00.0
# xl pci-assignable-list
07:00.0
# xl pci-attach hvm-domain 07:00.0
[error]
# xl pci-assignable-list
[nothing!]
# xl pci-attach pv-domain 07:00.0
Error: 07:00.0 not assignable!

In other words, even with this patch we are still going through some
of the motions of assigning the device.  If the device assignment
fails, *nothing* should be done; and the device should still be
available to be assigned to someone else.  Instead, it is partially
assigned, and after the failure is not available to be assigned to
another guest.

But this is good enough for now.

 -George

George Dunlap wrote on 2013-07-02:
> On Tue, Jul 2, 2013 at 3:27 AM, Zhang, Yang Z
<yang.z.zhang@intel.com> wrote:
>> George Dunlap wrote on 2013-07-01:
>>> On Mon, Jul 1, 2013 at 2:45 PM, George Dunlap
>>> <George.Dunlap@eu.citrix.com> wrote:
>>>> On Mon, Jul 1, 2013 at 2:08 PM, Jan Beulich
<JBeulich@suse.com> wrote:
>>>>> This in effect copies similar logic from xend: While
there''s no way to
>>>>> check whether a device is assigned to a particular guest,
>>>>> XEN_DOMCTL_test_assign_device at least allows checking
whether an
>>>>> IOMMU is there and whether a device has been assign to
_some_ guest.
>>>>> For the time being, this should be enough to cover for the
missing
>>>>> error checking/recovery in other parts of libxl''s
device assignment
>>>>> paths.
>>>>> 
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> 
>>>> Something isn''t quite right about this patch:
>>>> 
>>>> # xl create h0
>>>> Parsing config from h0
>>>> xc: info: VIRTUAL MEMORY ARRANGEMENT:
>>>>   Loader:        0000000000100000->000000000019ee28 Modules:
>>>>   0000000000000000->0000000000000000 TOTAL:
>>>>   0000000000000000->00000001ff800000 ENTRY ADDRESS:
0000000000100608
>>>>   xc: info: PHYSICAL MEMORY ALLOCATION: 4KB PAGES:
0x0000000000000200
>>>>   2MB PAGES: 0x00000000000007fb 1GB PAGES: 0x0000000000000004
>>>> libxl: error: libxl_pci.c:1046:libxl__device_pci_add: PCI
device
>>>> 0000:07:00.0 cannot be assigned - no IOMMU?
>>>> Daemon running with PID 3468
>>>> # xl pci-list h0
>>>> Vdev Device
>>>> 00.0 0000:07:00.0
>>>> # xl pci-assignable-list
>>>> #
>>>> 
>>>> In other words, the xenstore stuff is still happening.
>> Still got confused. If no IOMMU, why there will have device assignable?
But I
> think there may no good way to solve it since pci-assignalbe-list
don''t need
> IOMMU''s intercept. Perhaps we should show whether IOMMU is enabled
with
> ''xl pci-assignable-list '' to let user know more earlier.
> 
> Remember that PV guests can be assigned a device even without an IOMMU
> (although it''s less safe).
Yes. I mean pci-assignable-list can show whether IOMMU is enabled. Since PV
guest doesn''t need IOMMU, it can just ignore it. It just as a reminder
for user to do VT-d device pass-through.
> 
> Expected behavior:
> # xl pci-assignable-add 07:00.0
> # xl pci-assignable-list
> 07:00.0
> # xl pci-attach hvm-domain 07:00.0
> [error]
> # xl pci-assignable-list
> 07:00.0
> # xl pci-attach pv-domain 07:00.0
> [works]
> 
> Actual behavior:
> # xl pci-assignable-add 07:00.0
> # xl pci-assignable-list
> 07:00.0
> # xl pci-attach hvm-domain 07:00.0
> [error]
> # xl pci-assignable-list
> [nothing!]
> # xl pci-attach pv-domain 07:00.0
> Error: 07:00.0 not assignable!
> 
> In other words, even with this patch we are still going through some
> of the motions of assigning the device.  If the device assignment
> fails, *nothing* should be done; and the device should still be
> available to be assigned to someone else.  Instead, it is partially
> assigned, and after the failure is not available to be assigned to
> another guest.
> 
> But this is good enough for now.
> 
>  -George

Best regards,
Yang