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

[Wei Liu]

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 
> >>
> >>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 
> >>---
> >>  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

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

[Shannon Zhao]

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 
>>>
>>> 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 
>>> ---
>>>   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

-- 
Shannon


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

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

[Julien Grall]

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 

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 
---
  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,

--
Julien Grall

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

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

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:47AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> 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 
> ---
>  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
> 
> 

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

[Xen-devel] [PATCH v3 17/17] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Shannon Zhao]

From: Shannon Zhao 

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 
---
 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  */
+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



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