| Message ID | 1515506513-31961-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| Headers | show |
| Series | Reset SD cards attached to legacy-API controllers | expand |
Hi Peter, On 01/09/2018 11:01 AM, Peter Maydell wrote: > It turns out that we don't reset the SD card model if it's > attached to a legacy-API controller. For new-style controllers > where the SD card object is attached to an SD_BUS, the generic > qbus infrastructure takes care of ensuring that the card's > DeviceState::reset method gets called. But for the legacy > API where the card is created by calling sd_init(), the card > object has no SD_BUS to plug into, and so the reset function > must be called manually. > > This mostly had no ill effects because a guest OS will typically > perform an SD reset command as part of its device driver init > sequence. However it does mean that migration won't work, because > it's in sd_reset() that we set up the wpgrps_size field, and > without that the migration struct on reload won't match. > > This patchset fixes up the four legacy controllers to call > reset on the SD card device in their controller reset functions. > The first 3 are cc:stable as migration bugfixes. > > This fixes https://bugs.launchpad.net/qemu/+bug/1739378 > > Philippe: this probably clashes slightly with your sdcard > patchsets (and where you've updated devices to the new APIs > it becomes irrelevant), but I think we should apply these > first so we can have a simple patch to backport to the stable > tree. Sure, it makes sens. I didn't test migrating a card (but have a "sdcard migration qtest" entry in my TODO). this series: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Notes: > * OMAP doesn't migrate anyway, so isn't cc:stable > * for vexpress-a15, you also need "-machine secure=off": > a config with trustzone emulation enabled doesn't > successfully migrate (haven't yet debugged why) > > thanks > -- PMM > > Peter Maydell (4): > hw/sd/pl181: Reset SD card on controller reset > hw/sd/milkymist-memcard: Reset SD card on controller reset > hw/sd/ssi-sd: Reset SD card on controller reset > hw/sd/omap_mmc: Reset SD card on controller reset > > hw/sd/milkymist-memcard.c | 4 ++++ > hw/sd/omap_mmc.c | 14 ++++++++++---- > hw/sd/pl181.c | 4 ++++ > hw/sd/ssi-sd.c | 24 +++++++++++++++++++++++- > 4 files changed, 41 insertions(+), 5 deletions(-) >
On 9 January 2018 at 16:13, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > I didn't test migrating a card (but have a "sdcard migration qtest" > entry in my TODO). I usually test not by actually migrating but using 'savevm foo' in the monitor to save a snapshot and then -loadvm foo on the command line to resume it. thanks -- PMM
On 9 January 2018 at 16:13, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Peter, > > On 01/09/2018 11:01 AM, Peter Maydell wrote: >> It turns out that we don't reset the SD card model if it's >> attached to a legacy-API controller. For new-style controllers >> where the SD card object is attached to an SD_BUS, the generic >> qbus infrastructure takes care of ensuring that the card's >> DeviceState::reset method gets called. But for the legacy >> API where the card is created by calling sd_init(), the card >> object has no SD_BUS to plug into, and so the reset function >> must be called manually. >> >> This mostly had no ill effects because a guest OS will typically >> perform an SD reset command as part of its device driver init >> sequence. However it does mean that migration won't work, because >> it's in sd_reset() that we set up the wpgrps_size field, and >> without that the migration struct on reload won't match. >> >> This patchset fixes up the four legacy controllers to call >> reset on the SD card device in their controller reset functions. >> The first 3 are cc:stable as migration bugfixes. >> >> This fixes https://bugs.launchpad.net/qemu/+bug/1739378 >> >> Philippe: this probably clashes slightly with your sdcard >> patchsets (and where you've updated devices to the new APIs >> it becomes irrelevant), but I think we should apply these >> first so we can have a simple patch to backport to the stable >> tree. > > Sure, it makes sens. > > I didn't test migrating a card (but have a "sdcard migration qtest" > entry in my TODO). > > this series: > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Thanks for the review. I've applied this series to target-arm.next (with the fix to the milkymist commit message and the removal of s->mod = SSI_SD_CMD from the ssi-sd realize function that you suggested). -- PMM