From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4 26/33] xen/passthrough: Extend XEN_DOMCTL_*assign_device to support DT device Date: Tue, 31 Mar 2015 13:30:04 +0100 Message-ID: <551A934C.6090908@linaro.org> References: <1426793399-6283-1-git-send-email-julien.grall@linaro.org> <1426793399-6283-27-git-send-email-julien.grall@linaro.org> <1427801079.2115.84.camel@citrix.com> 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 1YcvJS-0005UC-ET for xen-devel@lists.xenproject.org; Tue, 31 Mar 2015 12:30:34 +0000 Received: by wgdm6 with SMTP id m6so17338605wgd.2 for ; Tue, 31 Mar 2015 05:30:31 -0700 (PDT) In-Reply-To: <1427801079.2115.84.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Wei Liu , Ian Jackson , tim@xen.org, stefano.stabellini@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.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