All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@ozlabs.org" <linuxppc-dev@ozlabs.org>,
	Ingo Molnar <mingo@redhat.com>, Paul Mackerras <paulus@samba.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Yinghai Lu <yinghai@kernel.org>
Subject: Re: [PATCH] irq: move some interrupt arch_* functions into struct irq_chip.
Date: Wed, 10 Mar 2010 09:42:08 -0800	[thread overview]
Message-ID: <m1iq94gjkf.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1268225473.11737.69196.camel@zakaz.uk.xensource.com> (Ian Campbell's message of "Wed\, 10 Mar 2010 12\:51\:13 +0000")

Ian Campbell <Ian.Campbell@citrix.com> writes:

> On Wed, 2010-03-10 at 12:06 +0000, Yinghai Lu wrote:
>> On Wed, Mar 10, 2010 at 2:55 AM,  <ijc@hellion.org.uk> wrote:
>> > From: Ian Campbell <ian.campbell@citrix.com>
>> >
>> > Move arch_init_copy_chip_data and arch_free_chip_data into function
>> > pointers in struct irq_chip since they operate on irq_desc->chip_data.
>> >
>> > arch_init_chip_data cannot be moved into struct irq_chip at this time
>> > because irq_desc->chip is not known at the time the irq_desc is
>> > setup. For now rename arch_init_chip_data to arch_init_irq_desc (for
>> > PowerPC, the only other user, whose usage better matches the new name)
>> > and on x86 convert arch_init_chip_data to ioapic_init_chip_data and
>> > call this whenever the IO APIC code allocates a new IRQ.
>> >
>> > I've retained the chip_data behaviour for uv_irq although it isn't
>> > clear to me if these interrupt types support migration or how closely
>> > related to the APIC modes they really are. If it weren't for this the
>> > ioapic_{init,copy,free}_chip_data functions could be static to
>> > io_apic.c.
>> >
>> > I've tested by booting on a 64 bit system, but it's not clear to me
>> > what actions I need to take to actually exercise some of these code
>> > paths.
>> >
>> 
>> can you just add another pointer field in irq_desc?
>> 
>> some kind of *irq_info etc.
>
> I think I don't understand what you are suggesting.

YH another field doesn't make much sense.  Xen is a bizarre subarch
with an incompatible irq model.  Xen simply needs the ability to
handle the entire lifetime of an irq_chip.

All we need between the Xen and the rest of x86 is a convention
so that we never manage the same irqs.   At least for domU we are
in an either/or situation so I don't see even that being a problem.

> There is already a pointer for irq_chip specific use i.e.
> irq_desc->chip_data. This patchset is just about ensuring that the field
> really is available to any chip implementation rather than just assuming
> it is always used for the acpi chip types (on x86 at least).

Ian Xen in this sense is simply not x86.  irq_cfg is not acpi or ioapic
or anything but x86 specific.  It has everything to do with having a per
cpu vector table of 256 entries and architecturally receiving a vector
number when an interrupt is fired.

It totally makes sense for Xen to do something different because
architecturally it has a completely different irq subsystem.

At the same time let's not pretend that the reason for this is anything
except that Xen has a completely different notion of interrupt delivery
than the rest of x86 and so it is it's own bizarre subarch.

This is not a case where you simply need a driver because something is
a bit different but fits into the existing model.

So the best solution here seems to be a parameter that we pass into
irq_to_desc_alloc_node that does what is needed.  The second best
would be to have arch_init_chip_data to call something like
platfrom_init_chip_data().    But I think we can avoid that in
this case.

Eric

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Yinghai Lu <yinghai@kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Jeremy Fitzhardinge <jeremy@goop.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	"x86\@kernel.org" <x86@kernel.org>,
	"linuxppc-dev\@ozlabs.org" <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] irq: move some interrupt arch_* functions into struct irq_chip.
