From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 13/21] xen/dts: Add hypercalls to retrieve device node information Date: Wed, 06 Aug 2014 16:17:48 +0100 Message-ID: <53E2471C.6020109@linaro.org> References: <1406818852-31856-1-git-send-email-julien.grall@linaro.org> <1406818852-31856-14-git-send-email-julien.grall@linaro.org> <53DB70FD020000780002850B@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XF2yR-0006R7-12 for xen-devel@lists.xenproject.org; Wed, 06 Aug 2014 15:17:55 +0000 Received: by mail-we0-f172.google.com with SMTP id x48so2796399wes.31 for ; Wed, 06 Aug 2014 08:17:53 -0700 (PDT) In-Reply-To: <53DB70FD020000780002850B@mail.emea.novell.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: Jan Beulich Cc: Ian Jackson , tim@xen.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org Hi Jan, On 08/01/2014 09:50 AM, Jan Beulich wrote: >>>> On 31.07.14 at 17:00, wrote: >> --- a/xen/common/domctl.c >> +++ b/xen/common/domctl.c >> @@ -315,6 +315,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >> case XEN_DOMCTL_createdomain: >> case XEN_DOMCTL_getdomaininfo: >> case XEN_DOMCTL_test_assign_device: >> + case XEN_DOMCTL_dtdev_op: >> d = NULL; >> break; >> default: >> @@ -1017,6 +1018,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >> } >> break; >> >> +#ifdef HAS_DEVICE_TREE >> + case XEN_DOMCTL_dtdev_op: >> + { >> + ret = dt_do_domctl(op); >> + copyback = 1; >> + } >> + break; >> +#endif > > No pointless braces please. Will drop it. >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -946,6 +946,44 @@ typedef struct xen_domctl_vcpu_msrs xen_domctl_vcpu_msrs_t; >> DEFINE_XEN_GUEST_HANDLE(xen_domctl_vcpu_msrs_t); >> #endif >> >> +/* Device Tree: Retrieve informations about a device node */ >> +struct xen_domctl_dtdev_op { >> + /* IN */ >> + uint32_t plen; /* Length of the path */ >> + XEN_GUEST_HANDLE(char) path; /* Path to the device tree node */ > > XEN_GUEST_HANDLE_64? Right. I never know when I should use XEN_GUEST_HANDLE_64 or XEN_GUEST_HANDLE. FYI, it's exactly the same on ARM. > And padding between the two above > members, or fields re-ordered? I can reorder the field. >> +#define DOMCTL_DTDEV_GET_INFO 0 >> +#define DOMCTL_DTDEV_GET_IRQ 1 >> +#define DOMCTL_DTDEV_GET_MMIO 2 >> +#define DOMCTL_DTDEV_GET_COMPAT 3 >> + uint8_t op; >> + uint32_t pad0:24; > > uint8_t pad0[3]. Ok. >> + uint32_t index; /* Index for the IRQ/MMIO to retrieve */ >> + /* OUT */ >> + union { >> + struct { >> + uint32_t num_irqs; /* Number of IRQs */ >> + uint32_t num_mmios; /* Number of MMIOs */ >> + uint32_t compat_len; /* Length of the compatible string */ >> + } info; >> + struct { >> + /* TODO: Do we need to handle MSI-X? */ >> + uint32_t irq; /* IRQ number */ >> + /* TODO: Describe with defines the IRQ type */ > > ??? For now we are using the DT_IRQ_TYPE_* provided by the device tree (see include/xen/device_tree.h). I haven't yet introduced specific IRQ type here. > Also, are you planning to address these two TODOs before this gets > ready to be committed? I plan to handle the latter TODO (i.e IRQ type), for the MSI I don't know how they will be described in the device tree. So I will drop the TODO. It will be fine because it's a DOMCTL so it's possible to change the interface easily. >> + uint32_t type; /* IRQ type (i.e edge, level...) */ > > #define-s to specify what values here mean? See my answer a bit above. >> + } irq; >> + struct { >> + xen_pfn_t mfn; >> + xen_pfn_t nr_mfn; >> + } mmio; >> + struct { >> + uint32_t len; /* IN: Size of buffer. OUT: Size copied */ >> + XEN_GUEST_HANDLE_64(char) compat; > > Padding again? I will invert the 2 fields. Regards, -- Julien Grall