| Message ID | 1467688367-17320-18-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. Another question to Julien and Stefano: is it normal for ARM guest to use hvm callback vector? Wei. > + val |= GUEST_EVTCHN_PPI & 0xff; > + rc = xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CALLBACK_IRQ, > + val); > + if (rc) > + return rc; > + > rc = libxl__prepare_dtb(gc, info, state, dom); > if (rc) > return rc; > -- > 2.0.4 > >
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. > > Another question to Julien and Stefano: is it normal for ARM guest to > use hvm callback vector? Yes. The HVM callback vector is used by ACPI guest to find the PPI (per-cpu interrupt) which will be used to notify event. This is how DOM0 is using ACPI on ARM (see [1]), I don't think we should differ here. BTW, I have noticed that the design doc is only available on the ML. Shannon, would it be possible to send a patch to add it in docs/misc/arm? [2] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html Regards,
On 2016/7/8 0:57, Julien Grall wrote: > 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. > ok, so do we decide to update that? >> >> Another question to Julien and Stefano: is it normal for ARM guest to >> use hvm callback vector? > > Yes. The HVM callback vector is used by ACPI guest to find the PPI > (per-cpu interrupt) which will be used to notify event. > > This is how DOM0 is using ACPI on ARM (see [1]), I don't think we should > differ here. > > BTW, I have noticed that the design doc is only available on the ML. > Shannon, would it be possible to send a patch to add it in docs/misc/arm? > Ok, I'll send a patch later. > [2] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html
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.
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 */ + val |= GUEST_EVTCHN_PPI & 0xff; + rc = xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CALLBACK_IRQ, + val); + if (rc) + return rc; + rc = libxl__prepare_dtb(gc, info, state, dom); if (rc) return rc;