Re: [Xen-devel] [PATCH v3] arm: irq: increase size of irq from uint8_t to uint32_t

2015-04-15 Thread Ian Campbell
On Wed, 2015-04-15 at 14:00 +0100, Julien Grall wrote:
 Hi Ian,
 
 On 15/04/15 13:10, Ian Campbell wrote:
  Switching Julien to his Citrix address which should probably be used in
  the future.
  
  On Wed, 2015-04-08 at 17:14 +0300, Iurii Konovalenko wrote:
  From: Iurii Konovalenko iurii.konovale...@globallogic.com
 
  Changes are dedicated to XEN_DOMCTL_irq_permission and
  IRQ pssthrough API functions.
 
  PHYSDEV_* operations already using 32 bits type but signed one.
 
  Although, PHYSDEV_* operations are not yet used on ARM and LPIs support
  (which are using very high number) are not supported yet, we don't need
  to care about theses for now.
  
  I may have slightly lost track, but I think we decided in Julien's
  passthrough thread not to use most of these interfaces on ARM, or am I
  confused?
 
 Only XEN_DOMCTL_bind_pt_irq will be used for ARM passthrough.
 
 I asked Iurii to mention PHYSDEV_* because the prototype is already
 valid but use int rather than unsigned int.
 
  If I'm correct then I think we can avoid messing with many of other
  ones, for example the ISA IRQ one doesn't need changing, does it? (ISA
  only had 16 IRQs IIRC...)
 
 Only the newly introduce function xc_domain_bind_pt_spi_irq will be used
 for ARM. I guess we can avoid to modify xc_domain_bind_pt*.

IMHO the only reason to modify the existing xc_domain_bind_pt* would be
to make the hypercall and libxc interfaces consistent, but in that case
the commit message should talk about that and not so much about ARM etc.

Actually as it is 2/3 of the commit message talks about something
unrelated which isn't changing, and the only other bit doesn't really
explain what or why, just that it is limited to certain interfaces.

So perhaps a clearer and more precise commit message might be all which
is needed to be changed here.

  I think it would be best if whichever bits of this are still relevant
  were folded into Julien's '[PATCH v5 p2 04/19] xen/arm: Implement
  hypercall DOMCTL_{,un}bind_pt_pirq' or at least presented as a followup
  to it.
  
  diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
  index 8803ab2..65fb866 100644
  --- a/xen/include/public/domctl.h
  +++ b/xen/include/public/domctl.h
  @@ -400,7 +400,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_setdebugging_t);
   
   /* XEN_DOMCTL_irq_permission */
   struct xen_domctl_irq_permission {
  -uint8_t pirq;
  +uint32_t pirq;
   uint8_t allow_access;/* flag to specify enable/disable of IRQ 
  access */
  
  I think we weren't going to end up using this one either, but again I
  might not be remembering correctly.
 
 Even though we won't support it on ARM for now, I think it's good to
 keep the interface consistent.
 
 It would avoid us to forget the problem when this will be support it later.

OK, but in that case the cc list needs to include the other hypervisor
maintainers and not just the ARM ones.

And in fact this whole patch needs to go to the tools maintainers
really, not so much to the ARM maintainers. get_maintainers.pl would
probably have helped.

Ian.


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


Re: [Xen-devel] [PATCH v3] arm: irq: increase size of irq from uint8_t to uint32_t

2015-04-15 Thread Ian Campbell
Switching Julien to his Citrix address which should probably be used in
the future.

On Wed, 2015-04-08 at 17:14 +0300, Iurii Konovalenko wrote:
 From: Iurii Konovalenko iurii.konovale...@globallogic.com
 
 Changes are dedicated to XEN_DOMCTL_irq_permission and
 IRQ pssthrough API functions.
 
 PHYSDEV_* operations already using 32 bits type but signed one.
 
 Although, PHYSDEV_* operations are not yet used on ARM and LPIs support
 (which are using very high number) are not supported yet, we don't need
 to care about theses for now.

I may have slightly lost track, but I think we decided in Julien's
passthrough thread not to use most of these interfaces on ARM, or am I
confused?

If I'm correct then I think we can avoid messing with many of other
ones, for example the ISA IRQ one doesn't need changing, does it? (ISA
only had 16 IRQs IIRC...)

