Ticket #11215 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

XO-1.75 may brick OpenFirmware by power button press during flash

Reported by: Quozl Owned by: greenfeld
Priority: normal Milestone: 1.75-firmware
Component: ofw - open firmware Version: Development firmware
Keywords: Cc:
Action Needed: no action Verified: no
Deployments affected: Blocked By:
Blocking:

Description

With XO-1 and XO-1.5 we disabled the power button during bulk write to the SPI FLASH, in order to reduce the risk of bricking caused by the user pressing the power button during flashing.

With XO-1.75 we have two SPI FLASH chips. When reflashing the EC, the power button is already disabled. When reflashing OpenFirmware, it is not.

Provide a method to disable the power button.

Attachments

0001-add-support-for-inhibit-of-power-button-force-off.patch Download (2.7 KB) - added by Quozl 3 years ago.
patch proposed for review
0001-clear-the-power-button-force-off-inhibit-on-next-pow.patch Download (0.5 KB) - added by Quozl 3 years ago.
additional patch on top of previous patch to ensure the inhibit is not persistent over host power cycle

Change History

Changed 3 years ago by Quozl

patch proposed for review

Changed 3 years ago by Quozl

additional patch on top of previous patch to ensure the inhibit is not persistent over host power cycle

in reply to: ↑ description   Changed 3 years ago by rsmith

Replying to Quozl:

Provide a method to disable the power button.

I think the best way to handle may be to put the EC into reset while updating the host SPI flash. A reboot happens after the reflash anyway. I worry that adding commands to enable/disable the power button will just result in power button behavior bugs.

follow-up: ↓ 3   Changed 3 years ago by Quozl

Please explain further. I don't see any way to put the EC into reset and keep it there.

  • EC_RST# (U20.36) is not connected, according to the schematic,
  • there's no command in sdicmd.c for putting the EC into reset that doesn't also let the EC restart again,
  • the reflash after reboot requires the EC to be running, since it uses 0x4b CMD_POWER_CYCLE, so putting it into reset and keeping it there would prevent reboot, and this does not seem right.

in reply to: ↑ 2   Changed 3 years ago by rsmith

Replying to Quozl:

Please explain further. I don't see any way to put the EC into reset and keep it there.

Same way its done when you re-program the EC. The EDI interface has access to all the registers and one of those allows you to put the EC into reset.

  Changed 3 years ago by Quozl

  • owner changed from rsmith to wmb@…
  • next_action changed from review to design
  • component changed from embedded controller to ofw - open firmware

Spent a few hours on q4b09jf which used EDI to put the EC into reset before trying to flash OpenFirmware ... but found that the generic SPI layer is set up for one use at a time, and OpenFirmware uses it both for the EDI interface to the EC and for the SPI interface to the SPI FLASH. More time will be required to get this right. I'm worried about reflash bugs.

  Changed 3 years ago by Quozl

  • owner changed from wmb@… to Quozl
  • status changed from new to assigned

  Changed 3 years ago by wmb@…

Approximate recipe:

: ignore-power-button  ( -- )
   edi-spi-start
   ['] reset-8051 catch if reset-8051 then
   use-ssp-spi
;
: ec-spi-reprogrammed   ( -- )
   edi-spi-start
   unreset-8051
;

' ec-spi-reprogrammed to spi-reprogrammed

  Changed 3 years ago by Quozl

  • next_action changed from design to add to build

Implemented in  svn 2523 and under local test in q4b09jg. Should be in Q4B10.

  Changed 3 years ago by Quozl

  • status changed from assigned to new
  • owner changed from Quozl to greenfeld
  • next_action changed from add to build to test in build

Fixed in q4b10, please test.

Test case: momentarily press and release power button and observe flicker of power indicator, then verify that the flicker does not occur while OpenFirmware Q4B10 is reflashing Q4B10. (It won't pass test if Q4B09 is reflashing to Q4B10.)

  Changed 3 years ago by greenfeld

  • next_action changed from test in build to diagnose

Tried to flash an XO 1.75 B1 with Q4C02 on it to Q4C04 after verifying the power button blinked briefly when the power button was pressed without reflashing.

Pressed the button while Writing was displayed during the update and the power button flashed. Held the button down while writing was still going on and the XO shut down without completing the update of OFW, bricking itself.

I need to know if this was expected or not, but don't think it was.

  Changed 3 years ago by Quozl

Worked fine in Q4B10, but apparently broke some time after that. I've reproduced the flicker during write on Q4C04ja.

Note that the test case did not involve holding the button down. That was intentional. It is sufficient to prove that the LED does not flicker, since the EC always flickers the LED on a power button press if the EC is operating. You should not have proceeded having observed the LED flicker.

(Unrelated to the ticket, it is likely that CForth is intact, and so debricking might only require serial port and rotate button on power up.)

  Changed 3 years ago by Quozl

  • next_action changed from diagnose to add to build

Was fixed in Q4B10, broke in Q4B11 due to svn 2561, fixed in svn 2680, will be included in next release of OpenFirmware.

Developer release available for testing  q4c04jc.rom built 2011-11-10 01:02:38.

There is no need to hold the button down in testing, however if you wish to do so check first that the power LED does not flicker.

As an additional precaution, one may check to see if the fix is present. On Q4C04jc and later, the following output shows the fix is present:

ok see flash-vulnerable(
: flash-vulnerable(   
   ignore-power-button hdd-led-on 
; 
ok 

On Q4C04 and earlier, the following output shows the fix is absent:

ok see flash-vulnerable(
: flash-vulnerable(   
   hdd-led-on 
; 
ok 

This check is relevant only to the firmware version performing the reflashing.

  Changed 3 years ago by Quozl

  • status changed from new to closed
  • next_action changed from add to build to no action
  • resolution set to fixed

Fixed in Q4C05.

Note: See TracTickets for help on using tickets.