xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ
@ 2016-02-29 20:39 Konrad Rzeszutek Wilk
  2016-02-29 20:42 ` Konrad Rzeszutek Wilk
  2016-03-01 10:25 ` Jan Beulich
  0 siblings, 2 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-29 20:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, boris.ostrovsky, Keir Fraser, Jan Beulich,
	Konrad Rzeszutek Wilk

Couple of updates:
 - Add an macro to make it easier to use the vector callback.

 - Fix the odditity in Xen internal usage of an enum which offset
   by one compared to the #defines. Make it the same.

 - This also clears up the printing of the Callback in the
   irq_dump() to match up with the #defines.

Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/arch/x86/hvm/irq.c          |  2 +-
 xen/include/asm-x86/hvm/irq.h   | 12 ++++++++----
 xen/include/public/hvm/hvm_op.h |  3 ++-
 xen/include/public/hvm/params.h | 15 +++++++++++++++
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index 5323d7c..0c6ead4 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -325,7 +325,7 @@ void hvm_set_callback_via(struct domain *d, uint64_t via)
     unsigned int gsi=0, pdev=0, pintx=0;
     uint8_t via_type;
 
-    via_type = (uint8_t)(via >> 56) + 1;
+    via_type = (uint8_t)(via >> HVM_PARAM_CALLBACK_TYPE_SHIFT);
     if ( ((via_type == HVMIRQ_callback_gsi) && (via == 0)) ||
          (via_type > HVMIRQ_callback_vector) )
         via_type = HVMIRQ_callback_none;
diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 73b8fb0..2a301db 100644
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -26,6 +26,8 @@
 #include <asm/hvm/vpic.h>
 #include <asm/hvm/vioapic.h>
 
