[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 2/2] hw/sd: skip double power-up in sd_vmstate_pre_load()
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [RFC PATCH 2/2] hw/sd: skip double power-up in sd_vmstate_pre_load() |
Date: |
Mon, 23 Jan 2023 12:09:38 +0000 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> +David / Juan / Peter for migration and timers.
>
> On 20/1/23 13:01, Daniel Henrique Barboza wrote:
> > At this moment any migration with the RISC-V sifive_u machine
> > fails with the following error:
> >
> > qemu-system-riscv64: ../hw/sd/sd.c:297: sd_ocr_powerup: Assertion
> > `!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)' failed.
> >
> > The assert was introduced by dd26eb43337a ("hw/sd: model a power-up
> > delay, as a workaround for an EDK2 bug"). It introduced a delayed timer
> > of 0.5ms to power up the card after the first ACMD41 command. The assert
> > prevents the card from being turned on twice.
> >
> > When migrating a machine that uses a sd card, e.g. RISC-V sifive_u, the
> > card is turned on during machine_init() in both source and destination
> > hosts.
If that's a problem, then why don't you make that machine_init code
check whether we're INMIGRATE, and skip the power on?
> When the migration stream finishes in the destination, the
> > pre_load() hook will attempt to turn on the card before loading its
> > vmstate. The assert() is always going to hit because the card was
> > already on.
> >
> > Change sd_vmstate_pre_load() to check first if the sd card is turned on
> > before executing a sd_ocr_powerup() and triggering the assert.
Again, not knowing this device, but the other thought is a Post_load
that determines if the OCR state hasn't been loaded, and then turns it
on.
Dave
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> > hw/sd/sd.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index bd88c1a8f0..4add719643 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -664,11 +664,19 @@ static int sd_vmstate_pre_load(void *opaque)
> > {
> > SDState *sd = opaque;
> > - /* If the OCR state is not included (prior versions, or not
> > + /*
> > + * If the OCR state is not included (prior versions, or not
> > * needed), then the OCR must be set as powered up. If the OCR state
> > * is included, this will be replaced by the state restore.
> > + *
> > + * However, there's a chance that the board will powerup the SD
> > + * before reaching INMIGRATE state in the destination host.
> > + * Powering up the SD again in this case will cause an assert
> > + * inside sd_ocr_powerup(). Skip sd_ocr_powerup() in this case.
> > */
> > - sd_ocr_powerup(sd);
> > + if (!sd_card_powered_up(sd)) {
> > + sd_ocr_powerup(sd);
> > + }
> > return 0;
> > }
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK