linux-kernel - Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

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: <56407967.1030506@arm.com>
Date:	Mon, 09 Nov 2015 10:45:59 +0000
From:	Marc Zyngier <marc.zyngier@....com>
To:	Jose Rivera <German.Rivera@...escale.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"arnd@...db.de" <arnd@...db.de>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Stuart Yoder <stuart.yoder@...escale.com>,
	Katz Itai <itai.katz@...escale.com>,
	Lijun Pan <Lijun.Pan@...escale.com>,
	Li Leo <LeoLi@...escale.com>,
	Scott Wood <scottwood@...escale.com>,
	"agraf@...e.de" <agraf@...e.de>,
	Hamciuc Bogdan <bhamciu1@...escale.com>,
	Marginean Alexandru <R89243@...escale.com>,
	Sharma Bhupesh <bhupesh.sharma@...escale.com>,
	Erez Nir <nir.erez@...escale.com>,
	Richard Schmitt <richard.schmitt@...escale.com>,
	"dan.carpenter@...cle.com" <dan.carpenter@...cle.com>,
	"jiang.liu@...ux.intel.com" <jiang.liu@...ux.intel.com>
Subject: Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for
 FSL-MC devices

On 06/11/15 23:20, Jose Rivera wrote:

[...]

>>> +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
>>> +				   struct fsl_mc_device_irq *mc_dev_irq) {
>>> +	int error;
>>> +	struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
>>> +	struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
>>> +	struct dprc_irq_cfg irq_cfg;

[...]

>>> +	/*
>>> +	 * DPRCs and other objects use different commands to set up IRQs,
>>> +	 * so we have to differentiate them here.
>>> +	 */
>>> +	if (owner_mc_dev == mc_bus_dev) {
>>> +		/*
>>> +		 * IRQ is for the mc_bus_dev's DPRC itself
>>> +		 */
>>> +		error = dprc_set_irq(mc_bus_dev->mc_io,
>>> +				     MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
>>> +				     mc_bus_dev->mc_handle,
>>> +				     mc_dev_irq->dev_irq_index,
>>> +				     &irq_cfg);
>>> +		if (error < 0) {
>>> +			dev_err(&owner_mc_dev->dev,
>>> +				"dprc_set_irq() failed: %d\n", error);
>>> +		}
>>> +	} else {
>>> +		error = dprc_set_obj_irq(mc_bus_dev->mc_io,
>>> +					 MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
>>> +					 mc_bus_dev->mc_handle,
>>> +					 owner_mc_dev->obj_desc.type,
>>> +					 owner_mc_dev->obj_desc.id,
>>> +					 mc_dev_irq->dev_irq_index,
>>> +					 &irq_cfg);
>>> +		if (error < 0) {
>>> +			dev_err(&owner_mc_dev->dev,
>>> +				"dprc_obj_set_irq() failed: %d\n", error);
>>> +		}
>>> +	}
>>
>> It feels a bit odd that you have all of this under a single MSI umbrella,
>> and yet have to differentiate between them. Do they have a different
>> programming interface? Or is that because these dprc_set_*_irq functions
>> do some other stuff behind the scene (I'm too lazy to check...)?
>>
> Due to MC firmware API limitations, dprc_set_obj_irq can only be used
> to set IRQs for the DPRC's children not for the DPRC itself.

Right. So this makes me wonder whether or not you have the right
approach here. The logic behind the bus type was that devices with a
common programming interface would share a bus type (the odd duck being
platform which is used to implement anything else).

Your description seems to suggest that only devices behind the DPRC
share that programming interface, and that the DPRC itself is the local
weirdo.  Should it be using the platform-msi subsystem instead? Or is it
just a matter of firmware oddity?

This is probably not a big deal, but it is worth keeping it in mind,
specially if that has visible consequences (in your device tree, for
example).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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/