Jan Beulich wrote:
Weidong,
in this patch you removed a bus/devfn check around an invocation of
domain_context_mapping_one() avoiding the attempt to call the
function again if it was already called for this very device. This
removal, however, conflicts with the context_present() check at the
top of domain_context_mapping_one() - in particular, pdev->domain
isn't set to the new owner yet, and hence the function fails.
The question now is whether some similar check should be restored,
or whether pdev->domain should get updated earlier. This may
I prefer to add the check.
need some additional consideration, since from looking at the code
I would say that reassign_device_ownership() needs some error
handling improvements too: Currently, partial failure isn't being
handled properly at all (respective devices are left in a half way
state - no longer properly assigned to Dom0, but also not yet
assigned to DomU).
Agree. The assignment should guarantee "done" or "none".
I also wonder what guarantee there is for a device to exist at
<secbus>:00.0 (since if there is none, the same context_present()
check could at least theoretically again lead to problems as it
checks for pci_get_pdev() returning non-NULL
Finally, isn't it inconsistent that only the original device gets its
->domain set to the new owner and gets moved to that domain's
device list, but neither the upstream bridge nor that bridge's
<secbus>:00.0 get handled the same way? What if below that
Yes, it's better to do the same for the upstream bridge.
bridge a device gets hot-added? Wouldn't that device
incorrectly end up in Dom0, with no failures because the bridge
still appears to be owned by Dom0 while it really isn't?
Do you want some error message for this case?
Regards,
Weidong
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
| 
Home