Copy magic to RAM before writing to flash

[mikevoyt]

Description

Some architectures (e.g., EasyDMA on nRF52 and nRF91) cannot
DMA data directly from internal flash to an external SPI flash.
An -EIO error is returned if this is attempted.

For nRF52 with QSPI, this limitation can be avoided by using
a write buffer; i.e., enabling CONFIG_NORDIC_QSPI_NOR_STACK_WRITE_BUFFER_SIZE
will first copy the data to a buffer in RAM before DMA'ing.

However, with the nRF91 for example, QSPI is not available, and the
generic NOR SPI driver does not support a write buffer.

This change allows writing the magic to flash, regardless of whether
or not a write buffer is enabled or available. It also has negligible impact
on other architectures, as it is simply copying 4 words to RAM.

Note that it is not possible to support the secondary image on external SPI flash
on the nRF91 without this change.

How was this tested?

Tested on an nRF91 with a W25Q64 external SPI flash. The following config was enabled
in order to use the external SPI flash for the mcuboot_secondary slot:

CONFIG_PM_EXTERNAL_FLASH_MCUBOOT_SECONDARY=y

A FOTA was performed. Before the change, the flash_area_write() call in boot_write_magic() would fail with an -EIO error, due to spi_nor_cmd_addr_write() failing in spi_nor.c.

After this change, the magic was successfully written and FOTA completed successfully.

[nvlsianpu]

@mikevoyt Your patch looks sane.
I think It would be better to redirect it to the upstream which is https://github.com/mcu-tools/mcuboot. It can be noted that it is kind of workaround (in the commit message).

We will inherit the patch form the upstream which is the right way for upgrading of common code.

[mikevoyt]

@mikevoyt Your patch looks sane. I think It would be better to redirect it to the upstream which is https://github.com/mcu-tools/mcuboot. It can be noted that it is kind of workaround (in the commit message).

We will inherit the patch form the upstream which is the right way for upgrading of common code.

Thanks for taking a look @nvlsianpu - I took a look at upstream mcuboot, and actually it appears they are now already copying the magic into RAM first before writing the magic (for another reason), so I suppose this change will eventually be picked up in sdk-mcuboot?:

int
boot_write_magic(const struct flash_area *fap)
{
    uint32_t off;
    uint32_t pad_off;
    int rc;
    uint8_t magic[BOOT_MAGIC_ALIGN_SIZE];
    uint8_t erased_val;

    off = boot_magic_off(fap);

    /* image_trailer structure was modified with additional padding such that
     * the pad+magic ends up in a flash minimum write region. The address
     * returned by boot_magic_off() is the start of magic which is not the
     * start of the flash write boundary and thus writes to the magic will fail.
     * To account for this change, write to magic is first padded with 0xFF
     * before writing to the trailer.
     */
    pad_off = ALIGN_DOWN(off, BOOT_MAX_ALIGN);

    erased_val = flash_area_erased_val(fap);

    memset(&magic[0], erased_val, sizeof(magic));
    memcpy(&magic[BOOT_MAGIC_ALIGN_SIZE - BOOT_MAGIC_SZ], BOOT_IMG_MAGIC, BOOT_MAGIC_SZ);

    BOOT_LOG_DBG("writing magic; fa_id=%d off=0x%lx (0x%lx)",
                 flash_area_get_id(fap), (unsigned long)off,
                 (unsigned long)(flash_area_get_off(fap) + off));
    rc = flash_area_write(fap, pad_off, &magic[0], BOOT_MAGIC_ALIGN_SIZE);

    if (rc != 0) {
        return BOOT_EFLASH;
    }

    return 0;
}

[nvlsianpu]

@mikevoyt That great, I didn't remember about that patch. Yes It will be incorporated here within next upmerge.

[mikevoyt]

@nvlsianpu cool, I will close out this PR with the assumption that this issue is resolved in the main mcuboot branch.