Re: [PATCH] hw/sd/sdcard: Fix handling of disabled boot partitions

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 1/10/24 15:01, Peter Maydell wrote:
> On Mon, 30 Sept 2024 at 21:05, Jan Lübbe <jlu@pengutronix.de> wrote:
>>
>> On Mon, 2024-09-30 at 15:18 +0100, Peter Maydell wrote:
>>> On Fri, 6 Sept 2024 at 17:51, Jan Luebbe <jlu@pengutronix.de> wrote:
>>>>
>>>> The enable bits in the EXT_CSD_PART_CONFIG ext_csd register do *not*
>>>> specify whether the boot partitions exist, but whether they are enabled
>>>> for booting. Existence of the boot partitions is specified by a
>>>> EXT_CSD_BOOT_MULT != 0.
>>>>
>>>> Currently, in the case of boot-partition-size=1M and boot-config=0,
>>>> Linux detects boot partitions of 1M. But as sd_bootpart_offset always
>>>> returns 0, all reads/writes are mapped to the same offset in the backing
>>>> file.
>>>>
>>>> Fix this bug by calculating the offset independent of which partition is
>>>> enabled for booting.
>>>
>>> Looking at the spec this change seems correct to me.
>>>
>>> Can you elaborate on when users might run into this bug?
>>> As far as I can see only aspeed.c sets boot-partition-size,
>>> and when it does so it also sets boot-config to 8. Or are
>>> we fixing this for the benefit of future board types?
>>
>> I stumbled across this when trying to use the eMMC emulation for the RAUC test
>> suite (with some unrelated local hacks, which I still need to clean up for
>> submission) [1]. Future boards would be affected as well.
>>
>> One other possible issue would be disabling the boot partition by using 'mmc
>> bootpart enable 0 0 /dev/mmcblk0' (or similar) from Linux. The layout of the
>> backing file shouldn't change in that case.
> 
> Thanks for the clarification. I've applied this patch to
> target-arm.next with the following paragraph added to the
> commit message:
>   This bug is unlikely to affect many users with QEMU's current set of
>   boards, because only aspeed sets boot-partition-size, and it also
>   sets boot-config to 8. So to run into this a user would have to
>   manually mark the boot partition non-booting from within the guest.
> 
> and I cc'd it to stable.

Thanks Jan for the fix and Peter for merging this patch!