| Message ID | 20220125135535.224061-2-grandpaul@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | A/B firmware update based in eMMC boot partition. | expand |
Hi, On 1/25/22 22:55, Ying-Chun Liu wrote: > From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> > > This commit implements write protection for active boot partition for > eMMC. The active boot partition is write protected, and it is still > able to write to the inactive boot partition. It seems that you want to enable Write-protect about boot partition without additional command, right? After initialized eMMC, how to update image into boot partition? Best Regards, Jaehoon Chung > > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Jaehoon Chung <jh80.chung@samsung.com> > --- > drivers/mmc/Kconfig | 10 ++++++++ > drivers/mmc/mmc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 68 insertions(+) > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > index e0927ce1c9..c4bae743e6 100644 > --- a/drivers/mmc/Kconfig > +++ b/drivers/mmc/Kconfig > @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING > This adds a command and an API to do hardware partitioning on eMMC > devices. > > +config MMC_WRITE_PROTECT_ACTIVE_BOOT > + bool "Write protection for active boot partition (eMMC)" > + depends on MMC_HW_PARTITIONING > + default n > + help > + Write protection for active boot partition when mmc initialized. > + This option protects the active boot partition so that it cannot > + be written. But it is still able to write to the inactive boot > + partition. > + > config SUPPORT_EMMC_RPMB > bool "Support eMMC replay protected memory block (RPMB)" > imply CMD_MMC_RPMB > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 4d9871d69f..8620918749 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc) > return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1); > } > > +/** > + * mmc_boot_wp_single_partition() - set write protection to a boot partition. > + * > + * This function sets a single boot partition to protect and leave the > + * other partition writable. > + * > + * @param mmc the mmc device. > + * @param partition 0 - first boot partition, 1 - second boot partition. > + */ > +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition) > +{ > + u8 value; > + int ret; > + > + value = 1; > + > + if (partition == 0) { > + value |= 0x80; > + ret = mmc_switch(mmc, > + EXT_CSD_CMD_SET_NORMAL, > + EXT_CSD_BOOT_WP, > + value); > + } else if (partition == 1) { > + value |= 0x80; > + value |= 0x02; > + ret = mmc_switch(mmc, > + EXT_CSD_CMD_SET_NORMAL, > + EXT_CSD_BOOT_WP, > + value); > + } else { > + ret = mmc_boot_wp(mmc); > + } > + > + return ret; > +} > + > +/** > + * mmc_boot_wp_active_partition() - set write protection to active boot > + * partition. > + * > + * @param mmc the mmc device. > + */ > +static int mmc_boot_wp_active_partition(struct mmc *mmc) > +{ > + u8 part; > + > + if (mmc->part_config == MMCPART_NOAVAILABLE) > + return -EOPNOTSUPP; > + > + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); > + > + return mmc_boot_wp_single_partition(mmc, part - 1); > +} > + > #if !CONFIG_IS_ENABLED(MMC_TINY) > static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode, > bool hsdowngrade) > @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc) > mmc->has_init = 0; > else > mmc->has_init = 1; > + > + if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT)) > + mmc_boot_wp_active_partition(mmc); > + > return err; > } >
Hi Jaehoon, There are 2 boot partitions on eMMC. So the active one is write-protected. Users can write the new firmware to another boot partition (inactive) which is not write-protected. And then switch it on. In U-boot, execute "mmc wp" write-protect all of the boot partitions. Maybe we can add an additional command that just write-protect only one boot partition. Yours, Paul On Thu, 27 Jan 2022 at 07:24, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi, > > On 1/25/22 22:55, Ying-Chun Liu wrote: > > From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> > > > > This commit implements write protection for active boot partition for > > eMMC. The active boot partition is write protected, and it is still > > able to write to the inactive boot partition. > > It seems that you want to enable Write-protect about boot partition > without additional command, right? > After initialized eMMC, how to update image into boot partition? > > Best Regards, > Jaehoon Chung > > > > > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> > > Cc: Peng Fan <peng.fan@nxp.com> > > Cc: Jaehoon Chung <jh80.chung@samsung.com> > > --- > > drivers/mmc/Kconfig | 10 ++++++++ > > drivers/mmc/mmc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 68 insertions(+) > > > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > > index e0927ce1c9..c4bae743e6 100644 > > --- a/drivers/mmc/Kconfig > > +++ b/drivers/mmc/Kconfig > > @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING > > This adds a command and an API to do hardware partitioning on > eMMC > > devices. > > > > +config MMC_WRITE_PROTECT_ACTIVE_BOOT > > + bool "Write protection for active boot partition (eMMC)" > > + depends on MMC_HW_PARTITIONING > > + default n > > + help > > + Write protection for active boot partition when mmc initialized. > > + This option protects the active boot partition so that it cannot > > + be written. But it is still able to write to the inactive boot > > + partition. > > + > > config SUPPORT_EMMC_RPMB > > bool "Support eMMC replay protected memory block (RPMB)" > > imply CMD_MMC_RPMB > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index 4d9871d69f..8620918749 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc) > > return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1); > > } > > > > +/** > > + * mmc_boot_wp_single_partition() - set write protection to a boot > partition. > > + * > > + * This function sets a single boot partition to protect and leave the > > + * other partition writable. > > + * > > + * @param mmc the mmc device. > > + * @param partition 0 - first boot partition, 1 - second boot partition. > > + */ > > +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition) > > +{ > > + u8 value; > > + int ret; > > + > > + value = 1; > > + > > + if (partition == 0) { > > + value |= 0x80; > > + ret = mmc_switch(mmc, > > + EXT_CSD_CMD_SET_NORMAL, > > + EXT_CSD_BOOT_WP, > > + value); > > + } else if (partition == 1) { > > + value |= 0x80; > > + value |= 0x02; > > + ret = mmc_switch(mmc, > > + EXT_CSD_CMD_SET_NORMAL, > > + EXT_CSD_BOOT_WP, > > + value); > > + } else { > > + ret = mmc_boot_wp(mmc); > > + } > > + > > + return ret; > > +} > > + > > +/** > > + * mmc_boot_wp_active_partition() - set write protection to active boot > > + * partition. > > + * > > + * @param mmc the mmc device. > > + */ > > +static int mmc_boot_wp_active_partition(struct mmc *mmc) > > +{ > > + u8 part; > > + > > + if (mmc->part_config == MMCPART_NOAVAILABLE) > > + return -EOPNOTSUPP; > > + > > + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); > > + > > + return mmc_boot_wp_single_partition(mmc, part - 1); > > +} > > + > > #if !CONFIG_IS_ENABLED(MMC_TINY) > > static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode, > > bool hsdowngrade) > > @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc) > > mmc->has_init = 0; > > else > > mmc->has_init = 1; > > + > > + if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT)) > > + mmc_boot_wp_active_partition(mmc); > > + > > return err; > > } > > > >
Hi Paul On 1/27/22 10:23, Paul Liu wrote: > Hi Jaehoon, > > There are 2 boot partitions on eMMC. So the active one is write-protected. > Users can write the new firmware to another boot partition (inactive) which > is not write-protected. > And then switch it on. I know there are two boot partitions. Well, it seems that is for A/B switching scheme, right? > > In U-boot, execute "mmc wp" write-protect all of the boot partitions. > Maybe we can add an additional command that just write-protect only one > boot partition. > > Yours, > Paul > > > On Thu, 27 Jan 2022 at 07:24, Jaehoon Chung <jh80.chung@samsung.com> wrote: > >> Hi, >> >> On 1/25/22 22:55, Ying-Chun Liu wrote: >>> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> >>> >>> This commit implements write protection for active boot partition for >>> eMMC. The active boot partition is write protected, and it is still >>> able to write to the inactive boot partition. >> >> It seems that you want to enable Write-protect about boot partition >> without additional command, right? >> After initialized eMMC, how to update image into boot partition? >> >> Best Regards, >> Jaehoon Chung >> >>> >>> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> >>> Cc: Peng Fan <peng.fan@nxp.com> >>> Cc: Jaehoon Chung <jh80.chung@samsung.com> >>> --- >>> drivers/mmc/Kconfig | 10 ++++++++ >>> drivers/mmc/mmc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 68 insertions(+) >>> >>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig >>> index e0927ce1c9..c4bae743e6 100644 >>> --- a/drivers/mmc/Kconfig >>> +++ b/drivers/mmc/Kconfig >>> @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING >>> This adds a command and an API to do hardware partitioning on >> eMMC >>> devices. >>> >>> +config MMC_WRITE_PROTECT_ACTIVE_BOOT >>> + bool "Write protection for active boot partition (eMMC)" >>> + depends on MMC_HW_PARTITIONING >>> + default n >>> + help >>> + Write protection for active boot partition when mmc initialized. >>> + This option protects the active boot partition so that it cannot >>> + be written. But it is still able to write to the inactive boot >>> + partition. >>> + >>> config SUPPORT_EMMC_RPMB >>> bool "Support eMMC replay protected memory block (RPMB)" >>> imply CMD_MMC_RPMB >>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >>> index 4d9871d69f..8620918749 100644 >>> --- a/drivers/mmc/mmc.c >>> +++ b/drivers/mmc/mmc.c >>> @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc) >>> return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1); >>> } >>> >>> +/** >>> + * mmc_boot_wp_single_partition() - set write protection to a boot >> partition. >>> + * >>> + * This function sets a single boot partition to protect and leave the >>> + * other partition writable. >>> + * >>> + * @param mmc the mmc device. >>> + * @param partition 0 - first boot partition, 1 - second boot partition. >>> + */ >>> +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition) >>> +{ >>> + u8 value; >>> + int ret; >>> + >>> + value = 1; >>> + >>> + if (partition == 0) { >>> + value |= 0x80; Use macro instead of 0x80. >>> + ret = mmc_switch(mmc, >>> + EXT_CSD_CMD_SET_NORMAL, >>> + EXT_CSD_BOOT_WP, >>> + value); >>> + } else if (partition == 1) { >>> + value |= 0x80; >>> + value |= 0x02; Ditto. >>> + ret = mmc_switch(mmc, >>> + EXT_CSD_CMD_SET_NORMAL, >>> + EXT_CSD_BOOT_WP, >>> + value); >>> + } else { >>> + ret = mmc_boot_wp(mmc); >>> + } how aboutusing switch statements? switch (partition) { case 0: value |= 0x80; case 1: value |= 0x02; mmc_switch(); break; default: mmc_boot_wp(); } >>> + >>> + return ret; >>> +} >>> + >>> +/** >>> + * mmc_boot_wp_active_partition() - set write protection to active boot >>> + * partition. >>> + * >>> + * @param mmc the mmc device. >>> + */ >>> +static int mmc_boot_wp_active_partition(struct mmc *mmc) >>> +{ >>> + u8 part; >>> + >>> + if (mmc->part_config == MMCPART_NOAVAILABLE) >>> + return -EOPNOTSUPP; >>> + >>> + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); >>> + >>> + return mmc_boot_wp_single_partition(mmc, part - 1); >>> +} >>> + >>> #if !CONFIG_IS_ENABLED(MMC_TINY) >>> static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode, >>> bool hsdowngrade) >>> @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc) >>> mmc->has_init = 0; >>> else >>> mmc->has_init = 1; >>> + >>> + if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT)) I think that "if (CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT) && !err)" is more better. >>> + mmc_boot_wp_active_partition(mmc); Need to handle error. Best Regards, Jaehoon Chung >>> + >>> return err; >>> } >>> >> >> >
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index e0927ce1c9..c4bae743e6 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING This adds a command and an API to do hardware partitioning on eMMC devices. +config MMC_WRITE_PROTECT_ACTIVE_BOOT + bool "Write protection for active boot partition (eMMC)" + depends on MMC_HW_PARTITIONING + default n + help + Write protection for active boot partition when mmc initialized. + This option protects the active boot partition so that it cannot + be written. But it is still able to write to the inactive boot + partition. + config SUPPORT_EMMC_RPMB bool "Support eMMC replay protected memory block (RPMB)" imply CMD_MMC_RPMB diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 4d9871d69f..8620918749 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc) return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1); } +/** + * mmc_boot_wp_single_partition() - set write protection to a boot partition. + * + * This function sets a single boot partition to protect and leave the + * other partition writable. + * + * @param mmc the mmc device. + * @param partition 0 - first boot partition, 1 - second boot partition. + */ +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition) +{ + u8 value; + int ret; + + value = 1; + + if (partition == 0) { + value |= 0x80; + ret = mmc_switch(mmc, + EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_BOOT_WP, + value); + } else if (partition == 1) { + value |= 0x80; + value |= 0x02; + ret = mmc_switch(mmc, + EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_BOOT_WP, + value); + } else { + ret = mmc_boot_wp(mmc); + } + + return ret; +} + +/** + * mmc_boot_wp_active_partition() - set write protection to active boot + * partition. + * + * @param mmc the mmc device. + */ +static int mmc_boot_wp_active_partition(struct mmc *mmc) +{ + u8 part; + + if (mmc->part_config == MMCPART_NOAVAILABLE) + return -EOPNOTSUPP; + + part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); + + return mmc_boot_wp_single_partition(mmc, part - 1); +} + #if !CONFIG_IS_ENABLED(MMC_TINY) static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode, bool hsdowngrade) @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc) mmc->has_init = 0; else mmc->has_init = 1; + + if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT)) + mmc_boot_wp_active_partition(mmc); + return err; }