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] [day] [month] [year] [list]

Message-ID: <DM2PR0301MB1309BE69516E98AE9E455CC9FE150@DM2PR0301MB1309.namprd03.prod.outlook.com>
Date:	Mon, 9 Nov 2015 21:18:45 +0000
From:	Jose Rivera <German.Rivera@...escale.com>
To:	Marc Zyngier <marc.zyngier@....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

> -----Original Message-----
> From: Marc Zyngier [mailto:marc.zyngier@....com]
> Sent: Monday, November 09, 2015 4:46 AM
> To: Rivera Jose-B46482; gregkh@...uxfoundation.org; arnd@...db.de;
> devel@...verdev.osuosl.org; linux-kernel@...r.kernel.org
> Cc: Yoder Stuart-B08248; katz Itai-RM05202; Pan Lijun-B44306; Li Yang-
> Leo-R58472; Wood Scott-B07421; agraf@...e.de; Hamciuc Bogdan-BHAMCIU1;
> Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794;
> Schmitt Richard-B43082; dan.carpenter@...cle.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?
> 
It's really a MC API oddity--  both the DPRC and the devices behind it share
the same MSI context (same ITT in the ITS) but they just require different
APIs to set the MSI addr/data.

> 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/