From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v3 20/24] xen/passthrough: Extend XEN_DOMCTL_assign_device to support DT device Date: Thu, 29 Jan 2015 15:03:36 +0000 Message-ID: References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-21-git-send-email-julien.grall@linaro.org> <54CA1D6A.8000609@linaro.org> <54CA3100.10500@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YGqe1-0001Uq-Tn for xen-devel@lists.xenproject.org; Thu, 29 Jan 2015 15:04:34 +0000 In-Reply-To: <54CA3100.10500@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Wei Liu , ian.campbell@citrix.com, Stefano Stabellini , tim@xen.org, Ian Jackson , stefano.stabellini@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Thu, 29 Jan 2015, Julien Grall wrote: > On 29/01/15 12:09, Stefano Stabellini wrote: > > On Thu, 29 Jan 2015, Julien Grall wrote: > >> Hi Stefano, > >> > >> On 29/01/15 10:29, Stefano Stabellini wrote: > >>>> +static bool_t iommu_dt_device_is_assigned(const struct dt_device_node *dev) > >>>> +{ > >>>> + bool_t assigned = 0; > >>>> + > >>>> + if ( !dt_device_is_protected(dev) ) > >>>> + return 1; > >>> > >>> Why return true here? > >> > >> Because any device not protected cannot be assigned to another guest. > >> This could be used by the toolstack to know whether the device is > >> assigned or not. > > > > I understand that much. > > > > > >> IHMO, returning 0 would be a false negative. > > > > Why? Returning 0 means that the device is not assigned, that would be > > correct. From this statement I think that actually you are thinking as > > if this function actually returned whether a given device is assignable. > > 0 means the device is not assigned and therefore can be used for > passthrough. I disagree on the "therefore". 0 means (or should mean that) the device is not assigned and stop there. > This function is used in the DOMCTL test_assign_device. That would mean > we will return 0 for non-protected device. That doesn't sound right as a > such device can never be assigned. assigned -> passed through to a VM assignable -> can potentially be assigned A non-assignable device cannot be assigned by definition. It does not make sense to returned true for it, if the question is "is it assigned?". > > In that case you should rename the function to > > > > iommu_dt_device_is_assignable > > dt_device_is_assignable is not more clear. I disagree. > Return 0 here would mean the > device cannot be assigned in the sense it's not protected. > > Moreover, this name would not be in sync with the DOMCTL test_assign_device.