1. 19 Mar, 2019 5 commits
  2. 18 Mar, 2019 6 commits
    • Evan Lojewski's avatar
      mboxd: Add support for file-backed flash devices. · a042978b
      Evan Lojewski authored
      
      This commit adds a new file based backing source.  The file based
      backing source takes a raw pnor image that is usually flashed to a mtd
      device.
      
      This backing source enabled rapid testing of pnor images.
      
      Tested on Witherspoon with the VPNOR and file backends, and Romulus for
      the MTD and file backends.
      
      Change-Id: I253ecfa6b58d071c7982f153ad50da8e4ad39fa2
      Signed-off-by: default avatarEvan Lojewski <github@meklort.com>
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      a042978b
    • Andrew Jeffery's avatar
      mboxd: Remove flash API compatibility shim · 0297e5b8
      Andrew Jeffery authored
      
      The flash API compatibility was kept to reduce the line noise in the
      previous backend patch. Remove the compatibility layer now and convert
      the remaining call-sites.
      
      Change-Id: I4b6e54f4463059a7804918add81e7572db7b7c21
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      0297e5b8
    • Evan Lojewski's avatar
      mboxd: Add a backend abstraction layer to mboxd. · f1e547c7
      Evan Lojewski authored
      
      Introduce a backend abstraction, enabling multiple implementations to be
      compiled in at once. This change formally abstracts the two existing
      backends, mtd and vpnor.
      
      With the backend abstraction in place, subsequent backends are easier to
      implement.
      
      This change is based of Evan's work and he retains authorship credit. I
      (AJ) have reworked the patch to pass the vpnor tests, refactored some
      parts to enable broader use of const structures and others to clarify
      the initialisation sequences.
      
      Due to the existing lack of abstraction the patch has unfortunately
      wide-ranging impacts. I've whittled it down as much as I consider
      reasonable.
      
      Change-Id: I29984a36dae4ea86ec00b853d2a756f0b9afb3ec
      Signed-off-by: default avatarEvan Lojewski <github@meklort.com>
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      f1e547c7
    • Andrew Jeffery's avatar
      flash: Introduce flash_validate() · cb93504e
      Andrew Jeffery authored
      
      Clean up the protocol_negotiate_version() mess. The existing approach
      came about due to viewing the vpnor implementation as an edge case in
      its own right. The code becomes much neater if we consider all backends
      as equal and afford them the callbacks necessary for correct behaviour.
      
      Change-Id: Ifaeee9da459818cf22b2f137ddc5b8d0356b9be9
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      cb93504e
    • Andrew Jeffery's avatar
      protocol: Introduce protocol_reset() · f69760da
      Andrew Jeffery authored
      
      protocol_reset() encapsulates the actions necessary to return the LPC
      state to what's required to boot the host. This is backend dependent;
      for the mtd backend we can simply point the bridge at the host flash
      AHB mapping, and for the virtual pnor we want to rearrange the content
      of the LPC reserved memory (leaving the bridge pointed there). In either
      case the state of the FWH address space is distured, so inform the host
      as necessary.
      
      Change-Id: Ie8efd1f703a3616c33f76f4e735c1efea039146c
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      f69760da
    • Andrew Jeffery's avatar
      vpnor: Test if HBB placement exceeds reserved memory bounds · 4e75a27a
      Andrew Jeffery authored
      
      If a host firmware image is provided where the placement of HBB exceeds
      the reserved memory size then an out-of-bounds write would occur.
      
      Change-Id: I0a98cb7417511cc8dd5bd2e12c9232ebc912dcd6
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      4e75a27a
  3. 15 Mar, 2019 4 commits
  4. 14 Feb, 2019 1 commit
  5. 13 Feb, 2019 1 commit
  6. 08 Jan, 2019 4 commits
    • Andrew Jeffery's avatar
      transport: dbus: Remove ProtocolReset and WindowReset signals · 9ed627ca
      Andrew Jeffery authored
      
      These are replaced by the equivalent properties.
      
      Change-Id: Ie2acb98cc592c0ed1f2039f8aa570f1c7944b1e2
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      9ed627ca
    • Andrew Jeffery's avatar
      vpnor: Enforce a read-only FFS ToC regardless of flags · 7a85d22a
      Andrew Jeffery authored
      
      The virtual PNOR implementation never properly honoured writes to the
      FFS ToC, as the ToC's binary representation is generated from the CSV
      file shipped in the PNOR squashfs image.
      
      What *did* happen was that opening a write window to the ToC region
      succeeded and writes could be flushed, but the flushed writes were never
      read, and ToC representation internal to mboxd was never updated to
      match the written state. Thus the written values "persisted" until the
      ToC's window fell out of the cache (with 64MiB reserved regions,
      probably on a host reboot).
      
      Short circuit the insanity of handling FFS more than we have to by
      forcefully marking the ToC as read-only, regardless of the flag
      configuration shipped in the CSV representation. This prevents the host
      from successfully opening a write window and thus the host can have no
      expectation of write persistence.
      
      Change-Id: Ib2788c56b245da506cb7d607c0758b17785766cf
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      7a85d22a
    • Andrew Jeffery's avatar
      vpnor: test: Add force_readonly_toc · 8a0efd5e
      Andrew Jeffery authored
      
      This ensures that the ToC presented to the host indicates that it is not
      writable. The virtual PNOR implementation has never properly honoured
      writes to the ToC, so lets at least tell the host.
      
      As the code has not yet been fixed to implement the desired behaviour,
      add the test to XFAIL_TESTS.
      
      Change-Id: Ia13a0f907f916d6dec3979b17685d54bc578a106
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      8a0efd5e
    • Andrew Jeffery's avatar
      vpnor: Add write-to-writable-ToC test · 89985754
      Andrew Jeffery authored
      
      Tests if mutations survive re-opening the window. They don't, so add the
      test to XFAIL_TESTS.
      
      Change-Id: Ic2f844c30a7da35033bf03012ea452718d2843e4
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      89985754
  7. 17 Dec, 2018 1 commit
  8. 26 Nov, 2018 1 commit
    • Andrew Jeffery's avatar
      protocol: Rework publication of events over DBus transport · fd4fa34d
      Andrew Jeffery authored
      
      A set of races was discovered around the propagation of HIOMAP protocol
      BMC status events during BMC shutdown. In particular the change impacts
      the design of the DBus transport defined in the protocol specification,
      as signalling of both acknowledgeable and non-acknowledgeable events
      could not be made atomic.
      
      A particular case where this matters is when the daemon is terminated,
      at which point it should simultaneously clear BMC_EVENT_DAEMON_READY and
      set BMC_EVENT_PROTOCOL_RESET. The DBus interface as designed required
      this be done as two separate messages, which lead to races propagating
      the complete state update to the host during shutdown of ipmid.
      
      Change-Id: Iaf38f77c28b8e4e4dd092b0de97dc7e777bfac65
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      fd4fa34d
  9. 08 Nov, 2018 1 commit
  10. 07 Nov, 2018 3 commits
    • Andrew Jeffery's avatar
      mboxd: Mark the protocol as reset on shutdown · fab672bd
      Andrew Jeffery authored
      
      This is necessary for the host firmware to properly recover from a
      daemon restart event, as it needs to re-perform the GET_INFO handshake
      and re-establish any window it had active prior to the daemon
      restarting.
      
      While we're here, rename the symbol to align with the documentation.
      
      Change-Id: I628d2ee5972177b7ad78392a86122d16104e7011
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      fab672bd
    • Andrew Jeffery's avatar
      mboxd: Broadcast the daemon is ready on all transports · fe0c9e86
      Andrew Jeffery authored
      
      The code as it stood only sent the state update at startup on the active
      transport, which is somewhat arbitrarily chosen as an implementation
      detail of the mbox initialisation function.
      
      If the host firmware is using IPMI, it will not learn of the update
      unless it attempts to contact mboxd, which it won't do if it knows the
      daemon isn't there, which it may have learned of by receiving a state
      update from the daemon's shutdown path. In this circumstance the host
      firmware is now stuck.
      
      Relieve the host firmware of this problem by always sending the daemon
      state on all supported transports. To avoid some insanity we introduce a
      new callback in struct transport_ops that allows use to send the BMC's
      entire event state rather than just set or clear updates.
      
      Change-Id: I094ff4089eeebd8be99fbd343b94f7bbef023fb1
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      fe0c9e86
    • Andrew Jeffery's avatar
      transport: Fix event handling · f62601b8
      Andrew Jeffery authored
      
      Events were not quite being handled as per the intent of the recent
      refactor: The protocol layer was meant to record the raw set of events
      and provide the protocol-version-specific mask to the transport layer,
      which the transport layer would then use to flush out the state in
      accordance with its implementation-specific requirements.
      
      What was going wrong was that the transport implementations were
      overwriting the raw set of events with the protocol-specific masked set
      of events, meaning that we'd lose the raw state and provide an
      incomplete BMC state value on protocol upgrade.
      
      Rework the event handling to make sure the responsibilities are properly
      split between the layers.
      
      Change-Id: Iace6615a121e4ce7dcca690d9adf62e5ab9ccee2
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      f62601b8
  11. 01 Nov, 2018 1 commit
  12. 12 Oct, 2018 3 commits
  13. 14 Sep, 2018 9 commits
    • Andrew Jeffery's avatar
      Documentation: Rewrite protocol specification · 93c8edc9
      Andrew Jeffery authored
      
      The rewrite addresses two main issues in the original documentation:
      
      1. The conflation of the protocol specification with the mailbox transport
      2. Formatting and discoverability of command and event definitions
      
      Additionally, the rewrite documents two new transports - IPMI and DBus.
      It's noted that DBus is intended as a transport internal to the BMC,
      while the IPMI transport is the new transport exposed to the host.
      
      Finally, some commands and events have been renamed, however this has no
      impact on the behaviour of the protocol.
      
      Change-Id: Icc78141f4ead4395e8a348b80443cadd2300a751
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      93c8edc9
    • Andrew Jeffery's avatar
      vpnor: pnor_partition: Debugging for Request.fulfil() · cc650618
      Andrew Jeffery authored
      
      Change-Id: If74d8d1983c2b32b3ed6264bbdb7f49118d5a1f2
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      cc650618
    • Andrew Jeffery's avatar
      control: Add FIXME in reset handling · cda29646
      Andrew Jeffery authored
      
      windows_reset_all() doesn't perform a flush of the currently open
      window like the comment suggests. Iron out whether the comment or the
      behaviour is incorrect.
      
      Change-Id: Id10384651c02e397f4c8d2a749b66f8881bfe08b
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      cda29646
    • Andrew Jeffery's avatar
      test: Add windows_equally_evictable · c84b8f8f
      Andrew Jeffery authored
      
      Tests for the condition where the minimum age is less than any window's
      current age, in which case we can get a NULL dereference if the
      windows_reset_all() implementation is broken.
      
          windows.c:409:23: runtime error: member access within null pointer of type 'struct window_context'
          ASAN:DEADLYSIGNAL
          =================================================================
          ==31400==ERROR: AddressSanitizer: SEGV on unknown address 0x00000004 (pc 0x0002b658 bp 0x74c00270 sp 0x7eb7c678 T0)
          ==31400==The signal is caused by a WRITE memory access.
          ==31400==Hint: address points to the zero page.
              #0 0x2b657 in window_reset windows.c:410
              #1 0x2cc9b in windows_create_map windows.c:572
              #2 0x1f3f3 in protocol_v1_create_window protocol.c:167
              #3 0x2121b in protocol_v2_create_window protocol.c:417
              #4 0x24cd7f in generic_vpnor_create_window vpnor/protocol.cpp:51
              #5 0x24d053 in protocol_v2_vpnor_create_window vpnor/protocol.cpp:63
              #6 0x2663b in mbox_handle_create_window transport_mbox.c:282
              #7 0x276db in handle_mbox_req transport_mbox.c:568
              #8 0x276db in transport_mbox_dispatch transport_mbox.c:649
              #9 0x17fcb in poll_loop mboxd.c:185
              #10 0x17fcb in main mboxd.c:423
              #11 0x46b68517 in __libc_start_main (/lib/libc.so.6+0x46b68517)
      
          AddressSanitizer can not provide additional info.
          SUMMARY: AddressSanitizer: SEGV windows.c:410 in window_reset
          ==31400==ABORTING
      
      Change-Id: I8161e2ea17953e196d4bb3ca90d19e44ec10c86d
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      c84b8f8f
    • Andrew Jeffery's avatar
      windows: Include stdbool.h · b5fd0a47
      Andrew Jeffery authored
      
      windows_reset_all() returns a boolean value, so make sure we don't place
      annoying requirements on the users.
      
      Change-Id: I0f9a45256e2236c0a76696e2cc8a2df2ad0a98bd
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      b5fd0a47
    • Andrew Jeffery's avatar
      transport: dbus: Implement erase · ea5400f5
      Andrew Jeffery authored
      
      Change-Id: Ic10ba2be78deb6307af195fc01daceb19a82ef95
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      ea5400f5
    • Andrew Jeffery's avatar
      transport: dbus: Implement flush · d66af2ab
      Andrew Jeffery authored
      
      Change-Id: If34f2f6cac002fae417d99baec8c08f675cdb974
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      d66af2ab
    • Andrew Jeffery's avatar
      transport: dbus: Implement mark_dirty · 40ef67be
      Andrew Jeffery authored
      
      Change-Id: I989d5e2a65950cb936fca77d28f7a540d3ec90a2
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      40ef67be
    • Andrew Jeffery's avatar
      transport: dbus: Implement close · c2cbb3d8
      Andrew Jeffery authored
      
      Change-Id: I9f261b56fe66e56d71c069d63f5caba5394cbe12
      Signed-off-by: Andrew Jeffery's avatarAndrew Jeffery <andrew@aj.id.au>
      c2cbb3d8