Skip to content

Commit

Permalink
Respect write and format flags on block device
Browse files Browse the repository at this point in the history
Default to writeable and formattable, but override based on bdev flags, or partition NSBOOT permissions
  • Loading branch information
will-v-pi committed Dec 13, 2024
1 parent 9dc87f0 commit 0e2ec19
Showing 1 changed file with 33 additions and 11 deletions.
44 changes: 33 additions & 11 deletions main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4023,7 +4023,8 @@ uint32_t get_family_id(uint8_t file_idx) {
}

#if HAS_LIBUSB
std::shared_ptr<vector<tuple<uint32_t, uint32_t>>> get_partitions(picoboot::connection &con) {
// Returns [(start, end, permissions)]
std::shared_ptr<vector<tuple<uint32_t, uint32_t, uint32_t>>> get_partitions(picoboot::connection &con) {
picoboot_memory_access raw_access(con);
if (get_model(raw_access) != rp2350) {
// Not an rp2350, so no partitions
Expand Down Expand Up @@ -4052,7 +4053,7 @@ std::shared_ptr<vector<tuple<uint32_t, uint32_t>>> get_partitions(picoboot::conn
resident_partition_t unpartitioned = *(resident_partition_t *) &loc_flags_id_buf_32[lfi_pos];
lfi_pos += 2;

vector<tuple<uint32_t, uint32_t>> ret;
vector<tuple<uint32_t, uint32_t, uint32_t>> ret;

if (!has_pt || !partition_count) {
// there is no partition table, or it is empty
Expand All @@ -4064,14 +4065,16 @@ std::shared_ptr<vector<tuple<uint32_t, uint32_t>>> get_partitions(picoboot::conn
for (unsigned int i = 0; i < partition_count; i++) {
uint32_t location_and_permissions = loc_flags_id_buf_32[lfi_pos++];
uint32_t flags_and_permissions = loc_flags_id_buf_32[lfi_pos++];
uint32_t permissions = location_and_permissions & flags_and_permissions & PICOBIN_PARTITION_PERMISSIONS_BITS;
uint64_t id;
if (flags_and_permissions & PICOBIN_PARTITION_FLAGS_HAS_ID_BITS) {
id = loc_flags_id_buf_32[lfi_pos] | ((uint64_t) loc_flags_id_buf_32[lfi_pos + 1] << 32u);
lfi_pos += 2;
}
ret.push_back(std::make_tuple(
((location_and_permissions >> PICOBIN_PARTITION_LOCATION_FIRST_SECTOR_LSB) & 0x1fffu) * 4096,
(((location_and_permissions >> PICOBIN_PARTITION_LOCATION_LAST_SECTOR_LSB) & 0x1fffu) + 1) * 4096
(((location_and_permissions >> PICOBIN_PARTITION_LOCATION_LAST_SECTOR_LSB) & 0x1fffu) + 1) * 4096,
permissions
));
if ((location_and_permissions ^ flags_and_permissions) &
PICOBIN_PARTITION_PERMISSIONS_BITS) {
Expand All @@ -4081,7 +4084,7 @@ std::shared_ptr<vector<tuple<uint32_t, uint32_t>>> get_partitions(picoboot::conn
}
}

return std::make_shared<vector<tuple<uint32_t, uint32_t>>>(ret);
return std::make_shared<vector<tuple<uint32_t, uint32_t, uint32_t>>>(ret);
}
#endif

Expand Down Expand Up @@ -5332,6 +5335,8 @@ struct _lfs_setup {
std::shared_ptr<memory_access> access;
uint32_t base_addr;
uint32_t size;
bool writeable = true;
bool formattable = true;
};
_lfs_setup lfs_setup;

Expand All @@ -5341,13 +5346,23 @@ int lfs_read(const struct lfs_config *c, lfs_block_t block, lfs_off_t off, void
}

int lfs_prog(const struct lfs_config *c, lfs_block_t block, lfs_off_t off, const void *buffer, lfs_size_t size) {
lfs_setup.access->write(lfs_setup.base_addr + (block * c->block_size) + off, (uint8_t*)buffer, size);
return LFS_ERR_OK;
if (lfs_setup.writeable) {
lfs_setup.access->write(lfs_setup.base_addr + (block * c->block_size) + off, (uint8_t*)buffer, size);
return LFS_ERR_OK;
} else {
fail(ERROR_NOT_POSSIBLE, "This block device is not writeable");
return LFS_ERR_IO;
}
};

int lfs_erase(const struct lfs_config *c, lfs_block_t block) {
lfs_setup.con->flash_erase(lfs_setup.base_addr + (block * c->block_size), c->block_size);
return LFS_ERR_OK;
if (lfs_setup.writeable) {
lfs_setup.con->flash_erase(lfs_setup.base_addr + (block * c->block_size), c->block_size);
return LFS_ERR_OK;
} else {
fail(ERROR_NOT_POSSIBLE, "This block device is not writeable");
return LFS_ERR_IO;
}
};

int lfs_sync(const struct lfs_config *c) {
Expand Down Expand Up @@ -5419,6 +5434,7 @@ void do_lfs_op(device_map &devices, lfs_op_fn lfs_op) {
}
uint32_t start = std::get<0>((*partitions)[settings.bdev.partition]);
uint32_t end = std::get<1>((*partitions)[settings.bdev.partition]);
lfs_setup.writeable = std::get<2>((*partitions)[settings.bdev.partition]) & PICOBIN_PARTITION_PERMISSION_NSBOOT_W_BITS;
start += FLASH_START;
end += FLASH_START;
if (end <= start) {
Expand All @@ -5444,6 +5460,8 @@ void do_lfs_op(device_map &devices, lfs_op_fn lfs_op) {
if (bi_bdev.flags & BINARY_INFO_BLOCK_DEV_FLAG_READ) ss << "r";
if (bi_bdev.flags & BINARY_INFO_BLOCK_DEV_FLAG_WRITE) ss << "w";
if (bi_bdev.flags & BINARY_INFO_BLOCK_DEV_FLAG_REFORMAT) ss << "f";
lfs_setup.writeable = bi_bdev.flags & BINARY_INFO_BLOCK_DEV_FLAG_WRITE;
lfs_setup.formattable = bi_bdev.flags & BINARY_INFO_BLOCK_DEV_FLAG_REFORMAT;
}
string s = ss.str();
fos << "embedded drive: " << s << "\n";
Expand Down Expand Up @@ -5486,9 +5504,13 @@ void do_lfs_op(device_map &devices, lfs_op_fn lfs_op) {
int err = lfs_mount(&lfs, &cfg);
if (err == LFS_ERR_CORRUPT) {
if (settings.bdev.format) {
fos << "Formatting LittleFS file system\n";
lfs_format(&lfs, &cfg);
lfs_mount(&lfs, &cfg);
if (lfs_setup.formattable) {
fos << "Formatting LittleFS file system\n";
lfs_format(&lfs, &cfg);
lfs_mount(&lfs, &cfg);
} else {
fail(ERROR_NOT_POSSIBLE, "This block device is not formattable");
}
} else {
fail(ERROR_CONNECTION, "LittleFS file system is corrupted - add -f flag to format it (this may result in data loss)");
}
Expand Down

3 comments on commit 0e2ec19

@lurch
Copy link
Contributor

@lurch lurch commented on 0e2ec19 Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for a partition to be writeable but not readable? Does that need to be taken into account too, or is that irrelevant for picotool?

@will-v-pi
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for a partition to be writeable but not readable? Does that need to be taken into account too, or is that irrelevant for picotool?

Technically yes, but that doesn't need to be accounted for because if it's not readable then you can't read the filesystem information from it, and therefore picotool will just throw a permissions error whenever you try to use any of the bdev commands.

@lurch
Copy link
Contributor

@lurch lurch commented on 0e2ec19 Dec 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, that's a good point - if the partition is non-readable, and it has a filesystem on it, then there's no way of knowing which blocks need to be written, in order to update the filesystem! 😂

Please sign in to comment.