After 'import flash', 'flash.erase()' does not appear work on Amigo TFT. #233
Comments
|
Here says that: "SPIFFS is a file system intended for SPI NOR flash devices on embedded targets. It supports wear levelling, file system consistency checks, and more." So I think it is expected multiple copies of the same file throughout all the available space on SPIFFS. Wikipedia says this on NOR flash: "Erasure must happen a block at a time, and resets all the bits in the erased block back to one. Typical block sizes are 64, 128, or 256 KiB.". So I think it is not that simple to erase a NOR flash memory, maybe it is not properly implemented... a way to do this is by saving zeroes or ones throughout all the available space as you did. The point is, why erase if we can just overwrite? Maybe SPIFFS just try to save the firmware on the "next available space", but sometimes this leads to error because there was no "available space" to do the this operation and it is left incomplete, then the next type you save the firmware SPIFFS do it the right way... this a bug can be of SPIFFS or MaixPy implementation of it? I don't know 😞 |
|
@tadeubas , I think SPIFFS is not used to save the firmware, it will only be used on the SPIFFS area, above 0xd00000 @jdlcdl |
|
Thx @odudex for the explanation! For this wipe feature, I think under tools would be just fine. |
It could be, I will certainly take a look; I had not even looked there yet. Thank you for the pointer. |
A "Restore defaults" option under Settings is a good idea and should be its own feature separate from wiping. You might have stored encrypted mnemonics on the device but accidentally changed some settings and just want to return to the defaults, in which case you wouldn't want to wipe out your mnemonics... As for "full wipes," it's unfortunate that we can't do this by default now due to the possibility that mnemonics (or settings) might be stored on the flash. :/ My original vision for Krux was that it would be a stateless device with a readonly filesystem that you could knowingly plug hardware pieces into to gain features like mnemonic storage or settings persistence. For example, a microSD for encrypted mnemonics or microSD for settings with mnemonics on dedicated USB-C devices like Yubikeys. This way, Krux can always choose the most hardcore "wipe" possible on shutdown, and you always know exactly where everything is stored. Hmm. |
|
Your "original vision for Krux" is still avaiable at releases under version 22.08.2. FOSS works that way, if someone want to change something, it changes accordingly to his view, if others like and share the same view, they change too on a specific repo. Lots of forks and versions could be available at the same time and the "original" and or "official" version or view could change with time. Now regarding the feature, I don't see any problem to create a "restore defaults" and "full wipe" of what is connected to the device (flash/SDCard) |
|
Discussion, criticism and questioning is healthy, and with planning and teamwork we can achieve higher gols than we would working independently. Also, something I learned with time is group achievements are far more satisfying than individual ones. Let's make an effort and to try to integrate our ideas and expectations the best possible way. |
"Amen" to that! Each individual has blind-spots, in a team, they have fewer. |
Agreed. Particularly about planning and discussion! ;) You don't want a project run by a group of people who agree on everything, otherwise it's an echo chamber subject to mob rule. To be clear, I can see why the feature is liked, and I'm not saying it should be removed. However, such a big change to the project should have had more discussion around it, particularly all the possible edge cases that would come as a result of a core assumption being violated. I'll take the blame here that I should have been more active and diligent about this before it was added. All I can say is that I honestly didn't have the time to dedicate to this project the past year. Development continued without me or my input, so my only recourse is to retroactively voice my concerns now. Incidentally, this is an issue I have with large PRs that are not focused on one feature: it's difficult to review them adequately or individually discuss each feature contained within, especially when the features intertwine and become dependent on one another. I don't like making someone throw out their hard work, so the result is I have to rubber stamp the whole thing. It's like an omnibus bill in congress. The outcome is suboptimal. Going forward, I'm going to be more strict about forcing adherence to the guidelines for contributing here: https://github.com/selfcustody/krux#contributing Anyway, the reason I don't like having a "Wipe Device" feature is that I think it should be the default behavior. And if we add it as an option, I think it will confuse a lot of users who will suddenly think, "Wait, all the times I used it before it wasn't wiping anything?" I'd rather we open a discussion about the different ways Krux is used, both as stateless and stateful, and see if we can more holistically design the UI around that. In general, I think holistically reviewing the UI/UX of Krux every year or so would be useful to avoid adding too many options and menus and keep Krux simple and unsurprising (another core tenet). Established projects often run into the problem of local maxima optimization due to not taking a step back and thinking from first principles how they would design the project if given a blank slate. |
|
So being objective, @jreesun, do you think the option to save settings and encrypted mnemonics on internal flash should be removed? |
In particular, I think we should remember what made us "Kruxers" (except @jreesun who is the "Adamic Kruxer"). I don't have the technical knowledge of krux codes (my contribution is more in the pedagogical aspect through tools with GUIs). Therefore, I will give my opinion as a user. I fell in love with the project due to a paradigmatic exchange: the airgapped, amnesic and easily accessible tripartition (it would be possible to argue: "It has SeedSigner and SpecterDIY", but so, where I live, it is much easier to have a Sipeed and installing software than assembling the previous ones). And from this I concluded that this was my demand. On the other hand, there are other demands, including from people who have experience with another paradigms (coldcard or trezor, for example) whose seed needs a security component. Krux's initial proposal is that even this would not be necessary. However, everything that is created is transformed. And with Krux it would be no different; there were demands in Telegram groups for the most different implementations, including seed memorization, whose security component is encryption (I personally use and like it stateless).
Would it be the case to have two firmware versions for each device? One stateless and one stateful? An initial setup on device that make it stateless or stateful? I don't know. But I think we should ask the majority of users whether this should remain the case or not (maybe by Cryptpad?). |
|
@jdlcdl I believe I fixed the |
|
Nice! I recall that I had tested erasing 4096 as well as 65536 byte blocks, both with no luck. Since you have left the larger blocksize version unchanged will make for an effective test that your changes, and not something else in the sipeed update, is the solution. I will take a look again. thank you. |
|
The changes affect both sizes(there's also a previous commit) , there was an error with the "if" too. Please check both! |
|
It's working, but actually, so is the larger block size erase, which I didn't expect to work at all since you didn't fix that in your latest commit. I started by confirming that without your updated sipeed_update branch, flash.erase() still fails as it did in the past. After booting firmware with your sipeed_update branch at c5127396:
I wonder if simply updating sipeed's MaixPy solves this without your changes. I do recall seeing some w25qxx changes, but I'll admit that I didn't understand their significance. I did a I did a Summary: Also, I confirm that a reboot is needed to setup spiffs again. My final test session from the usb console below after booting only your first fix w/ MaixPy at 3ff9b4a: and my def all_bytes_are(byte, begin, length, block_size=2**12, verbose=False):
'''
returns True if all bytes in flash are the same as byte, otherwise False
assumes that utils.flash_read() behaves as if imported from Maix
'''
from binascii import hexlify
answer = True
if verbose:
print("Checking if %s bytes of flash at %s are all 0x%s..." % (
length, hex(begin), hexlify(byte).decode()), end='')
bytes_read = 0
while bytes_read < length:
if bytes_read + block_size < length:
if utils.flash_read(begin+bytes_read, block_size) != byte * block_size:
answer = False
break
bytes_read += block_size
else:
if utils.flash_read(begin+bytes_read, length-bytes_read) != byte * (length - bytes_read):
answer = False
break
bytes_read += length - bytes_read
if verbose:
print('.', end='')
if verbose:
print("\nthe %s bytes at %s are %s 0x%s." % (
length, hex(begin), 'ALL' if answer else 'NOT all', hexlify(byte).decode()))
return answer |
|
Maybe part of the last commit is unnecessary, but some is. If you will read a block right after erasing it, and don't wait for the |
Besides it is working I was still thinking on optimizations for this, and using your method from first post I noticed something: The |
|
Also, do you agree this verification could be added directly on the Maixpy erase method (in C), as it probably would be faster than doing it within the python application? |
|
As for the comparison never being true, I have not witnessed that in the past but I only ran this function a few times originally, then twice yesterday. What I was intending for this code to do was to print nothing when a block is already empty (all b'\xff'), else "erased" if the flash.erase actually worked, or "written" if it had to write the bytes the slow-ish way. If you figure out how to reproduce all bytes as b'\xff' and this still doesn't work, I'd love to be able to do it myself. Does it work with smaller block sizes? I do get memory allocation errors often for block sizes 128k and above, gc.collect() helps sometimes. I've never had the same issue with 64k blocks which gc.collect() didn't resolve. I would hope to figure out if flash.erase() is working religiously and then just assume that it will work each time. What I've read of these SPI flash devices is that they're fast and reading and writing, but when it comes to the special erase functions, they're especially efficient (as long as there is a particular function for efficient erase, like "the whole chip" or "4k" or "64k" at a time). I'd hate to start double-guessing and slowly verifying otherwise efficient operations that should just work as expected. If we end up having to verify, I'd agree that in C it should be much faster, I hope it's not necessary with your latest changes. |
|
I've added another def erase_spiffs():
'''
writes 0xff to entire SPIFFS because it appears flash.erase() doesn't work on Amigo TFT
assumes `import flash` origin is selfcustody/MaixPy/components/micropython/port/src/flash
'''
FLASH_SIZE = 2**24
SPIFFS_ADDR = 0xd00000
BLOCK_SIZE = 0x10000
import flash
empty_buf = b'\xff' * BLOCK_SIZE
for address in range(SPIFFS_ADDR, FLASH_SIZE, BLOCK_SIZE):
if flash.read(address, BLOCK_SIZE) == empty_buf:
print('%d bytes at %s already empty' % (BLOCK_SIZE, hex(address)))
continue
flash.erase(address, BLOCK_SIZE)
if flash.read(address, BLOCK_SIZE) == empty_buf:
print('%d bytes at %s erased' % (BLOCK_SIZE, hex(address)))
continue
flash.write(address, empty_buf)
if flash.read(address, BLOCK_SIZE) == empty_buf:
print('%d bytes at %s written w/ 0xff' % (BLOCK_SIZE, hex(address)))
continueUsing the current MaixPy w/ broken flash.erase(), I get... After booting odudex's MaixPy sipeed_update branch at c512739, I get: If we look at the bytes of spiffs (I have a hacky hexdump), before being erased, they look like: ...then it wraps around. After being erased, same would show all empty: My hexdump tool for use at the console: class HexDumpSPIFlash:
'''
hex dump for maixpy 16MB SPI Flash
assumes that utils.flash_read() behaves as if imported from Maix
'''
size = 2**24
def __init__(self, begin=0x0, width=16, lines=16, squeeze=True):
self.cursor = begin
self.configure(width=width, lines=lines, squeeze=squeeze)
def next(self):
pass
def prev(self):
page_size = self.width * self.lines
self.cursor = (self.size + self.cursor - (page_size * 2)) % self.size
def seek(self, address):
self.cursor = address % self.size
def configure(self, width=None, lines=None, squeeze=True):
if type(width) == int and width > 0:
self.width = width
if type(lines) == int and lines > 0:
self.lines = lines
if type(squeeze) == bool:
self.squeeze = squeeze
byte_format = ' '.join([' '.join(['{:02x}']*4)]*(self.width//4))
if self.width % 4:
byte_format = ' '.join([byte_format, ' '.join(['{:02x}']*(self.width%4))])
self.fmt = '{} {} |{}|'.format('{:06x}', byte_format, '{:.1s}'*self.width)
def read(self, update_cursor=False):
def format_record(address, record):
if len(record) == self.width:
return self.fmt.format(
*[address]
+[x for x in record]
+[len(repr(str(chr(x))))==3 and str(chr(x)) or '.' for x in record]
)
else:
return '{:06x} {} [EOR]'.format(
address,
' '.join(['{:02x}'.format(x) for x in record])
)
first = self.cursor
answer, buf, i_buf, line_no, repeats, last_record = [], (None, b''), 0, 0, 0, (None, b'')
while line_no < self.lines:
if i_buf + 1 >= len(buf[1]):
buf = (first, utils.flash_read(first, self.width*self.lines))
i_buf = 0
record = (first, buf[1][i_buf:i_buf+self.width])
if repeats:
if record[1] != last_record[1]:
answer.extend([
'... {:d} squeezed'.format(repeats-1) if repeats>1 else '',
format_record(*last_record),
format_record(*record)
])
repeats = 0
line_no += 3
else:
repeats += 1
else:
if self.squeeze and record[1] == last_record[1]:
repeats += 1
else:
answer.append(format_record(*record))
line_no += 1
i_buf += len(record[1])
first = (first + len(record[1])) % self.size
last_record = record
if repeats:
answer.extend([
'... {:d} squeezed'.format(repeats-1) if repeats>1 else '',
format_record(*record)
])
if update_cursor:
self.cursor = first
return '\n'.join(answer)
def run(self):
def set_lines():
self.configure(lines=int(input('Enter number of lines: ')))
def set_width():
self.configure(width=int(input('Enter number of bytes per line: ')))
def toggle_squeeze():
self.configure(squeeze=not self.squeeze)
def seek():
address = input('Enter an address: ')
if address[:2] == '0b':
address = int(address[2:], 2)
elif address[:2] == '0x':
address = int(address[2:], 16)
else:
address = int(address)
self.seek(address)
repl = {
'j': self.next,
'k': self.prev,
'l': set_lines,
'w': set_width,
's': toggle_squeeze,
'/': seek,
}
print(self.read(update_cursor=True))
while True:
_in = input('\b')
if _in and _in[0] in repl:
repl[_in]()
print(self.read(update_cursor=True))
elif _in == 'q':
return
else: print('Try one of %s or "q" to quit.' % [x for x in repl.keys()]) |
|
Wow, I've been learning a lot with your tools and methods, thank you! |
|
Thank you for that last comment. It makes my exploration into flash feel worth while. I'll admit, my original thoughts when I received my amigo gift were "Wow, 16MB of internal flash? What's that work-out to be, like room for 500k private keys? How can we inspect that and be sure?" I have more hacky scripts, hopefully they'll evolve into something that others can use easily. |
|
Remembering that I'd confirmed a reboot works to setup spiffs space after it's been erased w/ |
|
Can we consider this done in release 24.03.0 ? |
|
Yes, @jdlcdl if there's something else please let us know and we can re-open it. |
This issue is much longer than I initially intended. TL;DR it's not a concerning issue at all, I'm simply sharing.
In my quest to be able to explain every byte on my Amigo's 16MB onboard SPI Flash storage, I was digging into the 3MB of SPIFFS between 0xd00000 and the end of flash. I found many copies of
settings.jsonspread sparsely in this space, as well as some 4-byte markers (spaced 131072 bytes apart) that I assume are formatting. I wanted to start with a clean slate. Whileos.remove()andos.flash_format()work to remove files from SPIFFS, they're certainly not clearing data bytes; I didn't truly expect that they would.I recall that in
src/krux/firmware.pyflash is written viaimport flashwith aflash.erase()step prior toflash.write()here on the develop_rc branch (and that it did so two lines earlier on the main branch) but when I tried to use it to erase blocks in SPIFFS space I had no luck. I didn't know if SPIFFS was silently protecting me from direct access to the bytes owned by that filesystem, so I tried to useflash.write()andflash.read()to verify that it does not. I did the same outside of SPIFFS and had success each time, howeverflash.erase()never worked for me anywhere.I am assuming that
import flashis made possible via MaixPy/components/micropython/port/src/flash and aw25qxx.hheader inMaixPy/components/drivers/flash/include. Reviewing those informed me that I should only be usingflash.erase()for 4096 and 65536 sized blocks at aligned addresses, assrc/krux/firmware.pydoes, but I had already tried this to no avail. I also tried other block sizes and did NOT get any exceptions, which surprised me. Maybe my assumption at the beginning of this paragraph is wrong.I was able to find a related discussion here but it was more about resolving a compile-time error and not about whether
flash.erase()worked or not.Maybe
flash.erase()works, and/or is necessary on some boards, and isn't supported on others? I have found otherw25q*drivers that use the same 0x20 and 0xD8 for erasing 4096 and 65536 sized blocks just as the krux header suggests, as well as some that do not even implement erasing blocks.For my short term goal, I'm assuming that the
flash.erase()step insrc/krux/firmware.pyis doing nothing for my amigo, and that theflash.write()steps are working just fine, so I have resorted to the following to erase my SPIFFS, which I paste and call from the usb console:I've left it here in case others with different boards would like to confirm. If the messages end with "erased", then
flash.erase()indeed is working for some boards.Note: This only works because SPIFFS is the very end of the 16MB flash, if SPIFFS were one byte larger, or moved one byte further, it could overflow into Kboot stage 0 and brick the Amigo until battery discharged, then would require ktool reflash.
In past discussions that were more focused around not bricking devices due to overflow here, It was opined that in regards to SPIFFS overflowing, we might just limit the number of encrypted seeds stored there. Now that I have looked and found SPIFFS to be sparse and cluttered w/ old data, and with formatting markers throughout, I suspect that SPIFFS is respectful of its 0x300000 size and that this should not be a concern -- assuming that krux devs are respectful that SPIFFS address + size does not overrun 16MB.
The text was updated successfully, but these errors were encountered: