[PATCH] fastboot: Re-implement erase according to spec

[Sam Protsenko]

Hi Eugeniu

On Tue, Jan 14, 2020 at 8:30 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hi Sam,
>
> [Relaying the same message to your gmail address]
> [Cc-ing Peng, as U-Boot MMC maintainer]
> [Cc-ing Faiz, as the one who already reviewed the patch]
>
> On Thu, Jan 09, 2020 at 08:47:15PM +0200, Sam Protsenko wrote:
> > Fastboot specification [1] requires MMC to be filled with 0xFF's on
> > "fastboot erase" command:
> >
> >     erase:%s           Erase the indicated partition (clear to 0xFFs)
> >
> > Current "fastboot erase" implementation uses actual MMC erase operation
> > blk_derase(), which can fill MMC either with 0x00 or 0xFF, depending on
> > used MMC controller; from [2]:
> >
> >     The content of an explicitly erased memory range shall be '0' or '1'
> >     depending on different memory technology.
> >
> > For example, on BeagleBoard X15 it fills memory with '0'.
> >
> > Furthermore, the minimal amount of memory blk_derase() can erase is
> > "erase group size", and it's usually 512 KiB or more. So if we try to
> > erase some partition which is smaller than 512 KiB, "fastboot erase"
> > won't work at all, due to alignment. It's good practice to align all
> > partitions to "erase group size" boundary, but it's not a strict
> > requirement, so we just can't use blk_derase() due to this restriction.
> >
> > In order to provide the consistent way to erase partitions, adhere to
> > the fastboot spec, and fix erasing of small partitions, let's use
> > regular MMC write operation and fill the partition with 0xFF.
> >
> > [1] https://android.googlesource.com/platform/system/core/+/refs/tags/android-10.0.0_r25/fastboot/README.md#command-reference
> > [2] https://www.jedec.org/system/files/docs/JESD84-A441.pdf
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > ---
> >  drivers/fastboot/fb_mmc.c | 90 +++++++++++++++++++++++++--------------
> >  1 file changed, 59 insertions(+), 31 deletions(-)
>
> Roman (CC-ed) reported below [0-1] measurements results with and w/o
> the patch when erasing the eMMC 'system' partition on R-Car H3ULCB.
>
> As can be seen, 'fastboot erase' now needs roughly one order of
> magnitude more time to perform its job. That's a big performance hit and
> it is not easy to live with.
>
> I tried to track down the original commit adding the "0xff" mention to
> fastboot docs, but couldn't find any discussion attached to it:
>
>   erase:%s           Erase the indicated partition (clear to 0xFFs)
>
> I think that interpreting the above statement as a requirement is a
> matter of perspective. The statement could have also been written as
> a pure empirical observation on specific HW. It could also have been
> a reflection of author's experience, who might only have dealt with
> 0xFF-filled-on-erase MMC technologies.
>
> In any case and regardless of the above, filling memory with 0xFF on
> erase for ALL platforms seems rather expensive. Any ideas/thoughts?
>
> Could userspace stay agnostic whether memory is 0x0 or 0xFF on erase and
> just concentrate on the useful contents/payload?
>

The main point of my patch is to fix the erasing of partitions smaller
than group erase size (512 KiB on BeagleBoard X15). For example, it's
impossible to erase 'misc' partition (128 KiB), as such an operation
would erase 512 KiB, so alignment code leads to no-op. Also, I
wouldn't jump into conclusions about fastboot spec. Relying on 0xFF
can be useful e.g. for testing purposes.

All that said, I'm positive that this issue should be fixed. The only
way I see to avoid performance degradation is to:
  - use actual MMC erase operation when erasing partition >= group erase size
  - use MMC write operation when erasing partition < group erase size

But that leaves unhandled the case when partition is bigger than group
erase size, but not a multiple o group erase size (so the remainder
won't be erased). Of course we can do MMC write op for the not aligned
remainder, but how would we know if MMC controller fills with 0x00 of
with 0xFF on MMC erase op? So all those options considered, I decided
to go with just MMC write operation for "fastboot erase".

I will try to come up with v2 soon, but I don't see any ideal/elegant
solution here. So if you have any ideas, please share.

> [0] Before applying the patch:
>     $ time fastboot erase system
>     ******** Did you mean to fastboot format this ext4 partition?
>     erasing 'system_a'...
>     OKAY [  6.521s]
>     finished. total time: 6.521s
>
>     real 0m6,588s
>     user 0m0,005s
>     sys 0m0,000s
>
> [1] After applying the patch:
>     $ time fastboot erase system
>     ******** Did you mean to fastboot format this ext4 partition?
>     erasing 'system_a'...
>     OKAY [ 51.256s]
>     finished. total time: 51.256s
>
>     real 0m51,478s
>     user 0m0,004s
>     sys 0m0,000s
>
> --
> Best Regards,
> Eugeniu