From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 20/24] xen/passthrough: Extend XEN_DOMCTL_assign_device to support DT device Date: Thu, 29 Jan 2015 13:09:20 +0000 Message-ID: <54CA3100.10500@linaro.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YGor0-0007gf-Ry for xen-devel@lists.xenproject.org; Thu, 29 Jan 2015 13:09:50 +0000 Received: by mail-wi0-f174.google.com with SMTP id n3so26032704wiv.1 for ; Thu, 29 Jan 2015 05:09:49 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: Wei Liu , ian.campbell@citrix.com, tim@xen.org, Ian Jackson , stefano.stabellini@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org 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. 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. > In that case you should rename the function to > > iommu_dt_device_is_assignable dt_device_is_assignable is not more clear. 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. Regards, -- Julien Grall