[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ



Hi Stefano,

On 17/03/14 19:06, Stefano Stabellini wrote:
On Tue, 11 Mar 2014, Jan Beulich wrote:
On 11.03.14 at 16:16, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
Hello Jan,

On 02/24/2014 02:48 PM, Julien Grall wrote:
On 02/24/2014 02:32 PM, Jan Beulich wrote:
On 24.02.14 at 15:08, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
(Adding Jan for x86 part).

On 02/20/2014 09:29 PM, Julien Grall wrote:
Hi Ian,

On 02/19/2014 11:55 AM, Ian Campbell wrote:
On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote:
On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
interrupt.

Mention here that you are therefore creating a linked list of actions
for each interrupt.

If you use xen/list.h for this then you get a load of helpers and
iterators which would save you open coding them.

After thinking, using xen/list.h won't really remove open code, except
removing "action_ptr" in release_dt_irq.

Calling release_dt_irq to an IRQ with multiple action shouldn't be
called often. Therefore, having both prev and next is a waste of space.

Jan, as it's common code, do you have any thoughts?

In fact I'm not convinced this action chaining is correct in the first
place, as mentioned by Ian too (considering the potential sharing
between hypervisor and guest). Furthermore, if this is really just
about IOMMU handlers, why can't the SMMU code register a single
action and disambiguate by the dev_id argument passed to the
handler?

The patch #3 of this serie protects the IRQ to be shared with the domain.

I should have remove "eg ARM SMMU" in the description. ARM SMMU is not
the only the case, we don't know in advance if the IRQ will be shared
(except browsing the DT and checking if this IRQ was used by another
devices...). We may have the same thing with other devices.

The logic is painful to handle internally in ARM SMMU driver while we
can handle it generically. No need to duplicate the code when a new
driver will have the same problem.

I haven't heard any answer from you. Shall I take as a "go"?

I'm sorry, this got lost between other stuff. Honestly I'm still not
convinced generic multi-action IRQ support is indeed useful.

I agree.
In general if an IRQ is shared among multiple devices, it is likely to
go to Dom0 and have a single action from Xen point of view.
An IRQ shared between Xen and a guest is a very bad idea.

I guess you agree with Jan, rigth? If so, I think you misunderstood the goal of this patch. This patch does *NOT* add support for IRQ sharing between domains and Xen (patch #3 is preventing that).

Some devices are describing a same interrupt twice in the device tree and the action is not the same to accomplish.

For instance for the SMMU on midway, the device tree bindings is:

              smmu_sata: smmu@9,20180000 {
                      compatible = "arm,mmu-400";
                      reg = <0x9 0x20180000 0x10000>;
                      mmu-masters = <&sata 0 1 2 3 4 5 6 7 8 9>;
                      #global-interrupts = <1>;
                      interrupts = <0 114 4 0 114 4>;
                      calxeda,smmu-secure-config-access;
                      arm,smmu-isolate-devices;
             };

As you can see the same interrupts is used twice: the first one is used for the global interrupt and the second one as context interrupt. For the latter, each time a new context bank is created (e.g a device is passthrough to IOMMU), we need to register another handler.

Now we have 2 solutions to implement it:
1) Implement it directly in the SMMU drivers => We are assuming we wouldn't this situation on another IOMMU drivers
        2) Implement it generically

The former one is complicated to implement, because it's not fixed if the IRQ will be described twice or not. We can take advantage of the generic code for this purpose.

Regards,

--
Julien Grall

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.