Date: Wed, 10 Mar 2010 09:42:08 -0800	[thread overview]
Message-ID: <m1iq94gjkf.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1268225473.11737.69196.camel@zakaz.uk.xensource.com> (Ian Campbell's message of "Wed\, 10 Mar 2010 12\:51\:13 +0000")

Ian Campbell <Ian.Campbell@citrix.com> writes:

> On Wed, 2010-03-10 at 12:06 +0000, Yinghai Lu wrote:
>> On Wed, Mar 10, 2010 at 2:55 AM,  <ijc@hellion.org.uk> wrote:
>> > From: Ian Campbell <ian.campbell@citrix.com>
>> >
>> > Move arch_init_copy_chip_data and arch_free_chip_data into function
>> > pointers in struct irq_chip since they operate on irq_desc->chip_data.
>> >
>> > arch_init_chip_data cannot be moved into struct irq_chip at this time
>> > because irq_desc->chip is not known at the time the irq_desc is
>> > setup. For now rename arch_init_chip_data to arch_init_irq_desc (for
>> > PowerPC, the only other user, whose usage better matches the new name)
>> > and on x86 convert arch_init_chip_data to ioapic_init_chip_data and
>> > call this whenever the IO APIC code allocates a new IRQ.
>> >
>> > I've retained the chip_data behaviour for uv_irq although it isn't
>> > clear to me if these interrupt types support migration or how closely
>> > related to the APIC modes they really are. If it weren't for this the
>> > ioapic_{init,copy,free}_chip_data functions could be static to
>> > io_apic.c.
>> >
>> > I've tested by booting on a 64 bit system, but it's not clear to me
>> > what actions I need to take to actually exercise some of these code
>> > paths.
>> >
>> 
>> can you just add another pointer field in irq_desc?
>> 
>> some kind of *irq_info etc.
>
> I think I don't understand what you are suggesting.

YH another field doesn't make much sense.  Xen is a bizarre subarch
with an incompatible irq model.  Xen simply needs the ability to
handle the entire lifetime of an irq_chip.

All we need between the Xen and the rest of x86 is a convention
so that we never manage the same irqs.   At least for domU we are
in an either/or situation so I don't see even that being a problem.

> There is already a pointer for irq_chip specific use i.e.
> irq_desc->chip_data. This patchset is just about ensuring that the field
> really is available to any chip implementation rather than just assuming
> it is always used for the acpi chip types (on x86 at least).

Ian Xen in this sense is simply not x86.  irq_cfg is not acpi or ioapic
or anything but x86 specific.  It has everything to do with having a per
cpu vector table of 256 entries and architecturally receiving a vector
number when an interrupt is fired.

It totally makes sense for Xen to do something different because
architecturally it has a completely different irq subsystem.

At the same time let's not pretend that the reason for this is anything
except that Xen has a completely different notion of interrupt delivery
than the rest of x86 and so it is it's own bizarre subarch.

This is not a case where you simply need a driver because something is
a bit different but fits into the existing model.

So the best solution here seems to be a parameter that we pass into
irq_to_desc_alloc_node that does what is needed.  The second best
would be to have arch_init_chip_data to call something like
platfrom_init_chip_data().    But I think we can avoid that in
this case.

Eric


  reply	other threads:[~2010-03-10 17:42 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-03  3:31 x86: fix race in create_irq_nr on irq_desc Brandon Philips
2010-02-03 10:20 ` Yinghai Lu
2010-02-03 17:42   ` Brandon Philips
2010-02-03 19:31     ` Yinghai Lu
2010-02-04  3:17       ` Brandon Philips
2010-02-05  8:45     ` [PATCH] x86: keep chip_data in create_irq_nr Yinghai Lu
2010-02-05 21:05       ` Brandon Philips
2010-02-05 21:42         ` H. Peter Anvin
2010-02-05 21:09       ` [PATCH] x86: keep chip_data in create_irq_nr and destroy_irq Brandon Philips
2010-02-05 22:44         ` Yinghai Lu
2010-02-05 22:55           ` Brandon Philips
2010-02-06  0:06             ` Yinghai Lu
2010-02-06  0:18               ` [PATCH v2] " Brandon Philips
2010-02-06  6:42                 ` [PATCH v3] " Brandon Philips
2010-02-06  7:16                   ` Yinghai Lu
2010-02-06 20:05                     ` Brandon Philips
2010-02-07 21:02                     ` [PATCH v4] " Brandon Philips
2010-02-19  6:06                       ` [tip:x86/urgent] x86, irq: Keep " tip-bot for Brandon Philips
2010-02-26 10:26                       ` [tip:x86/irq] x86: apic: Fix mismerge, add arch_probe_nr_irqs() again tip-bot for Ingo Molnar
2010-02-26 18:19                         ` Yinghai Lu
2010-02-27  9:10                           ` Ingo Molnar
2010-02-27  9:37                             ` Eric W. Biederman
2010-02-27  9:53                               ` Ingo Molnar
2010-02-27 10:12                                 ` Eric W. Biederman
2010-03-01 11:22                           ` Ian Campbell
2010-03-01 18:34                             ` Eric W. Biederman
2010-03-01 21:44                               ` Ian Campbell
2010-03-01 21:58                                 ` Eric W. Biederman
2010-03-02  8:31                                   ` Thomas Gleixner
2010-03-10 10:55                                   ` Ian Campbell
2010-03-10 10:55                                     ` [PATCH] x86: namespace some I/O APIC related structures and functions ijc
2010-03-10 17:07                                       ` Eric W. Biederman
2010-03-10 10:55                                     ` [PATCH] irq: move some interrupt arch_* functions into struct irq_chip ijc
2010-03-10 10:55                                       ` ijc
2010-03-10 11:00                                       ` Ian Campbell
2010-03-10 11:00                                         ` Ian Campbell
2010-03-10 17:18                                         ` Eric W. Biederman
2010-03-10 17:18                                           ` Eric W. Biederman
2010-03-10 17:41                                           ` Ian Campbell
2010-03-10 17:41                                             ` Ian Campbell
2010-03-10 18:11                                             ` Eric W. Biederman
2010-03-10 18:11                                               ` Eric W. Biederman
2010-03-10 12:06                                       ` Yinghai Lu
2010-03-10 12:06                                         ` Yinghai Lu
2010-03-10 12:51                                         ` Ian Campbell
2010-03-10 12:51                                           ` Ian Campbell
2010-03-10 17:42                                           ` Eric W. Biederman [this message]
2010-03-10 17:42                                             ` Eric W. Biederman
2010-03-10 17:50                                             ` Ian Campbell
2010-03-10 17:50                                               ` Ian Campbell
2010-03-10 18:15                                               ` Eric W. Biederman
2010-03-10 18:15                                                 ` Eric W. Biederman
2010-03-10 18:28                                                 ` Ian Campbell
2010-03-10 18:28                                                   ` Ian Campbell
2010-03-10 18:27                                             ` Jeremy Fitzhardinge
2010-03-10 18:27                                               ` Jeremy Fitzhardinge
2010-03-10 18:59                                           ` Yinghai Lu
2010-03-10 18:59                                             ` Yinghai Lu
2010-03-10 19:15                                             ` Eric W. Biederman
2010-03-10 19:15                                               ` Eric W. Biederman
2010-03-10 22:07                                       ` Michael Ellerman
2010-03-10 22:07                                         ` Michael Ellerman
2010-03-10 10:55                                     ` [PATCH] x86: irq_desc->chip_data is always correct whether or not SPARSE_IRQ is enabled ijc
2010-03-01 22:01                                 ` [tip:x86/irq] x86: apic: Fix mismerge, add arch_probe_nr_irqs() again Jeremy Fitzhardinge
2010-02-27 12:57                       ` [tip:x86/apic] " tip-bot for Ingo Molnar
2010-02-03 10:32 ` x86: fix race in create_irq_nr on irq_desc Yinghai Lu
  -- strict thread matches above, loose matches on Subject: below --
2010-03-12  9:44 [GITPULL+PATCH 0/2] irq: move some interrupt arch_* functions into struct irq_chip Ian Campbell
2010-03-12  9:45 ` [PATCH] " Ian Campbell
2010-03-12  9:45   ` Ian Campbell
2010-03-12 19:26   ` Yinghai Lu
2010-03-12 19:26     ` Yinghai Lu
2010-03-13  0:29     ` Eric W. Biederman
2010-03-13  0:29       ` Eric W. Biederman
2010-03-16  8:50       ` Ian Campbell
2010-03-16  8:50         ` Ian Campbell
2010-03-16  9:18         ` Eric W. Biederman
2010-03-16  9:18           ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1iq94gjkf.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.