| Message ID | 20241028162308.2152712-1-jlu@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/sd/sdcard: Fix calculation of size when using eMMC boot partitions | expand |
On 10/28/24 17:23, Jan Luebbe wrote: > The sd_bootpart_offset() function calculates the *runtime* offset which > changes as the guest switches between accessing the main user data area > and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK > bits, so it shouldn't be used to calculate the main user data area size. > > Instead, subtract the boot_part_size directly (twice, as there are two > identical boot partitions defined by the eMMC spec). Fixes: c8cb19876d3e ("hw/sd/sdcard: Support boot area in emmc image") > Suggested-by: Cédric Le Goater <clg@kaod.org> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de> Tested-by: Guenter Roeck <linux@roeck-us.net> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > hw/sd/sd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 2d3467c3d956..8430d5ae361c 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev) > sect = 0; > } > size = sect << HWBLOCK_SHIFT; > - size -= sd_bootpart_offset(sd); > + if (sd_is_emmc(sd)) { > + size -= sd->boot_part_size * 2; > + } > > sect = sd_addr_to_wpnum(size) + 1; >
Philippe, I can take this patch through the aspeed branch. Thanks, C. On 10/28/24 17:23, Jan Luebbe wrote: > The sd_bootpart_offset() function calculates the *runtime* offset which > changes as the guest switches between accessing the main user data area > and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK > bits, so it shouldn't be used to calculate the main user data area size. > > Instead, subtract the boot_part_size directly (twice, as there are two > identical boot partitions defined by the eMMC spec). > > Suggested-by: Cédric Le Goater <clg@kaod.org> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de> > ---> hw/sd/sd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 2d3467c3d956..8430d5ae361c 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev) > sect = 0; > } > size = sect << HWBLOCK_SHIFT; > - size -= sd_bootpart_offset(sd); > + if (sd_is_emmc(sd)) { > + size -= sd->boot_part_size * 2; > + } > > sect = sd_addr_to_wpnum(size) + 1; >
On 2/11/24 12:06, Cédric Le Goater wrote: > Philippe, > > I can take this patch through the aspeed branch. I missed this patch because it was deeply buried in a long thread. Since you mentioned this feature will be reworked in the next cycle: Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Thanks, > > C. > > On 10/28/24 17:23, Jan Luebbe wrote: >> The sd_bootpart_offset() function calculates the *runtime* offset which >> changes as the guest switches between accessing the main user data area >> and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK >> bits, so it shouldn't be used to calculate the main user data area size. >> >> Instead, subtract the boot_part_size directly (twice, as there are two >> identical boot partitions defined by the eMMC spec). >> >> Suggested-by: Cédric Le Goater <clg@kaod.org> >> Signed-off-by: Jan Luebbe <jlu@pengutronix.de> >> ---> hw/sd/sd.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/sd/sd.c b/hw/sd/sd.c >> index 2d3467c3d956..8430d5ae361c 100644 >> --- a/hw/sd/sd.c >> +++ b/hw/sd/sd.c >> @@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev) >> sect = 0; >> } >> size = sect << HWBLOCK_SHIFT; >> - size -= sd_bootpart_offset(sd); >> + if (sd_is_emmc(sd)) { >> + size -= sd->boot_part_size * 2; >> + } >> sect = sd_addr_to_wpnum(size) + 1; >
28.10.2024 19:23, Jan Luebbe wrote: > The sd_bootpart_offset() function calculates the *runtime* offset which > changes as the guest switches between accessing the main user data area > and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK > bits, so it shouldn't be used to calculate the main user data area size. > > Instead, subtract the boot_part_size directly (twice, as there are two > identical boot partitions defined by the eMMC spec). Should I pick this up for stable-9.1 branch? Thanks, /mjt > Suggested-by: Cédric Le Goater <clg@kaod.org> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de> > --- > hw/sd/sd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 2d3467c3d956..8430d5ae361c 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev) > sect = 0; > } > size = sect << HWBLOCK_SHIFT; > - size -= sd_bootpart_offset(sd); > + if (sd_is_emmc(sd)) { > + size -= sd->boot_part_size * 2; > + } > > sect = sd_addr_to_wpnum(size) + 1;
On 11/5/24 17:13, Michael Tokarev wrote: > 28.10.2024 19:23, Jan Luebbe wrote: >> The sd_bootpart_offset() function calculates the *runtime* offset which >> changes as the guest switches between accessing the main user data area >> and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK >> bits, so it shouldn't be used to calculate the main user data area size. >> >> Instead, subtract the boot_part_size directly (twice, as there are two >> identical boot partitions defined by the eMMC spec). > > Should I pick this up for stable-9.1 branch? yes. Thanks, C.
diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 2d3467c3d956..8430d5ae361c 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -826,7 +826,9 @@ static void sd_reset(DeviceState *dev) sect = 0; } size = sect << HWBLOCK_SHIFT; - size -= sd_bootpart_offset(sd); + if (sd_is_emmc(sd)) { + size -= sd->boot_part_size * 2; + } sect = sd_addr_to_wpnum(size) + 1;
The sd_bootpart_offset() function calculates the *runtime* offset which changes as the guest switches between accessing the main user data area and the boot partitions by writing to the EXT_CSD_PART_CONFIG_ACC_MASK bits, so it shouldn't be used to calculate the main user data area size. Instead, subtract the boot_part_size directly (twice, as there are two identical boot partitions defined by the eMMC spec). Suggested-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Jan Luebbe <jlu@pengutronix.de> --- hw/sd/sd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)