linux-kernel - Re: [PATCH net-next] myri10ge: allow per-board firmware overriding

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20090417095038.49b745bf@dhcp-lab-109.englab.brq.redhat.com>
Date:	Fri, 17 Apr 2009 09:50:38 +0200
From:	Stanislaw Gruszka <sgruszka@...hat.com>
To:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc:	Brice Goglin <brice@...i.com>,
	"David S. Miller" <davem@...emloft.net>,
	Linux Network Development list <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] myri10ge: allow per-board firmware overriding

On Thu, 16 Apr 2009 14:24:59 +0200
Brice Goglin <brice@...i.com> wrote:

> --- linux-tmp.old/drivers/net/myri10ge/myri10ge.c	2009-04-08 06:56:50.000000000 +0200
> +++ linux-tmp/drivers/net/myri10ge/myri10ge.c	2009-04-09 14:00:16.000000000 +0200
> @@ -255,6 +255,7 @@
>  	u32 read_write_dma;
>  	u32 link_changes;
>  	u32 msg_enable;
> +	unsigned int board_number;
>  };
>  
>  static char *myri10ge_fw_unaligned = "myri10ge_ethp_z8e.dat";
> @@ -266,6 +267,13 @@
>  module_param(myri10ge_fw_name, charp, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(myri10ge_fw_name, "Firmware image name");
>  
> +#define MYRI10GE_MAX_BOARDS 8
> +static char *myri10ge_fw_names[MYRI10GE_MAX_BOARDS] =
> +    {[0...(MYRI10GE_MAX_BOARDS - 1)] = NULL };
> +module_param_array_named(myri10ge_fw_names, myri10ge_fw_names, charp, NULL,
> +			 0444);
> +MODULE_PARM_DESC(myri10ge_fw_name, "Firmware image names per board");
> +
>  static int myri10ge_ecrc_enable = 1;
>  module_param(myri10ge_ecrc_enable, int, S_IRUGO);
>  MODULE_PARM_DESC(myri10ge_ecrc_enable, "Enable Extended CRC on PCI-E");
> @@ -3258,6 +3266,8 @@
>  
>  static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
>  {
> +	int overridden = 0;
> +
>  	if (myri10ge_force_firmware == 0) {
>  		int link_width, exp_cap;
>  		u16 lnk;
> @@ -3291,10 +3301,18 @@
>  		}
>  	}
>  	if (myri10ge_fw_name != NULL) {
> -		dev_info(&mgp->pdev->dev, "overriding firmware to %s\n",
> -			 myri10ge_fw_name);
> +		overridden = 1;
>  		mgp->fw_name = myri10ge_fw_name;
>  	}
> +	if (mgp->board_number < MYRI10GE_MAX_BOARDS &&
> +	    myri10ge_fw_names[mgp->board_number] != NULL &&
> +	    strlen(myri10ge_fw_names[mgp->board_number])) {
> +		mgp->fw_name = myri10ge_fw_names[mgp->board_number];
> +		overridden = 1;
> +	}
> +	if (overridden)
> +		dev_info(&mgp->pdev->dev, "overriding firmware to %s\n",
> +			 mgp->fw_name);
>  }
>  
>  #ifdef CONFIG_PM
> @@ -3759,6 +3777,7 @@
>  	int status = -ENXIO;
>  	int dac_enabled;
>  	unsigned hdr_offset, ss_offset;
> +	static int board_number;
>  
>  	netdev = alloc_etherdev_mq(sizeof(*mgp), MYRI10GE_MAX_SLICES);
>  	if (netdev == NULL) {
> @@ -3775,6 +3794,7 @@
>  	mgp->pause = myri10ge_flow_control;
>  	mgp->intr_coal_delay = myri10ge_intr_coal_delay;
>  	mgp->msg_enable = netif_msg_init(myri10ge_debug, MYRI10GE_MSG_DEFAULT);
> +	mgp->board_number = board_number;
>  	init_waitqueue_head(&mgp->down_wq);
>  
>  	if (pci_enable_device(pdev)) {
> @@ -3925,6 +3945,7 @@
>  			 netdev->irq, mgp->tx_boundary, mgp->fw_name,
>  			 (mgp->wc_enabled ? "Enabled" : "Disabled"));
>  
> +	board_number++;
>  	return 0;
>  
>  abort_with_state:

Code assumes boards are always initialized in the same order. I suppose
this is always true and is nothing wrong here. However perhaps linux could
provide some form of modules/boot parameters that are used with
an identification number like MAC address. Example:

module my_parms="id1:param1;id2:param2;" 

char *get_module_param_id(const char *param, const char *id)

This would help in situation like this - admin will not have to thinking
about boards initialization ordering. Besides some other drivers (like MTD
cmd partitions) use own parsing for similar things, I think would be
nice to unify things. What You think?

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