* [PATCH v2] net/r8169: update the new parser for the new firmware
@ 2011-06-15 3:45 Hayes Wang
2011-06-16 22:59 ` Francois Romieu
0 siblings, 1 reply; 7+ messages in thread
From: Hayes Wang @ 2011-06-15 3:45 UTC (permalink / raw)
To: romieu; +Cc: netdev, linux-kernel, Hayes Wang
Update the parser for the new firmware which is embedded some information.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/r8169.c | 96 +++++++++++++++++++++++++++++++++++++--------------
1 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index ef1ce2e..bd9e78f 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -594,6 +594,14 @@ struct ring_info {
u8 __pad[sizeof(void *) - sizeof(u32)];
};
+struct fw_info {
+ u32 magic;
+ char version[32];
+ u32 fw_start;
+ u32 fw_len;
+ u8 chksum;
+};
+
enum features {
RTL_FEATURE_WOL = (1 << 0),
RTL_FEATURE_MSI = (1 << 1),
@@ -1740,21 +1748,57 @@ static void rtl_writephy_batch(struct rtl8169_private *tp,
#define PHY_DELAY_MS 0xe0000000
#define PHY_WRITE_ERI_WORD 0xf0000000
+static int is_firmware_valid(const struct firmware *fw)
+{
+ struct fw_info *f_info = (struct fw_info *)fw->data;
+
+ if (f_info->magic == 0) {
+ size_t i, fw_size;
+ u8 checksum = 0;
+
+ for (i = 0; i < fw->size; i++) {
+ checksum += fw->data[i];
+ }
+
+ if (checksum != 0)
+ return 0;
+
+ fw_size = (fw->size - le32_to_cpu(f_info->fw_start)) / 4;
+ if (fw_size < le32_to_cpu(f_info->fw_len))
+ return 0;
+ } else if (fw->size % 4) {
+ return 0;
+ }
+
+ return 1;
+}
+
static void
rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
{
- __le32 *phytable = (__le32 *)fw->data;
+ struct fw_info *f_info = (struct fw_info *)fw->data;
struct net_device *dev = tp->dev;
- size_t index, fw_size = fw->size / sizeof(*phytable);
+ __le32 *phytable;
+ size_t i, fw_size;
u32 predata, count;
- if (fw->size % sizeof(*phytable)) {
- netif_err(tp, probe, dev, "odd sized firmware %zd\n", fw->size);
+ if (!is_firmware_valid(fw)) {
+ netif_err(tp, probe, dev, "Invalid firwmare\n");
return;
}
- for (index = 0; index < fw_size; index++) {
- u32 action = le32_to_cpu(phytable[index]);
+ if (f_info->magic == 0) {
+ netif_info(tp, probe, dev, "firmware: %s\n", f_info->version);
+
+ phytable = (__le32 *)(fw->data + le32_to_cpu(f_info->fw_start));
+ fw_size = le32_to_cpu(f_info->fw_len);
+ } else {
+ phytable = (__le32 *)fw->data;
+ fw_size = fw->size / sizeof(*phytable);
+ }
+
+ for (i = 0; i < fw_size; i++) {
+ u32 action = le32_to_cpu(phytable[i]);
u32 regno = (action & 0x0fff0000) >> 16;
switch(action & 0xf0000000) {
@@ -1769,14 +1813,14 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
break;
case PHY_BJMPN:
- if (regno > index) {
+ if (regno > i) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
}
break;
case PHY_READCOUNT_EQ_SKIP:
- if (index + 2 >= fw_size) {
+ if (i + 2 >= fw_size) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
@@ -1785,7 +1829,7 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
case PHY_COMP_EQ_SKIPN:
case PHY_COMP_NEQ_SKIPN:
case PHY_SKIPN:
- if (index + 1 + regno >= fw_size) {
+ if (i + 1 + regno >= fw_size) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
@@ -1805,8 +1849,8 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
predata = 0;
count = 0;
- for (index = 0; index < fw_size; ) {
- u32 action = le32_to_cpu(phytable[index]);
+ for (i = 0; i < fw_size; ) {
+ u32 action = le32_to_cpu(phytable[i]);
u32 data = action & 0x0000ffff;
u32 regno = (action & 0x0fff0000) >> 16;
@@ -1817,54 +1861,54 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
case PHY_READ:
predata = rtl_readphy(tp, regno);
count++;
- index++;
+ i++;
break;
case PHY_DATA_OR:
predata |= data;
- index++;
+ i++;
break;
case PHY_DATA_AND:
predata &= data;
- index++;
+ i++;
break;
case PHY_BJMPN:
- index -= regno;
+ i -= regno;
break;
case PHY_READ_EFUSE:
predata = rtl8168d_efuse_read(tp->mmio_addr, regno);
- index++;
+ i++;
break;
case PHY_CLEAR_READCOUNT:
count = 0;
- index++;
+ i++;
break;
case PHY_WRITE:
rtl_writephy(tp, regno, data);
- index++;
+ i++;
break;
case PHY_READCOUNT_EQ_SKIP:
- index += (count == data) ? 2 : 1;
+ i += (count == data) ? 2 : 1;
break;
case PHY_COMP_EQ_SKIPN:
if (predata == data)
- index += regno;
- index++;
+ i += regno;
+ i++;
break;
case PHY_COMP_NEQ_SKIPN:
if (predata != data)
- index += regno;
- index++;
+ i += regno;
+ i++;
break;
case PHY_WRITE_PREVIOUS:
rtl_writephy(tp, regno, predata);
- index++;
+ i++;
break;
case PHY_SKIPN:
- index += regno + 1;
+ i += regno + 1;
break;
case PHY_DELAY_MS:
mdelay(data);
- index++;
+ i++;
break;
case PHY_READ_MAC_BYTE:
--
1.7.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-15 3:45 [PATCH v2] net/r8169: update the new parser for the new firmware Hayes Wang
@ 2011-06-16 22:59 ` Francois Romieu
2011-06-17 3:25 ` hayeswang
0 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2011-06-16 22:59 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, linux-kernel
Hayes Wang <hayeswang@realtek.com> :
> Update the parser for the new firmware which is embedded some information.
I have modified several things :
- s/u32/__le32/ in fw_info
- fix unsigned (size_t) comparisons
- more size checks before dereferencing fw_info
The new firmware format should be the same. The old r8168d-1.fw firmware
proved usable when prefixed with :
0000000: 0000 0000 3031 0000 0000 0000 0000 0000 ....01..........
0000010: 0000 0000 0000 0000 0000 0000 0000 0000 ................
0000020: 0000 0000 3000 0000 7501 0000 a000 0000 ....0...u.......
I realized after testing that netif_err could be abused with non-string
fw_info.version. :o/
Comments ?
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 7310824..5a0a7c3 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1741,21 +1741,82 @@ static void rtl_writephy_batch(struct rtl8169_private *tp,
#define PHY_DELAY_MS 0xe0000000
#define PHY_WRITE_ERI_WORD 0xf0000000
+struct fw_info {
+ u32 magic;
+ char version[32];
+ __le32 fw_start;
+ __le32 fw_len;
+ u8 chksum;
+} __packed;
+
+struct fw_phy_code {
+ __le32 *code;
+ size_t size;
+};
+
+#define FW_OPCODE_SIZE sizeof(typeof(*((struct fw_phy_code *)0)->code))
+
+static bool rtl_fw_format_ok(struct rtl8169_private *tp, struct net_device *dev,
+ const struct firmware *fw, struct fw_phy_code *pc)
+{
+ struct fw_info *fw_info = (struct fw_info *)fw->data;
+ bool rc = false;
+
+ if (fw->size < FW_OPCODE_SIZE)
+ goto out;
+
+ if (fw_info->magic == 0) {
+ size_t i, size, start;
+ u8 checksum = 0;
+
+ if (fw->size < sizeof(*fw_info))
+ goto out;
+
+ for (i = 0; i < fw->size; i++)
+ checksum += fw->data[i];
+ if (checksum != 0)
+ goto out;
+
+ start = le32_to_cpu(fw_info->fw_start);
+ if (start > fw->size)
+ goto out;
+
+ size = le32_to_cpu(fw_info->fw_len);
+ if (size > (fw->size - start) / FW_OPCODE_SIZE)
+ goto out;
+
+ netif_info(tp, probe, dev, "firmware: %s\n", fw_info->version);
+ pc->code = (__le32 *)(fw->data + start);
+ pc->size = size;
+ } else {
+ if (fw->size % FW_OPCODE_SIZE)
+ goto out;
+
+ netif_info(tp, probe, dev, "legacy firmware format detected\n");
+ pc->code = (__le32 *)fw->data;
+ pc->size = fw->size / FW_OPCODE_SIZE;
+ }
+
+ rc = true;
+out:
+ return rc;
+}
+
static void
rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
{
- __le32 *phytable = (__le32 *)fw->data;
struct net_device *dev = tp->dev;
- size_t index, fw_size = fw->size / sizeof(*phytable);
+ struct fw_phy_code phytable;
u32 predata, count;
+ size_t i;
- if (fw->size % sizeof(*phytable)) {
- netif_err(tp, probe, dev, "odd sized firmware %zd\n", fw->size);
+ if (!rtl_fw_format_ok(tp, dev, fw, &phytable)) {
+ netif_err(tp, probe, dev, "invalid firwmare\n");
return;
}
- for (index = 0; index < fw_size; index++) {
- u32 action = le32_to_cpu(phytable[index]);
+ for (i = 0; i < phytable.size; i++) {
+ u32 action = le32_to_cpu(phytable.code[i]);
u32 regno = (action & 0x0fff0000) >> 16;
switch(action & 0xf0000000) {
@@ -1770,14 +1831,14 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
break;
case PHY_BJMPN:
- if (regno > index) {
+ if (regno > i) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
}
break;
case PHY_READCOUNT_EQ_SKIP:
- if (index + 2 >= fw_size) {
+ if (i + 2 >= phytable.size) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
@@ -1786,7 +1847,7 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
case PHY_COMP_EQ_SKIPN:
case PHY_COMP_NEQ_SKIPN:
case PHY_SKIPN:
- if (index + 1 + regno >= fw_size) {
+ if (i + 1 + regno >= phytable.size) {
netif_err(tp, probe, tp->dev,
"Out of range of firmware\n");
return;
@@ -1806,8 +1867,8 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
predata = 0;
count = 0;
- for (index = 0; index < fw_size; ) {
- u32 action = le32_to_cpu(phytable[index]);
+ for (i = 0; i < phytable.size; ) {
+ u32 action = le32_to_cpu(phytable.code[i]);
u32 data = action & 0x0000ffff;
u32 regno = (action & 0x0fff0000) >> 16;
@@ -1818,54 +1879,54 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
case PHY_READ:
predata = rtl_readphy(tp, regno);
count++;
- index++;
+ i++;
break;
case PHY_DATA_OR:
predata |= data;
- index++;
+ i++;
break;
case PHY_DATA_AND:
predata &= data;
- index++;
+ i++;
break;
case PHY_BJMPN:
- index -= regno;
+ i -= regno;
break;
case PHY_READ_EFUSE:
predata = rtl8168d_efuse_read(tp->mmio_addr, regno);
- index++;
+ i++;
break;
case PHY_CLEAR_READCOUNT:
count = 0;
- index++;
+ i++;
break;
case PHY_WRITE:
rtl_writephy(tp, regno, data);
- index++;
+ i++;
break;
case PHY_READCOUNT_EQ_SKIP:
- index += (count == data) ? 2 : 1;
+ i += (count == data) ? 2 : 1;
break;
case PHY_COMP_EQ_SKIPN:
if (predata == data)
- index += regno;
- index++;
+ i += regno;
+ i++;
break;
case PHY_COMP_NEQ_SKIPN:
if (predata != data)
- index += regno;
- index++;
+ i += regno;
+ i++;
break;
case PHY_WRITE_PREVIOUS:
rtl_writephy(tp, regno, predata);
- index++;
+ i++;
break;
case PHY_SKIPN:
- index += regno + 1;
+ i += regno + 1;
break;
case PHY_DELAY_MS:
mdelay(data);
- index++;
+ i++;
break;
case PHY_READ_MAC_BYTE:
^ permalink raw reply related [flat|nested] 7+ messages in thread* RE: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-16 22:59 ` Francois Romieu
@ 2011-06-17 3:25 ` hayeswang
2011-06-17 6:39 ` Francois Romieu
0 siblings, 1 reply; 7+ messages in thread
From: hayeswang @ 2011-06-17 3:25 UTC (permalink / raw)
To: 'Francois Romieu'; +Cc: netdev, linux-kernel
> -----Original Message-----
> From: Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Friday, June 17, 2011 6:59 AM
> To: Hayeswang
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] net/r8169: update the new parser for
> the new firmware
>
> Hayes Wang <hayeswang@realtek.com> :
> > Update the parser for the new firmware which is embedded
> some information.
>
> I have modified several things :
> - s/u32/__le32/ in fw_info
> - fix unsigned (size_t) comparisons
> - more size checks before dereferencing fw_info
>
Thanks. They are fine.
> The new firmware format should be the same. The old
> r8168d-1.fw firmware proved usable when prefixed with :
>
> 0000000: 0000 0000 3031 0000 0000 0000 0000 0000 ....01..........
> 0000010: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> 0000020: 0000 0000 3000 0000 7501 0000 a000 0000 ....0...u.......
>
> I realized after testing that netif_err could be abused with
> non-string fw_info.version. :o/
>
Excuse me. I don't understand what you want to express. Do you mean the
situation of the old paser with the new firmware for checking the firmware? For
the normal situation, the old paser would not use the new firmware. And I put
zero in front of the new firmware to prevent the old paser from running it. That
is all I do.
If you don't mean that, I could promise the new firmware I release would contain
the valid string unless someone modifies it.
> Comments ?
>
Best Regards,
Hayes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-17 3:25 ` hayeswang
@ 2011-06-17 6:39 ` Francois Romieu
2011-06-17 12:22 ` Ben Hutchings
0 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2011-06-17 6:39 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, linux-kernel
hayeswang <hayeswang@realtek.com> :
> Francois Romieu :
[...]
> > The new firmware format should be the same. The old
> > r8168d-1.fw firmware proved usable when prefixed with :
> >
> > 0000000: 0000 0000 3031 0000 0000 0000 0000 0000 ....01..........
> > 0000010: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> > 0000020: 0000 0000 3000 0000 7501 0000 a000 0000 ....0...u.......
> >
> > I realized after testing that netif_err could be abused with
> > non-string fw_info.version. :o/
> >
>
> Excuse me. I don't understand what you want to express. Do you mean the
> situation of the old paser with the new firmware for checking the firmware?
No. The code does a printk with a %s specifier on fw_info.version while there
is no evidence that fw_info.version is 0 terminated.
> For the normal situation, the old paser would not use the new firmware.
> And I put zero in front of the new firmware to prevent the old paser from
> running it. That is all I do.
It is fine.
--
Ueimor
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-17 6:39 ` Francois Romieu
@ 2011-06-17 12:22 ` Ben Hutchings
2011-06-17 12:53 ` Francois Romieu
0 siblings, 1 reply; 7+ messages in thread
From: Ben Hutchings @ 2011-06-17 12:22 UTC (permalink / raw)
To: Francois Romieu; +Cc: hayeswang, netdev, linux-kernel
On Fri, 2011-06-17 at 08:39 +0200, Francois Romieu wrote:
> hayeswang <hayeswang@realtek.com> :
> > Francois Romieu :
> [...]
> > > The new firmware format should be the same. The old
> > > r8168d-1.fw firmware proved usable when prefixed with :
> > >
> > > 0000000: 0000 0000 3031 0000 0000 0000 0000 0000 ....01..........
> > > 0000010: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> > > 0000020: 0000 0000 3000 0000 7501 0000 a000 0000 ....0...u.......
> > >
> > > I realized after testing that netif_err could be abused with
> > > non-string fw_info.version. :o/
> > >
> >
> > Excuse me. I don't understand what you want to express. Do you mean the
> > situation of the old paser with the new firmware for checking the firmware?
>
> No. The code does a printk with a %s specifier on fw_info.version while there
> is no evidence that fw_info.version is 0 terminated.
I thought the idea of embedding a version here was to be able to report
in in get_drvinfo, not to print it on load.
Ben.
> > For the normal situation, the old paser would not use the new firmware.
> > And I put zero in front of the new firmware to prevent the old paser from
> > running it. That is all I do.
>
> It is fine.
>
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-17 12:22 ` Ben Hutchings
@ 2011-06-17 12:53 ` Francois Romieu
2011-06-17 14:50 ` Ben Hutchings
0 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2011-06-17 12:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: hayeswang, netdev, linux-kernel
Ben Hutchings <bhutchings@solarflare.com> :
[...]
> I thought the idea of embedding a version here was to be able to report
> in in get_drvinfo, not to print it on load.
Reporting the version for the new firmware format through ethtool is
currently hacked on.
Do you want the load time version / format messages removed ?
I think they can be of some use - at least temporarily - but I won't mind
removing them now.
--
Ueimor
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/r8169: update the new parser for the new firmware
2011-06-17 12:53 ` Francois Romieu
@ 2011-06-17 14:50 ` Ben Hutchings
0 siblings, 0 replies; 7+ messages in thread
From: Ben Hutchings @ 2011-06-17 14:50 UTC (permalink / raw)
To: Francois Romieu; +Cc: hayeswang, netdev, linux-kernel
On Fri, 2011-06-17 at 14:53 +0200, Francois Romieu wrote:
> Ben Hutchings <bhutchings@solarflare.com> :
> [...]
> > I thought the idea of embedding a version here was to be able to report
> > in in get_drvinfo, not to print it on load.
>
> Reporting the version for the new firmware format through ethtool is
> currently hacked on.
>
> Do you want the load time version / format messages removed ?
>
> I think they can be of some use - at least temporarily - but I won't mind
> removing them now.
I don't care one way or the other. But it's more important to report
the version through get_drvinfo.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-06-17 14:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-15 3:45 [PATCH v2] net/r8169: update the new parser for the new firmware Hayes Wang
2011-06-16 22:59 ` Francois Romieu
2011-06-17 3:25 ` hayeswang
2011-06-17 6:39 ` Francois Romieu
2011-06-17 12:22 ` Ben Hutchings
2011-06-17 12:53 ` Francois Romieu
2011-06-17 14:50 ` Ben Hutchings
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).