[PATCH v3] increase size of irq from uint8_t to uint32_t

[PATCH v3] increase size of irq from uint8_t to uint32_t

[Iurii Konovalenko]

From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>

Changes in v3:
    - Julien Grall's comment added to commit message

Changes in v2:
    - Commit message edited
    - Signed-off is set

Iurii Konovalenko (1):
  arm: irq: increase size of irq from uint8_t to uint32_t

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

-- 
1.9.1

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

[Iurii Konovalenko]

From: Iurii Konovalenko <iurii.konovalenko@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.konovalenko@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

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

[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.konovalenko@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.

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

[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.konovalenko@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

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

[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.konovalenko@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.

Re: [PATCH v3] increase size of irq from uint8_t to uint32_t

[Julien Grall]

Hi Iurii,

On 08/04/2015 16:14, Iurii Konovalenko wrote:
> From: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
>
> Changes in v3:
>      - Julien Grall's comment added to commit message
>
> Changes in v2:
>      - Commit message edited
>      - Signed-off is set
>
> Iurii Konovalenko (1):
>    arm: irq: increase size of irq from uint8_t to uint32_t

The platform device passthrough series has been pushed a month ago. Do 
you have any plan to resent this series before Xen 4.6?

Regards,

-- 
Julien Grall