I think it would be best if whichever bits of this are still relevant
were folded into Julien's '[PATCH v5 p2 04/19] xen/arm: Implement
hypercall DOMCTL_{,un}bind_pt_pirq' or at least presented as a followup
to it.

 diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
 index 8803ab2..65fb866 100644
 --- a/xen/include/public/domctl.h
 +++ b/xen/include/public/domctl.h
 @@ -400,7 +400,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_setdebugging_t);
  
  /* XEN_DOMCTL_irq_permission */
  struct xen_domctl_irq_permission {
 -uint8_t pirq;
 +uint32_t pirq;
  uint8_t allow_access;/* flag to specify enable/disable of IRQ access 
 */

I think we weren't going to end up using this one either, but again I
might not be remembering correctly.

Ian.


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


Re: [Xen-devel] [PATCH v3] arm: irq: increase size of irq from uint8_t to uint32_t

2015-04-15 Thread Julien Grall
Hi Ian,

On 15/04/15 13:10, Ian Campbell wrote:
 Switching Julien to his Citrix address which should probably be used in
 the future.
 
 On Wed, 2015-04-08 at 17:14 +0300, Iurii Konovalenko wrote:
 From: Iurii Konovalenko iurii.konovale...@globallogic.com

 Changes are dedicated to XEN_DOMCTL_irq_permission and
 IRQ pssthrough API functions.

 PHYSDEV_* operations already using 32 bits type but signed one.

 Although, PHYSDEV_* operations are not yet used on ARM and LPIs support
 (which are using very high number) are not supported yet, we don't need
 to care about theses for now.
 
 I may have slightly lost track, but I think we decided in Julien's
 passthrough thread not to use most of these interfaces on ARM, or am I
 confused?

Only XEN_DOMCTL_bind_pt_irq will be used for ARM passthrough.

I asked Iurii to mention PHYSDEV_* because the prototype is already
valid but use int rather than unsigned int.

 If I'm correct then I think we can avoid messing with many of other
 ones, for example the ISA IRQ one doesn't need changing, does it? (ISA
 only had 16 IRQs IIRC...)

Only the newly introduce function xc_domain_bind_pt_spi_irq will be used
for ARM. I guess we can avoid to modify xc_domain_bind_pt*.

 I think it would be best if whichever bits of this are still relevant
 were folded into Julien's '[PATCH v5 p2 04/19] xen/arm: Implement
 hypercall DOMCTL_{,un}bind_pt_pirq' or at least presented as a followup
 to it.
 
 diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
 index 8803ab2..65fb866 100644
 --- a/xen/include/public/domctl.h
 +++ b/xen/include/public/domctl.h
 @@ -400,7 +400,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_setdebugging_t);
  
  /* XEN_DOMCTL_irq_permission */
  struct xen_domctl_irq_permission {
 -uint8_t pirq;
 +uint32_t pirq;
  uint8_t allow_access;/* flag to specify enable/disable of IRQ 
 access */
 
 I think we weren't going to end up using this one either, but again I
 might not be remembering correctly.

Even though we won't support it on ARM for now, I think it's good to
keep the interface consistent.

It would avoid us to forget the problem when this will be support it later.

Regards,

-- 
Julien Grall

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


[Xen-devel] [PATCH v3] arm: irq: increase size of irq from uint8_t to uint32_t

2015-04-08 Thread Iurii Konovalenko
From: Iurii Konovalenko iurii.konovale...@globallogic.com

Changes are dedicated to XEN_DOMCTL_irq_permission and
IRQ pssthrough API functions.

PHYSDEV_* operations already using 32 bits type but signed one.

Although, PHYSDEV_* operations are not yet used on ARM and LPIs support
(which are using very high number) are not supported yet, we don't need
to care about theses for now.

Signed-off-by: Iurii Konovalenko iurii.konovale...@globallogic.com
---
 tools/libxc/include/xenctrl.h | 10 +-
 tools/libxc/xc_domain.c   | 10 +-
 xen/include/public/domctl.h   |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 44c7ac0..c08c4da 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1401,7 +1401,7 @@ int xc_domain_ioport_permission(xc_interface *xch,
 
 int xc_domain_irq_permission(xc_interface *xch,
  uint32_t domid,
- uint8_t pirq,
+ uint32_t pirq,
  uint8_t allow_access);
 
 int xc_domain_iomem_permission(xc_interface *xch,
@@ -2087,7 +2087,7 @@ int xc_domain_unbind_msi_irq(xc_interface *xch,
 
 int xc_domain_bind_pt_irq(xc_interface *xch,
   uint32_t domid,
-  uint8_t machine_irq,
+  uint32_t machine_irq,
   uint8_t irq_type,
   uint8_t bus,
   uint8_t device,
@@ -2096,7 +2096,7 @@ int xc_domain_bind_pt_irq(xc_interface *xch,
 
 int xc_domain_unbind_pt_irq(xc_interface *xch,
   uint32_t domid,
-  uint8_t machine_irq,
+  uint32_t machine_irq,
   uint8_t irq_type,
   uint8_t bus,
   uint8_t device,
@@ -2105,14 +2105,14 @@ int xc_domain_unbind_pt_irq(xc_interface *xch,
 
 int xc_domain_bind_pt_pci_irq(xc_interface *xch,
   uint32_t domid,
-  uint8_t machine_irq,
+  uint32_t machine_irq,
   uint8_t bus,
   uint8_t device,
   uint8_t intx);
 
 int xc_domain_bind_pt_isa_irq(xc_interface *xch,
   uint32_t domid,
-  uint8_t machine_irq);
+  uint32_t machine_irq);
 
 int xc_domain_set_machine_address_size(xc_interface *xch,
   uint32_t domid,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 95e3098..3132d19 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1271,7 +1271,7 @@ int xc_vcpu_setcontext(xc_interface *xch,
 
 int xc_domain_irq_permission(xc_interface *xch,
  uint32_t domid,
- uint8_t pirq,
+ uint32_t pirq,
  uint8_t allow_access)
 {
 DECLARE_DOMCTL;
@@ -1775,7 +1775,7 @@ int xc_domain_unbind_msi_irq(
 int xc_domain_bind_pt_irq(
 xc_interface *xch,
 uint32_t domid,
-uint8_t machine_irq,
+uint32_t machine_irq,
 uint8_t irq_type,
 uint8_t bus,
 uint8_t device,
@@ -1816,7 +1816,7 @@ int xc_domain_bind_pt_irq(
 int xc_domain_unbind_pt_irq(
 xc_interface *xch,
 uint32_t domid,
-uint8_t machine_irq,
+uint32_t machine_irq,
 uint8_t irq_type,
 uint8_t bus,
 uint8_t device,
@@ -1857,7 +1857,7 @@ int xc_domain_unbind_pt_irq(
 int xc_domain_bind_pt_pci_irq(
 xc_interface *xch,
 uint32_t domid,
-uint8_t machine_irq,
+uint32_t machine_irq,
 uint8_t bus,
 uint8_t device,
 uint8_t intx)
@@ -1870,7 +1870,7 @@ int xc_domain_bind_pt_pci_irq(
 int xc_domain_bind_pt_isa_irq(
 xc_interface *xch,
 uint32_t domid,
-uint8_t machine_irq)
+uint32_t machine_irq)
 {
 
 return (xc_domain_bind_pt_irq(xch, domid, machine_irq,
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 8803ab2..65fb866 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -400,7 +400,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_setdebugging_t);
 
 /* XEN_DOMCTL_irq_permission */
 struct xen_domctl_irq_permission {
-uint8_t pirq;
+uint32_t pirq;
 uint8_t allow_access;/* flag to specify enable/disable of IRQ access */
 };
 typedef struct xen_domctl_irq_permission xen_domctl_irq_permission_t;
-- 
1.9.1


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