linux-kernel - Re: [char-misc-next 3/3] mei: add WPT second mei interface

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: <20140527212207.GB26089@kroah.com>
Date:	Tue, 27 May 2014 14:22:07 -0700
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Tomas Winkler <tomas.winkler@...el.com>
Cc:	arnd@...db.de, linux-kernel@...r.kernel.org,
	Alexander Usyskin <alexander.usyskin@...el.com>
Subject: Re: [char-misc-next 3/3] mei: add WPT second mei interface

On Tue, May 13, 2014 at 11:22:01AM +0300, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@...el.com>
> 
> Add WPT second mei interface.
> In order for correct device numbering we add mei device interface id
> to to mei_cfg structure
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> ---
>  drivers/misc/mei/hw-me-regs.h | 1 +
>  drivers/misc/mei/hw-me.c      | 5 +++++
>  drivers/misc/mei/hw-me.h      | 1 +
>  drivers/misc/mei/main.c       | 2 +-
>  drivers/misc/mei/mei_dev.h    | 2 ++
>  drivers/misc/mei/pci-me.c     | 1 +
>  6 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/mei/hw-me-regs.h b/drivers/misc/mei/hw-me-regs.h
> index a7856c0..c5feafd 100644
> --- a/drivers/misc/mei/hw-me-regs.h
> +++ b/drivers/misc/mei/hw-me-regs.h
> @@ -115,6 +115,7 @@
>  #define MEI_DEV_ID_LPT_HR     0x8CBA  /* Lynx Point H Refresh */
>  
>  #define MEI_DEV_ID_WPT_LP     0x9CBA  /* Wildcat Point LP */
> +#define MEI_DEV_ID_WPT_LP_2   0x9CBB  /* Wildcat Point LP 2 */
>  
>  /* Host Firmware Status Registers in PCI Config Space */
>  #define PCI_CFG_HFS_1         0x40
> diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> index 6a2d272..9066bb1 100644
> --- a/drivers/misc/mei/hw-me.c
> +++ b/drivers/misc/mei/hw-me.c
> @@ -844,6 +844,11 @@ const struct mei_cfg mei_me_pch_cfg = {
>  	MEI_CFG_PCH_HFS,
>  };
>  
> +/* PCH devices MEI 2 interface */
> +const struct mei_cfg mei_me_pch_2_cfg = {
> +	MEI_CFG_PCH_HFS,
> +	.mei_id = 1

That's going to be a recipe for disaster.  Have the MEI core allocate
the id numbers as things are registered, don't have the individual
drivers create their id.

> +};
>  
>  /* PCH Cougar Point and Patsburg with quirk for Node Manager exclusion */
>  const struct mei_cfg mei_me_pch_cpt_pbg_cfg = {
> diff --git a/drivers/misc/mei/hw-me.h b/drivers/misc/mei/hw-me.h
> index 12b0f4b..6fb841f 100644
> --- a/drivers/misc/mei/hw-me.h
> +++ b/drivers/misc/mei/hw-me.h
> @@ -41,6 +41,7 @@ struct mei_me_hw {
>  extern const struct mei_cfg mei_me_legacy_cfg;
>  extern const struct mei_cfg mei_me_ich_cfg;
>  extern const struct mei_cfg mei_me_pch_cfg;
> +extern const struct mei_cfg mei_me_pch_2_cfg;
>  extern const struct mei_cfg mei_me_pch_cpt_pbg_cfg;
>  extern const struct mei_cfg mei_me_lpt_cfg;
>  
> diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
> index c58e059..cf4ed5a 100644
> --- a/drivers/misc/mei/main.c
> +++ b/drivers/misc/mei/main.c
> @@ -667,7 +667,7 @@ int mei_register(struct mei_device *dev)
>  {
>  
>  	int ret, devno;
> -	int id = 0; /* FIXME: retrieve interface version*/
> +	int id = dev->cfg->mei_id;

Why can't you do this in patch 01 of this series?


>  
>  	/* Fill in the data structures */
>  	devno = MKDEV(MAJOR(mei_devt), id);
> diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> index a418565..a521783 100644
> --- a/drivers/misc/mei/mei_dev.h
> +++ b/drivers/misc/mei/mei_dev.h
> @@ -384,10 +384,12 @@ enum mei_pg_state {
>   *
>   * @fw_status - FW status
>   * @quirk_probe - device exclusion quirk
> + * @mei_id - id of mei device
>   */
>  struct mei_cfg {
>  	const struct mei_fw_status fw_status;
>  	bool (*quirk_probe)(struct pci_dev *pdev);
> +	int mei_id;

This should be part of the device, not the configuration, for the reason
specified above.

thanks,

greg k-h
--
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/