diff mbox

[V4,29/42] x86, irq: introduce two helper functions to support irqdomain map operation

Message ID 1402302011-23642-30-git-send-email-jiang.liu@linux.intel.com
State Not Applicable
Headers show

Commit Message

Jiang Liu June 9, 2014, 8:19 a.m. UTC
Currently there are multiple entries to program IOAPIC pins, such as
io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
setup_IO_APIC_irq_extra() etc.

This patch introduces two functions to help consolidate the code to
program IOAPIC pins. Function mp_set_pin_attr() is used to optionally
set trigger, polarity and NUMA node property for an IOAPIC pin.
If mp_set_pin_attr() is not invoked for a pin, the default configuration
from BIOS will be used.

Function mp_irqdomain_map() is an common implementation of irqdomain map()
operation. It figures out attribures for pin and then actually programs
the IOAPIC pin. We hope this will be the only entrance for programming
IOAPIC pin.

And the flow will:
1) caller such as xxx_pci_irq_enable figures out pin attributes.
2) Invoke mp_set_pin_attr() to set attributes for a pin. If the pin has
   already bin programmed,  mp_set_pin_attr() will aslo detects attribute
   confictions.
3) Invoke mp_map_pin_to_irq()
3.1) If IRQ has already been assigned, return irq_find_mapping()
3.2) Else irq_create_mapping()
		->irq_domain_associate()
			->mp_irqdomain_map()
				->io_apic_setup_irq_pin()

So every pin will only programmed once by mp_irqdomain_map(), so we
could kill io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
setup_IO_APIC_irq_extra() etc.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/io_apic.h |    5 ++
 arch/x86/kernel/apic/io_apic.c |   99 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 1 deletion(-)

Comments

