Re: Size of irq field

[Julien Grall <julien.grall@citrix.com>]

On 03/04/2015 14:40, Iurii Konovalenko wrote:
> Hi, Ian! Hi, Julien!

Hi Iurii,

> Thank you for your replies.
>
> On Thu, Apr 2, 2015 at 6:19 PM, Ian Campbell <ian.campbell@citrix.com
> <mailto:ian.campbell@citrix.com>> wrote:
>  > Please can you give some example of this?
> On Thu, Apr 2, 2015 at 6:34 PM, Julien Grall <julien.grall@citrix.com
> <mailto:julien.grall@citrix.com>> wrote:
>  > Can you give an example of files/structures using uint8_t for IRQ?
>
> As Julien wrote, it is related to irq_permission. I am not sure if it is
> supported on ARM, but we use it, and this function fails during domain
> creation, because IRQ number is truncated and permission operation is
> applied for incorrect IRQ, so domain creation fails. So, do we need to
> change logic and do not check IRQ permissions?

Where do you need to check the IRQ permission?

Currently (with my DT device passthrough series) only DOM0 is able to 
manage the IRQ (i.e bind/unbind) and we don't let the guest to do it. So 
I don't think it's necessary to to call xc_domain_irq_permission for guest.

The DOMCTL is buggy because it relies on vIRQ == IRQ as many place in 
Xen. Although it would work now because, we chose to map 1:1 the IRQ in 
the guest.

> Also, as we plan to use passtrough, we there are such places:

> In tools/libxc/include/xenctrl.h:
> int xc_domain_bind_pt_irq(xc_interface *xch,
>                            uint32_t domid,
> *uint8_t* machine_irq,
>                            uint8_t irq_type,
>                            uint8_t bus,
>                            uint8_t device,
>                            uint8_t intx,
>                            uint8_t isa_irq);
>
> int xc_domain_unbind_pt_irq(xc_interface *xch,
>                            uint32_t domid,
> *uint8_t* machine_irq,
>                            uint8_t irq_type,
>                            uint8_t bus,
>                            uint8_t device,
>                            uint8_t intx,
>                            uint8_t isa_irq);
>
> int xc_domain_bind_pt_pci_irq(xc_interface *xch,
>                                uint32_t domid,
> *uint8_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);
>
> And theirs implementation in tools/libxc/xc_domain.c

Whoops. I didn't spot those one thanks.

>
> On Thu, Apr 2, 2015 at 6:19 PM, Ian Campbell <ian.campbell@citrix.com
> <mailto:ian.campbell@citrix.com>> wrote:
>  > We should certainly fix anywhere which is not using a large enough data
>  > type.
> On Thu, Apr 2, 2015 at 6:34 PM, Julien Grall <julien.grall@citrix.com
> <mailto:julien.grall@citrix.com>> wrote:
>  > Ideally the IRQ should use uint32_t. This is allow us to support LPIs
> (IRQ
>  > number start a 8192 up to a very high number).
>
> I have patch that increase size to uint32_t for cases, I described
> previously. Can I push it for review?

Yes please.

Regards,

-- 
Julien Grall