linux-kernel - Re: [patch 04/16] genirq: Introduce irq_chip.irq_compose_msi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <546B10D2.4050300@huawei.com>
Date:	Tue, 18 Nov 2014 17:26:42 +0800
From:	"Yun Wu (Abel)" <wuyun.wu@...wei.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Grant Likely" <grant.likely@...aro.org>,
	Marc Zyngier <marc.zyngier@....com>,
	Yingjoe Chen <yingjoe.chen@...iatek.com>,
	Yijing Wang <wangyijing@...wei.com>
Subject: Re: [patch 04/16] genirq: Introduce irq_chip.irq_compose_msi_msg()
 to support stacked irqchip

On 2014/11/12 21:42, Thomas Gleixner wrote:

> From: Jiang Liu <jiang.liu@...ux.intel.com>
> 
> Add callback irq_compose_msi_msg to struct irq_chip, which will be used
> to support stacked irqchip.
> 
> Signed-off-by: Jiang Liu <jiang.liu@...ux.intel.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Grant Likely <grant.likely@...aro.org>
> Cc: Marc Zyngier <marc.zyngier@....com>
> Cc: Yingjoe Chen <yingjoe.chen@...iatek.com>
> Cc: Yijing Wang <wangyijing@...wei.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  include/linux/irq.h |    5 +++++
>  kernel/irq/chip.c   |   17 +++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 0adcbbbf2e87..536b7fc6c8f4 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -29,6 +29,7 @@ struct seq_file;
>  struct module;
>  struct irq_desc;
>  struct irq_data;
> +struct msi_msg;
>  typedef	void (*irq_flow_handler_t)(unsigned int irq,
>  					    struct irq_desc *desc);
>  typedef	void (*irq_preflow_handler_t)(struct irq_data *data);
> @@ -320,6 +321,7 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
>   *				any other callback related to this irq
>   * @irq_release_resources:	optional to release resources acquired with
>   *				irq_request_resources
> + * @irq_compose_msi_msg:	optional to compose message content for MSI
>   * @flags:		chip specific flags
>   */
>  struct irq_chip {
> @@ -356,6 +358,8 @@ struct irq_chip {
>  	int		(*irq_request_resources)(struct irq_data *data);
>  	void		(*irq_release_resources)(struct irq_data *data);
>  
> +	void		(*irq_compose_msi_msg)(struct irq_data *data, struct msi_msg *msg);
> +
>  	unsigned long	flags;
>  };
>  
> @@ -443,6 +447,7 @@ extern void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_bad_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_nested_irq(unsigned int irq);
>  
> +extern int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg);
>  #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
>  extern void irq_chip_ack_parent(struct irq_data *data);
>  extern int irq_chip_retrigger_hierarchy(struct irq_data *data);
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 12f3e72449eb..8f362db17a8a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -867,3 +867,20 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
>  	return -ENOSYS;
>  }
>  #endif
> +
> +int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> +	struct irq_data *pos = NULL;
> +
> +#ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
> +	for (; data; data = data->parent_data)
> +#endif
> +		if (data->chip && data->chip->irq_compose_msi_msg)
> +			pos = data;
> +	if (!pos)
> +		return -ENOSYS;
> +
> +	pos->chip->irq_compose_msi_msg(pos, msg);
> +
> +	return 0;
> +}

Adding message composing routine to struct irq_chip is OK to me, and it should
be because it is interrupt controllers' duty to compose messages (so that they
can parse the messages correctly without any pre-defined rules that endpoint
devices absolutely need not to know).
However a problem comes out when deciding which parameters should be passed to
this routine. A message can associate with multiple interrupts, which makes me
think composing messages for each interrupt is not that appropriate. And we
can take a look at the new routine irq_chip_compose_msi_msg(). It is called by
msi_domain_activate() which will be called by irq_domain_activate_irq() in
irq_startup() for each interrupt descriptor, result in composing a message for
each interrupt, right? (Unless requiring a judge on the parameter @data when
implementing the irq_compose_msi_msg() callback that only compose message for
the first entry of that message. But I really don't like that...)

Regards,
	Abel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/