Mika Westerberg Aug. 21, 2014, 2:17 p.m. UTC | #1
On Mon, Jun 09, 2014 at 04:19:58PM +0800, Jiang Liu wrote:
> Currently there are multiple entries to program IOAPIC pins, such as
> io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
> setup_IO_APIC_irq_extra() etc.
> 
> This patch introduces two functions to help consolidate the code to
> program IOAPIC pins. Function mp_set_pin_attr() is used to optionally
> set trigger, polarity and NUMA node property for an IOAPIC pin.
> If mp_set_pin_attr() is not invoked for a pin, the default configuration
> from BIOS will be used.
> 
> Function mp_irqdomain_map() is an common implementation of irqdomain map()
> operation. It figures out attribures for pin and then actually programs
> the IOAPIC pin. We hope this will be the only entrance for programming
> IOAPIC pin.
> 
> And the flow will:
> 1) caller such as xxx_pci_irq_enable figures out pin attributes.
> 2) Invoke mp_set_pin_attr() to set attributes for a pin. If the pin has
>    already bin programmed,  mp_set_pin_attr() will aslo detects attribute
>    confictions.
> 3) Invoke mp_map_pin_to_irq()
> 3.1) If IRQ has already been assigned, return irq_find_mapping()
> 3.2) Else irq_create_mapping()
> 		->irq_domain_associate()
> 			->mp_irqdomain_map()
> 				->io_apic_setup_irq_pin()
> 
> So every pin will only programmed once by mp_irqdomain_map(), so we
> could kill io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
> setup_IO_APIC_irq_extra() etc.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  arch/x86/include/asm/io_apic.h |    5 ++
>  arch/x86/kernel/apic/io_apic.c |   99 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 103 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> index 3e4bea3a52b1..c53587868590 100644
> --- a/arch/x86/include/asm/io_apic.h
> +++ b/arch/x86/include/asm/io_apic.h
> @@ -173,7 +173,9 @@ enum ioapic_domain_type {
>  };
>  
>  struct device_node;
> +struct irq_domain;
>  struct irq_domain_ops;
> +
>  struct ioapic_domain_cfg {
>  	enum ioapic_domain_type		type;
>  	const struct irq_domain_ops	*ops;
> @@ -192,6 +194,9 @@ extern u32 mp_pin_to_gsi(int ioapic, int pin);
>  extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
>  extern void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  				      struct ioapic_domain_cfg *cfg);
> +extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
> +			    irq_hw_number_t hwirq);
> +extern int mp_set_gsi_attr(u32 gsi, int trigger, int polarity, int node);
>  extern void __init pre_init_apic_IRQ0(void);
>  
>  extern void mp_save_irq(struct mpc_intsrc *m);
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 9c5f70699a80..61aae90f7e23 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -87,6 +87,14 @@ static DEFINE_RAW_SPINLOCK(vector_lock);
>  static DEFINE_MUTEX(ioapic_mutex);
>  static unsigned int ioapic_dynirq_base;
>  
> +struct mp_pin_info {
> +	int trigger;
> +	int polarity;
> +	int node;
> +	int set;
> +	u32 count;
> +};
> +
>  static struct ioapic {
>  	/*
>  	 * # of IRQ routing registers
> @@ -102,6 +110,7 @@ static struct ioapic {
>  	struct mp_ioapic_gsi  gsi_config;
>  	struct ioapic_domain_cfg irqdomain_cfg;
>  	struct irq_domain *irqdomain;
> +	struct mp_pin_info *pin_info;
>  	DECLARE_BITMAP(pin_programmed, MP_MAX_IOAPIC_PIN + 1);
>  } ioapics[MAX_IO_APICS];
>  
> @@ -147,6 +156,11 @@ static inline int mp_init_irq_at_boot(int ioapic, int irq)
>  	return ioapic == 0 || (irq >= 0 && irq < nr_legacy_irqs());
>  }
>  
> +static inline struct mp_pin_info *mp_pin_info(int ioapic_idx, int pin)
> +{
> +	return ioapics[ioapic_idx].pin_info + pin;
> +}
> +
>  static inline struct irq_domain *mp_ioapic_irqdomain(int ioapic)
>  {
>  	return ioapics[ioapic].irqdomain;
> @@ -1006,6 +1020,7 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
>  {
>  	int irq;
>  	struct irq_domain *domain = mp_ioapic_irqdomain(ioapic);
> +	struct mp_pin_info *info = mp_pin_info(ioapic, pin);
>  
>  	/*
>  	 * Don't use irqdomain to manage ISA IRQs because there may be
> @@ -1034,6 +1049,13 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
>  	irq = irq_find_mapping(domain, pin);
>  	if (irq <= 0 && (flags & IOAPIC_MAP_ALLOC))
>  		irq = alloc_irq_from_domain(domain, gsi, pin);
> +
> +	if (flags & IOAPIC_MAP_ALLOC) {
> +		if (irq > 0)
> +			info->count++;
> +		else if (info->count == 0)
> +			info->set = 0;
> +	}
>  	mutex_unlock(&ioapic_mutex);
>  
>  	return irq > 0 ? irq : -1;
> @@ -2923,18 +2945,27 @@ out:
>  
>  static int mp_irqdomain_create(int ioapic)
>  {
> +	size_t size;
>  	int hwirqs = mp_ioapic_pin_count(ioapic);
>  	struct ioapic *ip = &ioapics[ioapic];
>  	struct ioapic_domain_cfg *cfg = &ip->irqdomain_cfg;
>  	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(ioapic);
>  
> +	size = sizeof(struct mp_pin_info) * mp_ioapic_pin_count(ioapic);
> +	ip->pin_info = kzalloc(size, GFP_KERNEL);
> +	if (!ip->pin_info)
> +		return -ENOMEM;
> +
>  	if (cfg->type == IOAPIC_DOMAIN_INVALID)
>  		return 0;
>  
>  	ip->irqdomain = irq_domain_add_linear(cfg->dev, hwirqs, cfg->ops,
>  					      (void *)(long)ioapic);
> -	if(!ip->irqdomain)
> +	if(!ip->irqdomain) {
> +		kfree(ip->pin_info);
> +		ip->pin_info = NULL;
>  		return -ENOMEM;
> +	}
>  
>  	if (cfg->type == IOAPIC_DOMAIN_LEGACY ||
>  	    cfg->type == IOAPIC_DOMAIN_STRICT)
> @@ -3898,6 +3929,72 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  	nr_ioapics++;
>  }
>  
> +int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
> +		     irq_hw_number_t hwirq)
> +{
> +	int ioapic = (int)(long)domain->host_data;
> +	struct mp_pin_info *info = mp_pin_info(ioapic, hwirq);
> +	struct io_apic_irq_attr attr;
> +
> +	/*
> +	 * Skip the timer IRQ if there's a quirk handler installed and if it
> +	 * returns 1:
> +	 */
> +	if (apic->multi_timer_check &&
> +	    apic->multi_timer_check(ioapic, virq))
> +		return 0;
> +
> +	/* Get default attribute if not set by caller yet */
> +	if (!info->set) {
> +		u32 gsi = mp_pin_to_gsi(ioapic, hwirq);
> +
> +		if (acpi_get_override_irq(gsi, &info->trigger,
> +					  &info->polarity) < 0) {

Sadly this seems to break LPSS ACPI enumerated devices.

Before this change /proc/interrupts say:

   0:         14          0          0          0   IO-APIC-edge      timer
   1:         10          0          0          0   IO-APIC-edge      i8042
   4:         79          0          0          0   IO-APIC-edge      serial
   5:         52          0          0          0   IO-APIC-fasteoi   mmc0
   6:          0          0          0          0   IO-APIC-fasteoi   dw_dmac
   7:          0          0          0          0   IO-APIC-fasteoi   INT3432:00, INT3433:00
   8:          1          0          0          0   IO-APIC-edge      rtc0
   9:        174          0          0          0   IO-APIC-fasteoi   acpi
  57:        476          0          0          0   PCI-MSI-edge      xhci_hcd
  58:         17          0          0          0   PCI-MSI-edge      snd_hda_intel

After the change it looks like this:

  0:         14          0          0          0   IO-APIC-edge      timer
  1:         10          0          0          0   IO-APIC-edge      i8042
  4:         64          0          0          0   IO-APIC-edge      serial
  5:          0          0          0          0   IO-APIC-edge      mmc0
  6:          0          0          0          0   IO-APIC-edge      dw_dmac
  7:          0          0          0          0   IO-APIC-edge      INT3432:00, INT3433:00
  8:          1          0          0          0   IO-APIC-edge      rtc0
  9:        173          0          0          0   IO-APIC-fasteoi   acpi
 41:        471          0          0          0   PCI-MSI-edge      xhci_hcd
 42:         17          0          0          0   PCI-MSI-edge      snd_hda_intel

Notice the interrupt triggering of IRQs 5, 6, and 7 has changed from level to
edge. I also see this printed on the console:

[    1.676685] Failed to set pin attr for GSI6
[    1.691708] Failed to set pin attr for GSI7
[    1.706750] Failed to set pin attr for GSI7
[    1.721768] Failed to set pin attr for GSI13
[    1.736765] Failed to set pin attr for GSI5
[    1.838342] Failed to set pin attr for GSI6

Any ideas how to get that fixed?

We used to handle this in drivers/acpi/resource.c:acpi_dev_get_irqresource() so
that only IRQ() and IRQNoFlags() ACPI resources resort calling
acpi_get_override_irq(). Now that doesn't help anymore.

> +			/*
> +			 * PCI interrupts are always polarity one level
> +			 * triggered.
> +			 */
> +			info->trigger = 1;
> +			info->polarity = 1;
> +		}
> +		info->node = NUMA_NO_NODE;
> +		info->set = 1;
> +	}
> +	set_io_apic_irq_attr(&attr, ioapic, hwirq, info->trigger,
> +			     info->polarity);
> +
> +	return io_apic_setup_irq_pin(virq, info->node, &attr);
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Aug. 21, 2014, 4:57 p.m. UTC | #2
On Thu, Aug 21, 2014 at 05:17:29PM +0300, Mika Westerberg wrote:
> On Mon, Jun 09, 2014 at 04:19:58PM +0800, Jiang Liu wrote:
> > Currently there are multiple entries to program IOAPIC pins, such as
> > io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
> > setup_IO_APIC_irq_extra() etc.
> > 
> > This patch introduces two functions to help consolidate the code to
> > program IOAPIC pins. Function mp_set_pin_attr() is used to optionally
> > set trigger, polarity and NUMA node property for an IOAPIC pin.
> > If mp_set_pin_attr() is not invoked for a pin, the default configuration
> > from BIOS will be used.
> > 
> > Function mp_irqdomain_map() is an common implementation of irqdomain map()
> > operation. It figures out attribures for pin and then actually programs
> > the IOAPIC pin. We hope this will be the only entrance for programming
> > IOAPIC pin.
> > 
> > And the flow will:
> > 1) caller such as xxx_pci_irq_enable figures out pin attributes.
> > 2) Invoke mp_set_pin_attr() to set attributes for a pin. If the pin has
> >    already bin programmed,  mp_set_pin_attr() will aslo detects attribute
> >    confictions.
> > 3) Invoke mp_map_pin_to_irq()
> > 3.1) If IRQ has already been assigned, return irq_find_mapping()
> > 3.2) Else irq_create_mapping()
> > 		->irq_domain_associate()
> > 			->mp_irqdomain_map()
> > 				->io_apic_setup_irq_pin()
> > 
> > So every pin will only programmed once by mp_irqdomain_map(), so we
> > could kill io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
> > setup_IO_APIC_irq_extra() etc.
> > 
> > Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> > ---
> >  arch/x86/include/asm/io_apic.h |    5 ++
> >  arch/x86/kernel/apic/io_apic.c |   99 +++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 103 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> > index 3e4bea3a52b1..c53587868590 100644
> > --- a/arch/x86/include/asm/io_apic.h
> > +++ b/arch/x86/include/asm/io_apic.h
> > @@ -173,7 +173,9 @@ enum ioapic_domain_type {
> >  };
> >  
> >  struct device_node;
> > +struct irq_domain;
> >  struct irq_domain_ops;
> > +
> >  struct ioapic_domain_cfg {
> >  	enum ioapic_domain_type		type;
> >  	const struct irq_domain_ops	*ops;
> > @@ -192,6 +194,9 @@ extern u32 mp_pin_to_gsi(int ioapic, int pin);
> >  extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
> >  extern void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
> >  				      struct ioapic_domain_cfg *cfg);
> > +extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
> > +			    irq_hw_number_t hwirq);
> > +extern int mp_set_gsi_attr(u32 gsi, int trigger, int polarity, int node);
> >  extern void __init pre_init_apic_IRQ0(void);
> >  
> >  extern void mp_save_irq(struct mpc_intsrc *m);
> > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> > index 9c5f70699a80..61aae90f7e23 100644
> > --- a/arch/x86/kernel/apic/io_apic.c
> > +++ b/arch/x86/kernel/apic/io_apic.c
> > @@ -87,6 +87,14 @@ static DEFINE_RAW_SPINLOCK(vector_lock);
> >  static DEFINE_MUTEX(ioapic_mutex);
> >  static unsigned int ioapic_dynirq_base;
> >  
> > +struct mp_pin_info {
> > +	int trigger;
> > +	int polarity;
> > +	int node;
> > +	int set;
> > +	u32 count;
> > +};
> > +
> >  static struct ioapic {
> >  	/*
> >  	 * # of IRQ routing registers
> > @@ -102,6 +110,7 @@ static struct ioapic {
> >  	struct mp_ioapic_gsi  gsi_config;
> >  	struct ioapic_domain_cfg irqdomain_cfg;
> >  	struct irq_domain *irqdomain;
> > +	struct mp_pin_info *pin_info;
> >  	DECLARE_BITMAP(pin_programmed, MP_MAX_IOAPIC_PIN + 1);
> >  } ioapics[MAX_IO_APICS];
> >  
> > @@ -147,6 +156,11 @@ static inline int mp_init_irq_at_boot(int ioapic, int irq)
> >  	return ioapic == 0 || (irq >= 0 && irq < nr_legacy_irqs());
> >  }
> >  
> > +static inline struct mp_pin_info *mp_pin_info(int ioapic_idx, int pin)
> > +{
> > +	return ioapics[ioapic_idx].pin_info + pin;
> > +}
> > +
> >  static inline struct irq_domain *mp_ioapic_irqdomain(int ioapic)
> >  {
> >  	return ioapics[ioapic].irqdomain;
> > @@ -1006,6 +1020,7 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
> >  {
> >  	int irq;
> >  	struct irq_domain *domain = mp_ioapic_irqdomain(ioapic);
> > +	struct mp_pin_info *info = mp_pin_info(ioapic, pin);
> >  
> >  	/*
> >  	 * Don't use irqdomain to manage ISA IRQs because there may be
> > @@ -1034,6 +1049,13 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
> >  	irq = irq_find_mapping(domain, pin);
> >  	if (irq <= 0 && (flags & IOAPIC_MAP_ALLOC))
> >  		irq = alloc_irq_from_domain(domain, gsi, pin);
> > +
> > +	if (flags & IOAPIC_MAP_ALLOC) {
> > +		if (irq > 0)
> > +			info->count++;
> > +		else if (info->count == 0)
> > +			info->set = 0;
> > +	}
> >  	mutex_unlock(&ioapic_mutex);
> >  
> >  	return irq > 0 ? irq : -1;
> > @@ -2923,18 +2945,27 @@ out:
> >  
> >  static int mp_irqdomain_create(int ioapic)
> >  {
> > +	size_t size;
> >  	int hwirqs = mp_ioapic_pin_count(ioapic);
> >  	struct ioapic *ip = &ioapics[ioapic];
> >  	struct ioapic_domain_cfg *cfg = &ip->irqdomain_cfg;
> >  	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(ioapic);
> >  
> > +	size = sizeof(struct mp_pin_info) * mp_ioapic_pin_count(ioapic);
> > +	ip->pin_info = kzalloc(size, GFP_KERNEL);
> > +	if (!ip->pin_info)
> > +		return -ENOMEM;
> > +
> >  	if (cfg->type == IOAPIC_DOMAIN_INVALID)
> >  		return 0;
> >  
> >  	ip->irqdomain = irq_domain_add_linear(cfg->dev, hwirqs, cfg->ops,
> >  					      (void *)(long)ioapic);
> > -	if(!ip->irqdomain)
> > +	if(!ip->irqdomain) {
> > +		kfree(ip->pin_info);
> > +		ip->pin_info = NULL;
> >  		return -ENOMEM;
> > +	}
> >  
> >  	if (cfg->type == IOAPIC_DOMAIN_LEGACY ||
> >  	    cfg->type == IOAPIC_DOMAIN_STRICT)
> > @@ -3898,6 +3929,72 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
> >  	nr_ioapics++;
> >  }
> >  
> > +int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
> > +		     irq_hw_number_t hwirq)
> > +{
> > +	int ioapic = (int)(long)domain->host_data;
> > +	struct mp_pin_info *info = mp_pin_info(ioapic, hwirq);
> > +	struct io_apic_irq_attr attr;
> > +
> > +	/*
> > +	 * Skip the timer IRQ if there's a quirk handler installed and if it
> > +	 * returns 1:
> > +	 */
> > +	if (apic->multi_timer_check &&
> > +	    apic->multi_timer_check(ioapic, virq))
> > +		return 0;
> > +
> > +	/* Get default attribute if not set by caller yet */
> > +	if (!info->set) {
> > +		u32 gsi = mp_pin_to_gsi(ioapic, hwirq);
> > +
> > +		if (acpi_get_override_irq(gsi, &info->trigger,
> > +					  &info->polarity) < 0) {
> 
> Sadly this seems to break LPSS ACPI enumerated devices.
> 
> Before this change /proc/interrupts say:
> 
>    0:         14          0          0          0   IO-APIC-edge      timer
>    1:         10          0          0          0   IO-APIC-edge      i8042
>    4:         79          0          0          0   IO-APIC-edge      serial
>    5:         52          0          0          0   IO-APIC-fasteoi   mmc0
>    6:          0          0          0          0   IO-APIC-fasteoi   dw_dmac
>    7:          0          0          0          0   IO-APIC-fasteoi   INT3432:00, INT3433:00
>    8:          1          0          0          0   IO-APIC-edge      rtc0
>    9:        174          0          0          0   IO-APIC-fasteoi   acpi
>   57:        476          0          0          0   PCI-MSI-edge      xhci_hcd
>   58:         17          0          0          0   PCI-MSI-edge      snd_hda_intel
> 
> After the change it looks like this:
> 
>   0:         14          0          0          0   IO-APIC-edge      timer
>   1:         10          0          0          0   IO-APIC-edge      i8042
>   4:         64          0          0          0   IO-APIC-edge      serial
>   5:          0          0          0          0   IO-APIC-edge      mmc0
>   6:          0          0          0          0   IO-APIC-edge      dw_dmac
>   7:          0          0          0          0   IO-APIC-edge      INT3432:00, INT3433:00
>   8:          1          0          0          0   IO-APIC-edge      rtc0
>   9:        173          0          0          0   IO-APIC-fasteoi   acpi
>  41:        471          0          0          0   PCI-MSI-edge      xhci_hcd
>  42:         17          0          0          0   PCI-MSI-edge      snd_hda_intel
> 
> Notice the interrupt triggering of IRQs 5, 6, and 7 has changed from level to
> edge. I also see this printed on the console:
> 
> [    1.676685] Failed to set pin attr for GSI6
> [    1.691708] Failed to set pin attr for GSI7
> [    1.706750] Failed to set pin attr for GSI7
> [    1.721768] Failed to set pin attr for GSI13
> [    1.736765] Failed to set pin attr for GSI5
> [    1.838342] Failed to set pin attr for GSI6
> 
> Any ideas how to get that fixed?
> 
> We used to handle this in drivers/acpi/resource.c:acpi_dev_get_irqresource() so
> that only IRQ() and IRQNoFlags() ACPI resources resort calling
> acpi_get_override_irq(). Now that doesn't help anymore.

There's also a bugzilla bug filed about this here where touchpad stopped
working:

https://bugzilla.kernel.org/show_bug.cgi?id=82601
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu Aug. 21, 2014, 7 p.m. UTC | #3
Hi Mika,
	I'm out of office until next Tuesday. Will handle this
when back to work. Could you please help to take a look at
mp_set_gsi_attr() to check whether it could be used to resolve
this issue.
Regards!
Gerry

On 2014/8/22 0:57, Mika Westerberg wrote:
> On Thu, Aug 21, 2014 at 05:17:29PM +0300, Mika Westerberg wrote:
>> On Mon, Jun 09, 2014 at 04:19:58PM +0800, Jiang Liu wrote:
>>> Currently there are multiple entries to program IOAPIC pins, such as
>>> io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
>>> setup_IO_APIC_irq_extra() etc.
>>>
>>> This patch introduces two functions to help consolidate the code to
>>> program IOAPIC pins. Function mp_set_pin_attr() is used to optionally
>>> set trigger, polarity and NUMA node property for an IOAPIC pin.
>>> If mp_set_pin_attr() is not invoked for a pin, the default configuration
>>> from BIOS will be used.
>>>
>>> Function mp_irqdomain_map() is an common implementation of irqdomain map()
>>> operation. It figures out attribures for pin and then actually programs
>>> the IOAPIC pin. We hope this will be the only entrance for programming
>>> IOAPIC pin.
>>>
>>> And the flow will:
>>> 1) caller such as xxx_pci_irq_enable figures out pin attributes.
>>> 2) Invoke mp_set_pin_attr() to set attributes for a pin. If the pin has
>>>    already bin programmed,  mp_set_pin_attr() will aslo detects attribute
>>>    confictions.
>>> 3) Invoke mp_map_pin_to_irq()
>>> 3.1) If IRQ has already been assigned, return irq_find_mapping()
>>> 3.2) Else irq_create_mapping()
>>> 		->irq_domain_associate()
>>> 			->mp_irqdomain_map()
>>> 				->io_apic_setup_irq_pin()
>>>
>>> So every pin will only programmed once by mp_irqdomain_map(), so we
>>> could kill io_apic_setup_irq_pin_once(), io_apic_set_pci_routing() and
>>> setup_IO_APIC_irq_extra() etc.
>>>
>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>> ---
>>>  arch/x86/include/asm/io_apic.h |    5 ++
>>>  arch/x86/kernel/apic/io_apic.c |   99 +++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 103 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>>> index 3e4bea3a52b1..c53587868590 100644
>>> --- a/arch/x86/include/asm/io_apic.h
>>> +++ b/arch/x86/include/asm/io_apic.h
>>> @@ -173,7 +173,9 @@ enum ioapic_domain_type {
>>>  };
>>>  
>>>  struct device_node;
>>> +struct irq_domain;
>>>  struct irq_domain_ops;
>>> +
>>>  struct ioapic_domain_cfg {
>>>  	enum ioapic_domain_type		type;
>>>  	const struct irq_domain_ops	*ops;
>>> @@ -192,6 +194,9 @@ extern u32 mp_pin_to_gsi(int ioapic, int pin);
>>>  extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
>>>  extern void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>  				      struct ioapic_domain_cfg *cfg);
>>> +extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
>>> +			    irq_hw_number_t hwirq);
>>> +extern int mp_set_gsi_attr(u32 gsi, int trigger, int polarity, int node);
>>>  extern void __init pre_init_apic_IRQ0(void);
>>>  
>>>  extern void mp_save_irq(struct mpc_intsrc *m);
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index 9c5f70699a80..61aae90f7e23 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -87,6 +87,14 @@ static DEFINE_RAW_SPINLOCK(vector_lock);
>>>  static DEFINE_MUTEX(ioapic_mutex);
>>>  static unsigned int ioapic_dynirq_base;
>>>  
>>> +struct mp_pin_info {
>>> +	int trigger;
>>> +	int polarity;
>>> +	int node;
>>> +	int set;
>>> +	u32 count;
>>> +};
>>> +
>>>  static struct ioapic {
>>>  	/*
>>>  	 * # of IRQ routing registers
>>> @@ -102,6 +110,7 @@ static struct ioapic {
>>>  	struct mp_ioapic_gsi  gsi_config;
>>>  	struct ioapic_domain_cfg irqdomain_cfg;
>>>  	struct irq_domain *irqdomain;
>>> +	struct mp_pin_info *pin_info;
>>>  	DECLARE_BITMAP(pin_programmed, MP_MAX_IOAPIC_PIN + 1);
>>>  } ioapics[MAX_IO_APICS];
>>>  
>>> @@ -147,6 +156,11 @@ static inline int mp_init_irq_at_boot(int ioapic, int irq)
>>>  	return ioapic == 0 || (irq >= 0 && irq < nr_legacy_irqs());
>>>  }
>>>  
>>> +static inline struct mp_pin_info *mp_pin_info(int ioapic_idx, int pin)
>>> +{
>>> +	return ioapics[ioapic_idx].pin_info + pin;
>>> +}
>>> +
>>>  static inline struct irq_domain *mp_ioapic_irqdomain(int ioapic)
>>>  {
>>>  	return ioapics[ioapic].irqdomain;
>>> @@ -1006,6 +1020,7 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
>>>  {
>>>  	int irq;
>>>  	struct irq_domain *domain = mp_ioapic_irqdomain(ioapic);
>>> +	struct mp_pin_info *info = mp_pin_info(ioapic, pin);
>>>  
>>>  	/*
>>>  	 * Don't use irqdomain to manage ISA IRQs because there may be
>>> @@ -1034,6 +1049,13 @@ static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
>>>  	irq = irq_find_mapping(domain, pin);
>>>  	if (irq <= 0 && (flags & IOAPIC_MAP_ALLOC))
>>>  		irq = alloc_irq_from_domain(domain, gsi, pin);
>>> +
>>> +	if (flags & IOAPIC_MAP_ALLOC) {
>>> +		if (irq > 0)
>>> +			info->count++;
>>> +		else if (info->count == 0)
>>> +			info->set = 0;
>>> +	}
>>>  	mutex_unlock(&ioapic_mutex);
>>>  
>>>  	return irq > 0 ? irq : -1;
>>> @@ -2923,18 +2945,27 @@ out:
>>>  
>>>  static int mp_irqdomain_create(int ioapic)
>>>  {
>>> +	size_t size;
>>>  	int hwirqs = mp_ioapic_pin_count(ioapic);
>>>  	struct ioapic *ip = &ioapics[ioapic];
>>>  	struct ioapic_domain_cfg *cfg = &ip->irqdomain_cfg;
>>>  	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(ioapic);
>>>  
>>> +	size = sizeof(struct mp_pin_info) * mp_ioapic_pin_count(ioapic);
>>> +	ip->pin_info = kzalloc(size, GFP_KERNEL);
>>> +	if (!ip->pin_info)
>>> +		return -ENOMEM;
>>> +
>>>  	if (cfg->type == IOAPIC_DOMAIN_INVALID)
>>>  		return 0;
>>>  
>>>  	ip->irqdomain = irq_domain_add_linear(cfg->dev, hwirqs, cfg->ops,
>>>  					      (void *)(long)ioapic);
>>> -	if(!ip->irqdomain)
>>> +	if(!ip->irqdomain) {
>>> +		kfree(ip->pin_info);
>>> +		ip->pin_info = NULL;
>>>  		return -ENOMEM;
>>> +	}
>>>  
>>>  	if (cfg->type == IOAPIC_DOMAIN_LEGACY ||
>>>  	    cfg->type == IOAPIC_DOMAIN_STRICT)
>>> @@ -3898,6 +3929,72 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>  	nr_ioapics++;
>>>  }
>>>  
>>> +int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
>>> +		     irq_hw_number_t hwirq)
>>> +{
>>> +	int ioapic = (int)(long)domain->host_data;
>>> +	struct mp_pin_info *info = mp_pin_info(ioapic, hwirq);
>>> +	struct io_apic_irq_attr attr;
>>> +
>>> +	/*
>>> +	 * Skip the timer IRQ if there's a quirk handler installed and if it
>>> +	 * returns 1:
>>> +	 */
>>> +	if (apic->multi_timer_check &&
>>> +	    apic->multi_timer_check(ioapic, virq))
>>> +		return 0;
>>> +
>>> +	/* Get default attribute if not set by caller yet */
>>> +	if (!info->set) {
>>> +		u32 gsi = mp_pin_to_gsi(ioapic, hwirq);
>>> +
>>> +		if (acpi_get_override_irq(gsi, &info->trigger,
>>> +					  &info->polarity) < 0) {
>>
>> Sadly this seems to break LPSS ACPI enumerated devices.
>>
>> Before this change /proc/interrupts say:
>>
>>    0:         14          0          0          0   IO-APIC-edge      timer
>>    1:         10          0          0          0   IO-APIC-edge      i8042
>>    4:         79          0          0          0   IO-APIC-edge      serial
>>    5:         52          0          0          0   IO-APIC-fasteoi   mmc0
>>    6:          0          0          0          0   IO-APIC-fasteoi   dw_dmac
>>    7:          0          0          0          0   IO-APIC-fasteoi   INT3432:00, INT3433:00
>>    8:          1          0          0          0   IO-APIC-edge      rtc0
>>    9:        174          0          0          0   IO-APIC-fasteoi   acpi
>>   57:        476          0          0          0   PCI-MSI-edge      xhci_hcd
>>   58:         17          0          0          0   PCI-MSI-edge      snd_hda_intel
>>
>> After the change it looks like this:
>>
>>   0:         14          0          0          0   IO-APIC-edge      timer
>>   1:         10          0          0          0   IO-APIC-edge      i8042
>>   4:         64          0          0          0   IO-APIC-edge      serial
>>   5:          0          0          0          0   IO-APIC-edge      mmc0
>>   6:          0          0          0          0   IO-APIC-edge      dw_dmac
>>   7:          0          0          0          0   IO-APIC-edge      INT3432:00, INT3433:00
>>   8:          1          0          0          0   IO-APIC-edge      rtc0
>>   9:        173          0          0          0   IO-APIC-fasteoi   acpi
>>  41:        471          0          0          0   PCI-MSI-edge      xhci_hcd
>>  42:         17          0          0          0   PCI-MSI-edge      snd_hda_intel
>>
>> Notice the interrupt triggering of IRQs 5, 6, and 7 has changed from level to
>> edge. I also see this printed on the console:
>>
>> [    1.676685] Failed to set pin attr for GSI6
>> [    1.691708] Failed to set pin attr for GSI7
>> [    1.706750] Failed to set pin attr for GSI7
>> [    1.721768] Failed to set pin attr for GSI13
>> [    1.736765] Failed to set pin attr for GSI5
>> [    1.838342] Failed to set pin attr for GSI6
>>
>> Any ideas how to get that fixed?
>>
>> We used to handle this in drivers/acpi/resource.c:acpi_dev_get_irqresource() so
>> that only IRQ() and IRQNoFlags() ACPI resources resort calling
>> acpi_get_override_irq(). Now that doesn't help anymore.
> 
> There's also a bugzilla bug filed about this here where touchpad stopped
> working:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=82601
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 3e4bea3a52b1..c53587868590 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -173,7 +173,9 @@  enum ioapic_domain_type {
 };
 
 struct device_node;
+struct irq_domain;
 struct irq_domain_ops;
+
 struct ioapic_domain_cfg {
 	enum ioapic_domain_type		type;
 	const struct irq_domain_ops	*ops;
@@ -192,6 +194,9 @@  extern u32 mp_pin_to_gsi(int ioapic, int pin);
 extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
 extern void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
 				      struct ioapic_domain_cfg *cfg);
+extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
+			    irq_hw_number_t hwirq);
+extern int mp_set_gsi_attr(u32 gsi, int trigger, int polarity, int node);
 extern void __init pre_init_apic_IRQ0(void);
 
 extern void mp_save_irq(struct mpc_intsrc *m);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 9c5f70699a80..61aae90f7e23 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -87,6 +87,14 @@  static DEFINE_RAW_SPINLOCK(vector_lock);
 static DEFINE_MUTEX(ioapic_mutex);
 static unsigned int ioapic_dynirq_base;
 
+struct mp_pin_info {
+	int trigger;
+	int polarity;
+	int node;
+	int set;
+	u32 count;
+};
+
 static struct ioapic {
 	/*
 	 * # of IRQ routing registers
@@ -102,6 +110,7 @@  static struct ioapic {
 	struct mp_ioapic_gsi  gsi_config;
 	struct ioapic_domain_cfg irqdomain_cfg;
 	struct irq_domain *irqdomain;
+	struct mp_pin_info *pin_info;
 	DECLARE_BITMAP(pin_programmed, MP_MAX_IOAPIC_PIN + 1);
 } ioapics[MAX_IO_APICS];
 
@@ -147,6 +156,11 @@  static inline int mp_init_irq_at_boot(int ioapic, int irq)
 	return ioapic == 0 || (irq >= 0 && irq < nr_legacy_irqs());
 }
 
+static inline struct mp_pin_info *mp_pin_info(int ioapic_idx, int pin)
+{
+	return ioapics[ioapic_idx].pin_info + pin;
+}
+
 static inline struct irq_domain *mp_ioapic_irqdomain(int ioapic)
 {
 	return ioapics[ioapic].irqdomain;
@@ -1006,6 +1020,7 @@  static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
 {
 	int irq;
 	struct irq_domain *domain = mp_ioapic_irqdomain(ioapic);
+	struct mp_pin_info *info = mp_pin_info(ioapic, pin);
 
 	/*
 	 * Don't use irqdomain to manage ISA IRQs because there may be
@@ -1034,6 +1049,13 @@  static int mp_map_pin_to_irq(u32 gsi, int idx, int ioapic, int pin,
 	irq = irq_find_mapping(domain, pin);
 	if (irq <= 0 && (flags & IOAPIC_MAP_ALLOC))
 		irq = alloc_irq_from_domain(domain, gsi, pin);
+
+	if (flags & IOAPIC_MAP_ALLOC) {
+		if (irq > 0)
+			info->count++;
+		else if (info->count == 0)
+			info->set = 0;
+	}
 	mutex_unlock(&ioapic_mutex);
 
 	return irq > 0 ? irq : -1;
@@ -2923,18 +2945,27 @@  out:
 
 static int mp_irqdomain_create(int ioapic)
 {
+	size_t size;
 	int hwirqs = mp_ioapic_pin_count(ioapic);
 	struct ioapic *ip = &ioapics[ioapic];
 	struct ioapic_domain_cfg *cfg = &ip->irqdomain_cfg;
 	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(ioapic);
 
+	size = sizeof(struct mp_pin_info) * mp_ioapic_pin_count(ioapic);
+	ip->pin_info = kzalloc(size, GFP_KERNEL);
+	if (!ip->pin_info)
+		return -ENOMEM;
+
 	if (cfg->type == IOAPIC_DOMAIN_INVALID)
 		return 0;
 
 	ip->irqdomain = irq_domain_add_linear(cfg->dev, hwirqs, cfg->ops,
 					      (void *)(long)ioapic);
-	if(!ip->irqdomain)
+	if(!ip->irqdomain) {
+		kfree(ip->pin_info);
+		ip->pin_info = NULL;
 		return -ENOMEM;
+	}
 
 	if (cfg->type == IOAPIC_DOMAIN_LEGACY ||
 	    cfg->type == IOAPIC_DOMAIN_STRICT)
@@ -3898,6 +3929,72 @@  void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	nr_ioapics++;
 }
 
+int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
+		     irq_hw_number_t hwirq)
+{
+	int ioapic = (int)(long)domain->host_data;
+	struct mp_pin_info *info = mp_pin_info(ioapic, hwirq);
+	struct io_apic_irq_attr attr;
+
+	/*
+	 * Skip the timer IRQ if there's a quirk handler installed and if it
+	 * returns 1:
+	 */
+	if (apic->multi_timer_check &&
+	    apic->multi_timer_check(ioapic, virq))
+		return 0;
+
+	/* Get default attribute if not set by caller yet */
+	if (!info->set) {
+		u32 gsi = mp_pin_to_gsi(ioapic, hwirq);
+
+		if (acpi_get_override_irq(gsi, &info->trigger,
+					  &info->polarity) < 0) {
+			/*
+			 * PCI interrupts are always polarity one level
+			 * triggered.
+			 */
+			info->trigger = 1;
+			info->polarity = 1;
+		}
+		info->node = NUMA_NO_NODE;
+		info->set = 1;
+	}
+	set_io_apic_irq_attr(&attr, ioapic, hwirq, info->trigger,
+			     info->polarity);
+
+	return io_apic_setup_irq_pin(virq, info->node, &attr);
+}
+
+int mp_set_gsi_attr(u32 gsi, int trigger, int polarity, int node)
+{
+	int ret = 0;
+	int ioapic, pin;
+	struct mp_pin_info *info;
+
+	ioapic = mp_find_ioapic(gsi);
+	if (ioapic < 0)
+		return -ENODEV;
+
+	pin = mp_find_ioapic_pin(ioapic, gsi);
+	info = mp_pin_info(ioapic, pin);
+	trigger = trigger ? 1 : 0;
+	polarity = polarity ? 1 : 0;
+
+	mutex_lock(&ioapic_mutex);
+	if (!info->set) {
+		info->trigger = trigger;
+		info->polarity = polarity;
+		info->node = node;
+		info->set = 1;
+	} else if (info->trigger != trigger || info->polarity != polarity) {
+		ret = -EBUSY;
+	}
+	mutex_unlock(&ioapic_mutex);
+
+	return ret;
+}
+
 /* Enable IOAPIC early just for system timer */
 void __init pre_init_apic_IRQ0(void)
 {