Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial ADL-S internal programmer support #1

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

mkopec
Copy link
Member

@mkopec mkopec commented Apr 13, 2022

So far only Z690 chipset PCI ID has been added.

Signed-off-by: Michał Kopeć michal.kopec@3mdeb.com

Change-Id: Ifcb3a63a2386b9622e1e4bf3f77f0334136686fe
Signed-off-by: Michał Kopeć <michal.kopec@3mdeb.com>
@mkopec mkopec requested a review from miczyg1 April 13, 2022 13:27
@mkopec mkopec self-assigned this Apr 13, 2022
@miczyg1
Copy link

miczyg1 commented Apr 13, 2022

@miczyg1
Copy link

miczyg1 commented Apr 13, 2022

Well I will have to update it.

@miczyg1 miczyg1 merged commit bbb599e into dasharo-develop Apr 13, 2022
@miczyg1 miczyg1 deleted the alder_lake_s branch April 13, 2022 15:51
@macpijan
Copy link

We need in on Dasharo in the end. Also to include in the dts

SergiiDmytruk pushed a commit that referenced this pull request Jul 26, 2022
This change addresses the following ASAN error detected in the chromium
tree:

 * ASAN error detected:
 * =================================================================
 * ==12==ERROR: AddressSanitizer: global-buffer-overflow on address
0x55a8a046c916 at pc 0x55a8a038a21d bp 0x7ffd5dbc9ed0 sp 0x7ffd5dbc9ec8
 * READ of size 2 at 0x55a8a046c916 thread T0
 *     #0 0x55a8a038a21c in nicrealtek_init /build/amd64-generic/tmp/por
tage/sys-apps/flashrom-9999/work/flashrom-9999-build/../flashrom-9999/ni
crealtek.c:119:15
 *     #1 0x55a8a032f172 in __sanitizer::BufferedStackTrace::UnwindImpl(
unsigned long, unsigned long, void*, bool, unsigned int) ??:0:0
 *     #2 0x55a8a02b65b8 in __asan::ErrorGeneric::Print() ??:0:0
 *     #3 0x55a8a03294d5 in __asan::ScopedInErrorReport::~ScopedInErrorR
eport() ??:0:0
 *     #4 0x55a8a032c5ae in __asan::ReportGenericError(unsigned long, un
signed long, unsigned long, unsigned long, bool, unsigned long, unsigned
 int, bool) ??:0:0
 *     #5 0x55a8a032d0f7 in __asan_report_load2 ??:0:0
 *
 * 0x55a8a046c916 is located 18 bytes to the right of global variable 'm
ock_pci_dev' defined in '../flashrom-9999/tests/tests.c:50:16' (0x55a8a0
46c900) of size 4
 * SUMMARY: AddressSanitizer: global-buffer-overflow (/tmp/portage/sys-a
pps/flashrom-9999/work/flashrom-9999-build/tests/flashrom_unit_tests+0x1
9a21c)
 * Shadow bytes around the buggy address:
 *   0x0ab5940858d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *   0x0ab5940858e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *   0x0ab5940858f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *   0x0ab594085900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *   0x0ab594085910: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9 00 00
 * =>0x0ab594085920: 04 f9[f9]f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 *   0x0ab594085930: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 *   0x0ab594085940: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 *   0x0ab594085950: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 *   0x0ab594085960: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 *   0x0ab594085970: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
 * Shadow byte legend (one shadow byte represents 8 application bytes):
 *   Addressable:           00
 *   Partially addressable: 01 02 03 04 05 06 07
 *   Heap left redzone:       fa
 *   Freed heap region:       fd
 *   Stack left redzone:      f1
 *   Stack mid redzone:       f2
 *   Stack right redzone:     f3
 *   Stack after return:      f5
 *   Stack use after scope:   f8
 *   Global redzone:          f9
 *   Global init order:       f6
 *   Poisoned by user:        f7
 *   Container overflow:      fc
 *   Array cookie:            ac
 *   Intra object redzone:    bb
 *   ASan internal:           fe
 *   Left alloca redzone:     ca
 *   Right alloca redzone:    cb
 * ==12==ABORTING

BUG=b:224828279
TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom
BRANCH=none

Signed-off-by: Daniel Campello <campello@chromium.org>
Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8
Reviewed-on: https://review.coreboot.org/c/flashrom/+/62845
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
macpijan pushed a commit that referenced this pull request Aug 9, 2024
Waiting a full second is a very long time, especially when
default_delay() chooses to busy-loop. This code has been around for a
decade, with vague references to user reports:

  commit 8ab49e7
  Date:   Wed Aug 19 13:55:34 2009 +0000
  Disallow erase/write for known bad chips so people won't try without a clear understanding

Still, this logic does not belong in the high-level library logic, used
by all programmers and all chips. If there is a timing issue, it should
either be encoded in the appropriate programmer or flashchip timing.
However, we don't really know what chips were in use, as the above
commit doesn't have any links to reports. So in a feeble attempt at
avoiding breaking users here, we also surmise that...

 * SPI chips weren't all that common in 2009;
 * I'm mostly motivated by flashrom performance on Chromebooks, were SPI
   chips (and linux_mtd / BUS_PROG) are the rule; and
 * SPI chips have precise timing requirements and an appropriate BUSY
   status. So we guess that this "calm down" magic delay wouldn't be
   necessary there.

Thus, we allow this magic delay only on non-SPI (and non-BUG_PROG, used
by linux_mtd for one) buses as a compromise.

Now, this change has some (hopefully [1] tiny) chance of regression, so
we have the following considerations:

1. emergency_help_message() already provides documentation on how to
   contact support, in case we need to handle any user-reported
   regressions.
2. If there is any regression here, it's only in the --verify code; so
   we can always provide workarounds for testing this, to determine
   whether this change may have been at fault. For example, something
   like:

     flashrom --write /my/new/image.bin --noverify
     sleep 1
     flashrom --read /tmp/bar.bin
     cmp /my/new/image.bin /tmp/bar.bin

   If such problems occur, we can collect system/programmer/chip info to
   try to encode a more targeted delay into the appropriate
   chip/programmer implementation, and avoid penalizing the entire
   project like this.
3. We already have (embedded in erase_write()) erase verification that
   performs no such delay. So depending on the type of timing error that
   this delay was attempting to cover, we may have some proof that this
   delay is no longer necessary (or at least, that whatever systems were
   needing this delay in the first place are no longer caring about
   flashrom).
4. We've retained the delay for buses that were likely common in 2009
   (per the above "feeble attempt").

NB: I avoid using the BUS_NONSPI macro, because I want to exclude any
future buses from this workaround, even in the event that the BUS_NONSPI
category grows in the future.

[1] Famous last words.

BUG=b:325539928
TEST=`flashrom_tester --flashrom_binary=$(which flashrom) \
      internal Erase_and_Write Fail_to_verify`,
TEST=`vpd -i RW_VPD -s foo=bar; vpd -i RW_VPD -l; \
      vpd -i RW_VPD -d foo; vpd -i RW_VPD -l`
TEST=`elogtool list; elogtool add 0xa7; elogtool list`

  on (at least) 2 systems:
   #1: Kukui/Kakadu rev2 - MTD programmer /
       kernel 5.10.215-24542-g0515a679eb42 /
       CrOS ~ 15857.0.0
   #2: Zork/Dirinboz rev2 -
       chip name: vendor="Winbond" name="W25Q128.JW.DTR" /
       BIOS: Google_Dirinboz.13434.688.0 /
       kernel 5.4.267-21940-g67f70e251a74 /
       CrOS ~ 15753.43.0

Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/80807
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants