[RFC 1/2] drivers: mmc: write protect active boot area after mmc init.

[Jaehoon Chung]

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 at samsung.com> wrote:
> 
>> Hi,
>>
>> On 1/25/22 22:55, Ying-Chun Liu wrote:
>>> From: "Ying-Chun Liu (PaulLiu)" <paul.liu at 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 at linaro.org>
>>> Cc: Peng Fan <peng.fan at nxp.com>
>>> Cc: Jaehoon Chung <jh80.chung at 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;
>>>  }
>>>
>>
>>
>