Re: [PATCH v3 17/17] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Jul 07, 2016 at 05:57:09PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 07/07/16 17:15, Wei Liu wrote:
> >On Tue, Jul 05, 2016 at 11:12:47AM +0800, Shannon Zhao wrote:
> >>From: Shannon Zhao <shannon.zhao@linaro.org>
> >>
> >>The guest kernel will get the event channel interrupt information via
> >>domain param HVM_PARAM_CALLBACK_IRQ. Initialize it here.
> >>
> >>Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>---
> >>  tools/libxl/libxl_arm.c | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >>diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >>index bc38318..acacba0 100644
> >>--- a/tools/libxl/libxl_arm.c
> >>+++ b/tools/libxl/libxl_arm.c
> >>@@ -900,8 +900,19 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >>                                             struct xc_dom_image *dom)
> >>  {
> >>      int rc;
> >>+    uint64_t val;
> >>
> >>      assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> >>+
> >>+    /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
> >>+    val = (uint64_t)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> >>+    val |= (2 << 8); /* Active-low level-sensitive  */
> >
> >Please avoid using magic numbers here -- 56, 2 and 8.
> 
> The magic numbers are described in public/hvm/params.h however there is no
> defines associated to them.
> 
> The public header would need to be updated if we don't want the value
> hardcoded in libxl.
> 

Either update the public header or have some local #defines plus
appropriate comments on the what the actual source of those numbers is.

FWIW I certainly prefer the formal option.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel