|
| 1 | +From 719486dda84c1f058b24916dc386888cbb6ac1ee Mon Sep 17 00:00:00 2001 |
| 2 | +From: Mihir Patel < [email protected]> |
| 3 | +Date: Wed, 12 Feb 2025 21:04:48 +0000 |
| 4 | +Subject: [PATCH] Reset page select byte to 0 |
| 5 | + |
| 6 | +Description |
| 7 | +The optoe kernel driver currently assumes that the page select byte (page 0, byte 127) is set to 0h before accessing the upper memory on page 0h. |
| 8 | +This assumption causes failures when accessing the module's EEPROM in multiple scenarios. |
| 9 | +On the contrary, the driver currently sets the page select byte if the intended page to be accessed is > 0 as evident from the below code |
| 10 | +https://github.com/opencomputeproject/oom/blob/c32499a89dff005e7ff2acb48b33f55543ce5140/optoe/optoe.c#L344-L353 |
| 11 | + |
| 12 | +Impact |
| 13 | +If the optoe driver accesses any page except page 0h and the code for restoring the page select byte to 0h fails, subsequent attempts to |
| 14 | +access the upper memory of page 0h will fail because the page select byte from the previous transaction remains unchanged. |
| 15 | +https://github.com/opencomputeproject/oom/blob/c32499a89dff005e7ff2acb48b33f55543ce5140/optoe/optoe.c#L373-L392 |
| 16 | + |
| 17 | +Steps to recreate the failure scenario |
| 18 | +One way to recreate this issue is: |
| 19 | + |
| 20 | +1. Initiate CDB firmware download operation on a module supporting CDB foreground mode only |
| 21 | +2. Kill the process which is downloading the FW |
| 22 | +3. Dump the page select byte and ensure it displays a non-zero value |
| 23 | +4. Execute sfputil show fwversion EthernetXX CLI and you will observe a traceback. The traceback occurs because the vendor name is read |
| 24 | +with garbage content, as the page select byte is set to 0x9F instead of 0x0. |
| 25 | + |
| 26 | +Fix |
| 27 | +Before optoe accesses any address from upper page 0h, the page select byte is now set to 0. |
| 28 | +This fix is done only for non-SFP modules to limit the scope of impact. SFP modules require additional handling since the page select byte |
| 29 | +is on page A2h and NOT A0h unlike non-SFP modules which do not have the concept of A0h and A2h page. |
| 30 | + |
| 31 | +Testing |
| 32 | +The fix has been tested on a non-SFP module and it was ensured that the sfputil show fwversion EthernetXX CLI command executes successfully without any traceback. |
| 33 | + |
| 34 | +Signed-off-by: Mihir Patel < [email protected]> |
| 35 | +--- |
| 36 | + drivers/misc/eeprom/optoe.c | 21 +++++++++++++++++++++ |
| 37 | + 1 file changed, 21 insertions(+) |
| 38 | + |
| 39 | +diff --git a/drivers/misc/eeprom/optoe.c b/drivers/misc/eeprom/optoe.c |
| 40 | +index 22d2c0cd4..2ffcb23e9 100644 |
| 41 | +--- a/drivers/misc/eeprom/optoe.c |
| 42 | ++++ b/drivers/misc/eeprom/optoe.c |
| 43 | +@@ -528,6 +528,27 @@ static ssize_t optoe_eeprom_update_client(struct optoe_data *optoe, |
| 44 | + page, ret); |
| 45 | + return ret; |
| 46 | + } |
| 47 | ++ } else { |
| 48 | ++ /* |
| 49 | ++ * Reset page select byte to 0 when accessing upper memory (offset >= 128) of page 0h. |
| 50 | ++ * This is only done for non-SFP modules because: |
| 51 | ++ * 1. SFP modules have page select byte on A2h address (0x51), not A0h (0x50) unlike |
| 52 | ++ * non-SFP modules (QSFP, CMIS). This mandates additional handling for SFP modules. |
| 53 | ++ * 3. SFPs requiring page select changes are uncommon, so this fix targets non-SFP modules only |
| 54 | ++ * |
| 55 | ++ * Note: This fix can be enhanced in the future to support SFP modules if needed. |
| 56 | ++ */ |
| 57 | ++ if (optoe->dev_class != TWO_ADDR && phy_offset >= OPTOE_PAGE_SIZE) { |
| 58 | ++ page = 0; |
| 59 | ++ ret = optoe_eeprom_write(optoe, client, &page, |
| 60 | ++ OPTOE_PAGE_SELECT_REG, 1); |
| 61 | ++ if (ret < 0) { |
| 62 | ++ dev_err(&client->dev, |
| 63 | ++ "Non-SFP write page register for page %d failed ret:%d!\n", |
| 64 | ++ page, ret); |
| 65 | ++ return ret; |
| 66 | ++ } |
| 67 | ++ } |
| 68 | + } |
| 69 | + |
| 70 | + while (count) { |
| 71 | +-- |
| 72 | +2.25.1 |
| 73 | + |
0 commit comments