Re: [PATCH v4 26/33] xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 31/03/15 12:24, Ian Campbell wrote:
> On Thu, 2015-03-19 at 19:29 +0000, Julien Grall wrote:
>> +int iommu_do_dt_domctl(struct xen_domctl *domctl, struct domain *d,
>> +                       XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>> +{
>> +    int ret;
>> +    struct dt_device_node *dev;
>> +
>> +    /* TODO: How to deal with XSM? */
> 
> Looks like you do in the following code? Old comment?

Old comment. I forgot to drop it.

>> +    /* TODO: Do we need to check is_dying? Mostly to protect against
>> +     * hypercall trying to passthrough a device while we are
>> +     * dying.
> 
> iommu_do_pci_domctl does in specific casses (i.e. assign device). I
> guess you should follow that lead.

I'm not sure to fully understand when is_dying should be used or not.

Looking to the PCI code, the is_dying has been added when we add code to
deal with page.

I would be inclined to say it's only necessary when deadling with page.
Can someone confirm me?

Otherwise, I don't why is_dying should be check here and not in other call.

> 
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index 7f90150..a7cb272 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -475,12 +475,30 @@ typedef struct xen_domctl_sendtrigger xen_domctl_sendtrigger_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendtrigger_t);
>>  
>>
>> -/* Assign PCI device to HVM guest. Sets up IOMMU structures. */
>> +/* Assign a device to a guest. Sets up IOMMU structures. */
>>  /* XEN_DOMCTL_assign_device */
>>  /* XEN_DOMCTL_test_assign_device */
>> -/* XEN_DOMCTL_deassign_device */
>> +/*
>> + * XEN_DOMCTL_deassign_device: The behavior of this DOMCTL differs
>> + * between the different type of device:
>> + *  - PCI device (XEN_DOMCTL_DEV_PCI) will be reassigned to DOM0
>> + *  - non-PCI device (XEN_DOMCTL_DEV_PCI) will left unassigned. DOM0
> 
> Did you miss a ! before XEN_DOMCTL, or did you mean to say DT?

I intended to say XEN_DOMCTL_DT. I will fix it.

> From an ease of review PoV it would have been nice to add dev and u.pci
> first since that would be mechanical, but nevermind now.

Sorry, I was too lazy to do it.

Regards,

-- 
Julien Grall