diff mbox

[v4,14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

Message ID 1471343113-10652-15-git-send-email-zhaoshenglong@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shannon Zhao Aug. 16, 2016, 10:25 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
them in evtchn_fixup().

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/arch/arm/domain_build.c     | 8 +++++---
 xen/include/public/hvm/params.h | 4 ++++
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Wei Liu Aug. 24, 2016, 1:06 p.m. UTC | #1
CC Jan and Andrew to review change in hvm/params.h

On Tue, Aug 16, 2016 at 06:25:11PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
> them in evtchn_fixup().
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/arch/arm/domain_build.c     | 8 +++++---
>  xen/include/public/hvm/params.h | 4 ++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..94cd3ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>             d->arch.evtchn_irq);
>  
>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> -    val |= (2 << 8); /* Active-low level-sensitive  */
> -    val |= d->arch.evtchn_irq & 0xff;
> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
> +    /* Active-low level-sensitive  */
> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>  
>      /*
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>  
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>  /*
>   * How should CPU0 event-channel notifications be delivered?
>   *
> @@ -66,6 +67,9 @@
>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
>   * the notification is handled by the interrupt controller.
>   */
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
>  #endif
>  
>  /*
> -- 
> 2.0.4
> 
>
Jan Beulich Aug. 24, 2016, 1:15 p.m. UTC | #2
>>> On 24.08.16 at 15:06, <wei.liu2@citrix.com> wrote:
> CC Jan and Andrew to review change in hvm/params.h

Well, it looks reasonable, but this really needs to first be approved by
the ARM maintainers.

Jan

> On Tue, Aug 16, 2016 at 06:25:11PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>> 
>> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
>> them in evtchn_fixup().
>> 
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  xen/arch/arm/domain_build.c     | 8 +++++---
>>  xen/include/public/hvm/params.h | 4 ++++
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>> 
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 60db9e4..94cd3ce 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct 
> kernel_info *kinfo)
>>             d->arch.evtchn_irq);
>>  
>>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
>> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
>> -    val |= (2 << 8); /* Active-low level-sensitive  */
>> -    val |= d->arch.evtchn_irq & 0xff;
>> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 
> HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
>> +    /* Active-low level-sensitive  */
>> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
>> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
>> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
>>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>>  
>>      /*
>> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
>> index f7338a3..8a0327d 100644
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -30,6 +30,7 @@
>>   */
>>  
>>  #define HVM_PARAM_CALLBACK_IRQ 0
>> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>>  /*
>>   * How should CPU0 event-channel notifications be delivered?
>>   *
>> @@ -66,6 +67,9 @@
>>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated 
> to
>>   * the notification is handled by the interrupt controller.
>>   */
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
>>  #endif
>>  
>>  /*
>> -- 
>> 2.0.4
>> 
>>
Boris Ostrovsky Aug. 24, 2016, 10:27 p.m. UTC | #3
On 08/16/2016 06:25 AM, Shannon Zhao wrote:
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>  
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56

May be worth updating hvm_set_callback_via() with this parameter.

-boris
Julien Grall Aug. 29, 2016, 7 p.m. UTC | #4
Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
> them in evtchn_fixup().
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/arch/arm/domain_build.c     | 8 +++++---
>  xen/include/public/hvm/params.h | 4 ++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..94cd3ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>             d->arch.evtchn_irq);
>
>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> -    val |= (2 << 8); /* Active-low level-sensitive  */
> -    val |= d->arch.evtchn_irq & 0xff;
> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
> +    /* Active-low level-sensitive  */
> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;

The mask is pointless and a call to make things much worse if evtchn_irq 
is not a PPI (which will never happen).

>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>
>      /*
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>  /*
>   * How should CPU0 event-channel notifications be delivered?
>   *
> @@ -66,6 +67,9 @@
>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
>   * the notification is handled by the interrupt controller.
>   */
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff

Please drop the PPI_MASK, it is not correctly defined (there is only 16 
PPI going from 16 - 32) and pointless.

>  #endif
>
>  /*
>

Regards,
Shannon Zhao Aug. 30, 2016, 1:30 a.m. UTC | #5
On 2016/8/30 3:00, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
>> them in evtchn_fixup().
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  xen/arch/arm/domain_build.c     | 8 +++++---
>>  xen/include/public/hvm/params.h | 4 ++++
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 60db9e4..94cd3ce 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d,
>> struct kernel_info *kinfo)
>>             d->arch.evtchn_irq);
>>
>>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
>> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
>> -    val |= (2 << 8); /* Active-low level-sensitive  */
>> -    val |= d->arch.evtchn_irq & 0xff;
>> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI <<
>> HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
>> +    /* Active-low level-sensitive  */
>> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
>> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
>> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
> 
> The mask is pointless and a call to make things much worse if evtchn_irq
> is not a PPI (which will never happen).
> 
>>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>>
>>      /*
>> diff --git a/xen/include/public/hvm/params.h
>> b/xen/include/public/hvm/params.h
>> index f7338a3..8a0327d 100644
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -30,6 +30,7 @@
>>   */
>>
>>  #define HVM_PARAM_CALLBACK_IRQ 0
>> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>>  /*
>>   * How should CPU0 event-channel notifications be delivered?
>>   *
>> @@ -66,6 +67,9 @@
>>   * This is only used by ARM/ARM64 and masking/eoi the interrupt
>> associated to
>>   * the notification is handled by the interrupt controller.
>>   */
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
> 
> Please drop the PPI_MASK, it is not correctly defined (there is only 16
> PPI going from 16 - 32) and pointless.
> 
Sure. Thanks.
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 60db9e4..94cd3ce 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2019,9 +2019,11 @@  static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
            d->arch.evtchn_irq);
 
     /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
-    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
-    val |= (2 << 8); /* Active-low level-sensitive  */
-    val |= d->arch.evtchn_irq & 0xff;
+    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
+    /* Active-low level-sensitive  */
+    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
+            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
+    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
     d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
 
     /*
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index f7338a3..8a0327d 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -30,6 +30,7 @@ 
  */
 
 #define HVM_PARAM_CALLBACK_IRQ 0
+#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
 /*
  * How should CPU0 event-channel notifications be delivered?
  *
@@ -66,6 +67,9 @@ 
  * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
  * the notification is handled by the interrupt controller.
  */
+#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
+#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
+#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
 #endif
 
 /*