> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] > Sent: Thursday, August 21, 2014 3:18 PM > To: Alex Lemberg > Cc: linux-mmc@xxxxxxxxxxxxxxx; cjb@xxxxxxxxxx; Grant Grundler; Avi > Shchislowski; Gwendal Grignou (gwendal@xxxxxxxxxxxx) > Subject: Re: [RFC PATCH 1/1 v8]mmc: Support-FFU-for-eMMC-v5.0 > > [snip] > > >> >> By just browsing through the code for preparing and handling the > >> >> mmc data request in the above functions, I find quite some > >> >> duplication of code from the card/*.c files. Could we maybe share > >> >> some code between these layers to avoid duplications, and thus errors? > >> > > >> > Yes, we are aware of some duplications and tried to avoid it. > >> > Eventually, sharing code to avoid duplications will require changes > >> > in existing > >> card/*c files. > >> > We preferred not changing existing code as much as possible. > >> > >> Breaking out code and doing re-factoring in sometimes necessary. I > >> certainly think we should give it a try. > >> > >> My concern is not just duplication of code, but also duplication of > >> errors as that follows with it. > > > > Your concern is correct. > > Can we suggest taking this significant step separately from adding this patch > of FFU support? > > eMMC 5.0 is already there and vendors would like to start using this feature > in order to be able upgrading FW to their eMMC parts. > > No, I will only accept good quality code - sorry. But, I have a suggestion which > might simplify this patch significantly. :-) Read further down. > > [snip] > > >> >> > >> >> I would, as stated earlier, prefer one FFU operational code. Thus > >> >> it would be possible, before starting the FFU seuquence, to for > >> >> example do cache flushing, stopping BKOPS and suspend/flush the blk > queue etc. > >> >> I think this would be more safe. > >> > > >> > Agree, it probably would be more safe, but: > >> > - as mentioned before, we would like to split the process into two > >> > operational > >> codes due > >> > to different host power implementations > >> > - We implemented FFU as non-blocking procedure, which allows host > >> > sending > >> regular R/W commands > >> > between downloading FW data process and FW install > >> > >> Is there really a valid use case for that? > >> > > > > As mentioned above, in case we can assure that all hosts will perform > > eMMC card init routine on suspend/resume, we can unify those operations. > > > > > >> The reason we are considering to have this in-kernel is to get > >> robustness and to make sure the procedure don't get interrupted, at > >> least that is how I have interpreted the previous discussions. > >> > >> Maybe you can elaborate on the reasons for in-kernel once more? > > > > - In case FW data cannot be transferred by one singe transaction > > - The host need to be claimed during the FFU process to avoid interruptions > by other R/W IOs. > > - Device is set to FFU mode during FFU download operations, and no Normal > IOs are allowed during this process. > > Thanks for clarifying this. So, clearly we need some additional support in-kernel > to handle the synchronization mechanism. > > First, let's consider below snippet pasted from the eMMC5.0 spec: > -------------- > > Once in FFU Mode the host may send the new firmware bundle to the device > using one or more write commands. > > The host could regain regular functionally of write and read commands by > setting MODE_CONFIG field in the EXT_CSD back to Normal state. Switching > out of FFU Mode may abort the firmware download operation. When switched > back-in to FFU Mode, the host should check the FFU Status to get indication > about the number of sectors which were downloaded successfully by reading > the NUMBER_OF_FW_SECTORS_CORRECTLY_PROGRAMMED in the extended > CSD. In case the number of sectors which were downloaded successfully is zero > the host should re-start downloading the new firmware bundle from its first > sector. In case the number of sectors which were downloaded successfully is > positive the host should continue the download from the next sector, which > would resume the firmware download operation. > ------------- > > My conclusion from that is (correct me if I am wrong, since it may be vendor > specific), that only minor modifications should be needed to the mmc ioctl. We > need to add special treatment for when "cmd.opcode == > MMC_FFU_DOWNLOAD_OP". In such scenario the following sequence needs to > be maintained. > > 1. Claim host etc. > 2. Set FFU mode. > 3. Write some sectors to the FFU area, but in the same way as any other mmc > ioctl WRITE is done. > 4. Set normal mode. > 5. Relase host etc. > > That sequence then needs to be repeated to write the complete firmware, > depending on buffer size. Between each and every piece of FFU data that is > written, normal READ/WRITE operations can be served. > Right? Right > > User space will thus also be responsible of verifying the FFU download. It will > also have to find out whether SUPPORTED_MODE_OPERATION_CODES in > EXTCSD supported - and take relevant actions to complete the FFU upgrade > process. Right, and in this case user space will need to know exactly the way how FFU is work: - verify each FFU download operation - check that all FW data was downloaded successfully - choose and proceed with FFU install according to SUPPORTED_MODE_OPERATION_CODES Can we trust user space making the right implementation of FFU? Also, we were asked by Chromium team to change FFU implementation to be more secure/trusted using udev mechanism. Please see comments by Kees Cook in: "[RFC PATCH 1/1] mmc-utils: Support-sending-eMMC-5.0-FFU" thread. > > [snip] > > >> >> Another concern I see, is if the card might change identity due to > >> >> the firmware upgrade. The card identity is validated each system > >> >> resume cycle and failed validation is an error we can't recover from. > >> > > >> > The current assumption is that device will change its identity during the > FFU. > >> > But your concern is correct. > >> > In order to support this, the mmc_init_card() function call should > >> > be modified > >> and assume that the card is not "oldcard". > >> > >> This will be far more complex to support. It means we actually need > >> to remove the old card's struct device, it's corresponding blk device > >> and the blk device queue and for a NON_REMOVABLE host. Then trigger a > >> new mmc_rescan to detect and initialize the card again. I wondering > >> what upper mounted rootfs file systems thinks about that. :-) > >> > >> So, as for now, we should aim for the simple approach and thus don't > >> do re- initiation/power-cycle. Instead let's leave that to be done by > >> a "manual" power cycle of the platform. > > > > As mentioned above, we wanted to let host vendors to decide. > > If eMMC init is called during suspend/resume - no Power Cycle is required. > > I think you missed my point here. > > If the card identity changes and it's a NON_REMOVABLE host, we can't easily > just perform a suspend/resume cycle to complete the FFU upgrade. The best > and safest way is to power cycle the complete hw - always. > > Kind regards > Uffe ��.n��������+%������w��{.n�����{��i��)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