+#include <public/hvm/params.h>
+
 struct hvm_irq {
     /*
      * Virtual interrupt wires for a single PCI bus.
@@ -50,11 +52,13 @@ struct hvm_irq {
     /* Virtual interrupt and via-link for paravirtual platform driver. */
     uint32_t callback_via_asserted;
     union {
+        /* These MUST match with HVM_PARAM_CALLBACK_IRQ types. */
         enum {
-            HVMIRQ_callback_none,
-            HVMIRQ_callback_gsi,
-            HVMIRQ_callback_pci_intx,
-            HVMIRQ_callback_vector
+            HVMIRQ_callback_gsi = HVM_PARAM_CALLBACK_TYPE_GSI,
+            HVMIRQ_callback_pci_intx = HVM_PARAM_CALLBACK_TYPE_PCI_INTX,
+            HVMIRQ_callback_vector = HVM_PARAM_CALLBACK_TYPE_VECTOR,
+            /* Will change if we add more types. */
+            HVMIRQ_callback_none = HVM_PARAM_CALLBACK_TYPE_NUM,
         } callback_via_type;
     };
     union {
diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 1606185..5908f39 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -386,7 +386,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_ioreq_server_state_t);
  *                                 channel upcalls on the specified <vcpu>. If set,
  *                                 this vector will be used in preference to the
  *                                 domain global callback via (see
- *                                 HVM_PARAM_CALLBACK_IRQ).
+ *                                 HVM_PARAM_CALLBACK_IRQ and
+ *                                 HVM_PARAM_CALLBACK_VECTOR).
  */
 #define HVMOP_set_evtchn_upcall_vector 23
 struct xen_hvm_evtchn_upcall_vector {
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 73d4718..5c7fbc5 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -56,6 +56,21 @@
  */
 
 /*
+ * In the future this may change.
+ */
+#define HVM_PARAM_CALLBACK_TYPE_NUM      HVM_PARAM_CALLBACK_TYPE_VECTOR + 1
+/*
+ * The val[63:56] convenience shift.
+ */
+#define HVM_PARAM_CALLBACK_TYPE_SHIFT 56
+
+/*
+ * Wrapper around for HVM_PARAM_CALLBACK_TYPE_VECTOR.
+ */
+#define HVM_PARAM_CALLBACK_VECTOR(x) \
+            (((uint64_t)HVM_PARAM_CALLBACK_TYPE_VECTOR) << \
+                        HVM_PARAM_CALLBACK_TYPE_SHIFT | (x))
+/*
  * These are not used by Xen. They are here for convenience of HVM-guest
  * xenbus implementations.
  */
-- 
2.4.3


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ
  2016-02-29 20:39 [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ Konrad Rzeszutek Wilk
@ 2016-02-29 20:42 ` Konrad Rzeszutek Wilk
  2016-03-01 10:25 ` Jan Beulich
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-02-29 20:42 UTC (permalink / raw)
  To: xen-devel, Stefano Stabellini
  Cc: Andrew Cooper, boris.ostrovsky, Keir Fraser, Jan Beulich

On Mon, Feb 29, 2016 at 03:39:36PM -0500, Konrad Rzeszutek Wilk wrote:
> Couple of updates:
>  - Add an macro to make it easier to use the vector callback.
> 
>  - Fix the odditity in Xen internal usage of an enum which offset
>    by one compared to the #defines. Make it the same.
> 
>  - This also clears up the printing of the Callback in the
>    irq_dump() to match up with the #defines.
> 
> Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

CC-ing Stefano.
> ---
>  xen/arch/x86/hvm/irq.c          |  2 +-
>  xen/include/asm-x86/hvm/irq.h   | 12 ++++++++----
>  xen/include/public/hvm/hvm_op.h |  3 ++-
>  xen/include/public/hvm/params.h | 15 +++++++++++++++
>  4 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index 5323d7c..0c6ead4 100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -325,7 +325,7 @@ void hvm_set_callback_via(struct domain *d, uint64_t via)
>      unsigned int gsi=0, pdev=0, pintx=0;
>      uint8_t via_type;
>  
> -    via_type = (uint8_t)(via >> 56) + 1;
> +    via_type = (uint8_t)(via >> HVM_PARAM_CALLBACK_TYPE_SHIFT);
>      if ( ((via_type == HVMIRQ_callback_gsi) && (via == 0)) ||
>           (via_type > HVMIRQ_callback_vector) )
>          via_type = HVMIRQ_callback_none;
> diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
> index 73b8fb0..2a301db 100644
> --- a/xen/include/asm-x86/hvm/irq.h
> +++ b/xen/include/asm-x86/hvm/irq.h
> @@ -26,6 +26,8 @@
>  #include <asm/hvm/vpic.h>
>  #include <asm/hvm/vioapic.h>
>  
> +#include <public/hvm/params.h>
> +
>  struct hvm_irq {
>      /*
>       * Virtual interrupt wires for a single PCI bus.
> @@ -50,11 +52,13 @@ struct hvm_irq {
>      /* Virtual interrupt and via-link for paravirtual platform driver. */
>      uint32_t callback_via_asserted;
>      union {
> +        /* These MUST match with HVM_PARAM_CALLBACK_IRQ types. */
>          enum {
> -            HVMIRQ_callback_none,
> -            HVMIRQ_callback_gsi,
> -            HVMIRQ_callback_pci_intx,
> -            HVMIRQ_callback_vector
> +            HVMIRQ_callback_gsi = HVM_PARAM_CALLBACK_TYPE_GSI,
> +            HVMIRQ_callback_pci_intx = HVM_PARAM_CALLBACK_TYPE_PCI_INTX,
> +            HVMIRQ_callback_vector = HVM_PARAM_CALLBACK_TYPE_VECTOR,
> +            /* Will change if we add more types. */
> +            HVMIRQ_callback_none = HVM_PARAM_CALLBACK_TYPE_NUM,
>          } callback_via_type;
>      };
>      union {
> diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
> index 1606185..5908f39 100644
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -386,7 +386,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_ioreq_server_state_t);
>   *                                 channel upcalls on the specified <vcpu>. If set,
>   *                                 this vector will be used in preference to the
>   *                                 domain global callback via (see
> - *                                 HVM_PARAM_CALLBACK_IRQ).
> + *                                 HVM_PARAM_CALLBACK_IRQ and
> + *                                 HVM_PARAM_CALLBACK_VECTOR).
>   */
>  #define HVMOP_set_evtchn_upcall_vector 23
>  struct xen_hvm_evtchn_upcall_vector {
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 73d4718..5c7fbc5 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -56,6 +56,21 @@
>   */
>  
>  /*
> + * In the future this may change.
> + */
> +#define HVM_PARAM_CALLBACK_TYPE_NUM      HVM_PARAM_CALLBACK_TYPE_VECTOR + 1
> +/*
> + * The val[63:56] convenience shift.
> + */
> +#define HVM_PARAM_CALLBACK_TYPE_SHIFT 56
> +
> +/*
> + * Wrapper around for HVM_PARAM_CALLBACK_TYPE_VECTOR.
> + */
> +#define HVM_PARAM_CALLBACK_VECTOR(x) \
> +            (((uint64_t)HVM_PARAM_CALLBACK_TYPE_VECTOR) << \
> +                        HVM_PARAM_CALLBACK_TYPE_SHIFT | (x))
> +/*
>   * These are not used by Xen. They are here for convenience of HVM-guest
>   * xenbus implementations.
>   */
> -- 
> 2.4.3
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ
  2016-02-29 20:39 [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ Konrad Rzeszutek Wilk
  2016-02-29 20:42 ` Konrad Rzeszutek Wilk
@ 2016-03-01 10:25 ` Jan Beulich
  2016-03-01 14:32   ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2016-03-01 10:25 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Andrew Cooper, boris.ostrovsky, Keir Fraser, xen-devel

>>> On 29.02.16 at 21:39, <konrad.wilk@oracle.com> wrote:
> @@ -50,11 +52,13 @@ struct hvm_irq {
>      /* Virtual interrupt and via-link for paravirtual platform driver. */
>      uint32_t callback_via_asserted;
>      union {
> +        /* These MUST match with HVM_PARAM_CALLBACK_IRQ types. */
>          enum {
> -            HVMIRQ_callback_none,
> -            HVMIRQ_callback_gsi,
> -            HVMIRQ_callback_pci_intx,
> -            HVMIRQ_callback_vector
> +            HVMIRQ_callback_gsi = HVM_PARAM_CALLBACK_TYPE_GSI,
> +            HVMIRQ_callback_pci_intx = HVM_PARAM_CALLBACK_TYPE_PCI_INTX,
> +            HVMIRQ_callback_vector = HVM_PARAM_CALLBACK_TYPE_VECTOR,
> +            /* Will change if we add more types. */
> +            HVMIRQ_callback_none = HVM_PARAM_CALLBACK_TYPE_NUM,
>          } callback_via_type;
>      };

I.e. a domain will now start in HVMIRQ_callback_gsi mode (due to
HVM_PARAM_CALLBACK_TYPE_GSI = 0) instead of
HVMIRQ_callback_none? That can't be right.

> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -56,6 +56,21 @@
>   */
>  
>  /*
> + * In the future this may change.
> + */
> +#define HVM_PARAM_CALLBACK_TYPE_NUM      HVM_PARAM_CALLBACK_TYPE_VECTOR + 1

Missing parentheses.

> +/*
> + * The val[63:56] convenience shift.
> + */
> +#define HVM_PARAM_CALLBACK_TYPE_SHIFT 56
> +
> +/*
> + * Wrapper around for HVM_PARAM_CALLBACK_TYPE_VECTOR.

Looks like either "around" or "for" wants to be dropped. I also
think this would better live right next to that constant's definition.

Also note that all comments you add are single line ones, and
hence don't conform to our coding style (yes, there are other
[bad] examples of such in this file).

Jan


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ
  2016-03-01 10:25 ` Jan Beulich
@ 2016-03-01 14:32   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-01 14:32 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, boris.ostrovsky, Keir Fraser, xen-devel

On Tue, Mar 01, 2016 at 03:25:55AM -0700, Jan Beulich wrote:
> >>> On 29.02.16 at 21:39, <konrad.wilk@oracle.com> wrote:
> > @@ -50,11 +52,13 @@ struct hvm_irq {
> >      /* Virtual interrupt and via-link for paravirtual platform driver. */
> >      uint32_t callback_via_asserted;
> >      union {
> > +        /* These MUST match with HVM_PARAM_CALLBACK_IRQ types. */
> >          enum {
> > -            HVMIRQ_callback_none,
> > -            HVMIRQ_callback_gsi,
> > -            HVMIRQ_callback_pci_intx,
> > -            HVMIRQ_callback_vector
> > +            HVMIRQ_callback_gsi = HVM_PARAM_CALLBACK_TYPE_GSI,
> > +            HVMIRQ_callback_pci_intx = HVM_PARAM_CALLBACK_TYPE_PCI_INTX,
> > +            HVMIRQ_callback_vector = HVM_PARAM_CALLBACK_TYPE_VECTOR,
> > +            /* Will change if we add more types. */
> > +            HVMIRQ_callback_none = HVM_PARAM_CALLBACK_TYPE_NUM,
> >          } callback_via_type;
> >      };
> 
> I.e. a domain will now start in HVMIRQ_callback_gsi mode (due to
> HVM_PARAM_CALLBACK_TYPE_GSI = 0) instead of
> HVMIRQ_callback_none? That can't be right.

Argh. I was looking for all the cases for callback_via_type being set but
of course missed the most obvious one!

> 
> > --- a/xen/include/public/hvm/params.h
> > +++ b/xen/include/public/hvm/params.h
> > @@ -56,6 +56,21 @@
> >   */
> >  
> >  /*
> > + * In the future this may change.
> > + */
> > +#define HVM_PARAM_CALLBACK_TYPE_NUM      HVM_PARAM_CALLBACK_TYPE_VECTOR + 1
> 
> Missing parentheses.
> 
> > +/*
> > + * The val[63:56] convenience shift.
> > + */
> > +#define HVM_PARAM_CALLBACK_TYPE_SHIFT 56
> > +
> > +/*
> > + * Wrapper around for HVM_PARAM_CALLBACK_TYPE_VECTOR.
> 
> Looks like either "around" or "for" wants to be dropped. I also
> think this would better live right next to that constant's definition.

OK.
> 
> Also note that all comments you add are single line ones, and
> hence don't conform to our coding style (yes, there are other
> [bad] examples of such in this file).

/me nods. Will make it conform to the single line one type comment.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-03-01 14:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-29 20:39 [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ Konrad Rzeszutek Wilk
2016-02-29 20:42 ` Konrad Rzeszutek Wilk
2016-03-01 10:25 ` Jan Beulich
2016-03-01 14:32   ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).