[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a work
From: |
Andrew Baumann |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug |
Date: |
Wed, 23 Dec 2015 19:08:55 +0000 |
> From: Peter Crosthwaite [mailto:address@hidden
> Sent: Monday, 21 December 2015 14:57
> On Mon, Dec 21, 2015 at 2:25 PM, Andrew Baumann
> <address@hidden> wrote:
> >> From: qemu-devel-
> address@hidden
> >> [mailto:qemu-devel-
> >> address@hidden On Behalf Of
> >> Peter Crosthwaite
> >> Sent: Monday, 21 December 2015 13:46
> >> On Wed, Dec 16, 2015 at 11:02 AM, Andrew Baumann
> >> <address@hidden> wrote:
> >> > The SD spec for ACMD41 says that a zero argument is an "inquiry"
> >> > ACMD41, which does not start initialisation and is used only for
> >> > retrieving the OCR. However, Tianocore EDK2 (UEFI) has a bug [1]: it
> >> > first sends an inquiry (zero) ACMD41. If that first request returns an
> >> > OCR value with the power up bit (0x80000000) set, it assumes the card
> >> > is ready and continues, leaving the card in the wrong state. (My
> >> > assumption is that this works on hardware, because no real card is
> >> > immediately powered up upon reset.)
> >> >
> >> > This change models a delay of 0.5ms from the first ACMD41 to the
> power
> >> > being up. However, it also immediately sets the power on upon seeing a
> >> > non-zero (non-enquiry) ACMD41. This speeds up UEFI boot; it should
> >> > also account for guests that simply delay after card reset and then
> >> > issue an ACMD41 that they expect will succeed.
> > [...]
> >> > @@ -449,6 +462,8 @@ static const VMStateDescription sd_vmstate = {
> >> > .fields = (VMStateField[]) {
> >> > VMSTATE_UINT32(mode, SDState),
> >> > VMSTATE_INT32(state, SDState),
> >> > + VMSTATE_UINT32(ocr, SDState),
> >> > + VMSTATE_TIMER_PTR(ocr_power_timer, SDState),
> >>
> >> If you change the VMSTATE layout, you need to bump the version. As
> >> this is very common code, it may have stricter version bump
> >> requirements. Last I knew however, there was a way to add new fields
> >> at the end of VMSD without breaking backwards compatibility. Peter or
> >> Juan may know more.
> >
> > I'll admit that I didn't think about these issues when adding the fields,
> > but
> after a (quick) look at vmstate_save_state() and vmstate_load_state(), they
> seem to be using named fields in a json format, so I don't think the order of
> fields should matter if we are adding new ones. However, if we want to be
> able to migrate sd instances across across this change, then we'll need to
> arrange for the OCR to appear already powered-on if we're coming from a
> previous version. Does VMState have a way to do that? Essentially I just
> need to specify a default value for the ocr field if coming from an old
> vmstate
> version <= 1 that differs from the value set in sd_reset().
> >
>
> You can open code post_load logic as a callback, and I think you have
> access to the image version from there.
So, how about something like this:
+static int sd_vmstate_post_load(void *opaque, int version_id)
+{
+ SDState *sd = opaque;
+
+ if (version_id < 2) {
+ /* prior versions did not save the OCR or model a power up
+ * delay, so we need to mirror this state */
+ sd_ocr_powerup(sd);
+ }
+
+ return 0;
+}
+
static const VMStateDescription sd_vmstate = {
.name = "sd-card",
- .version_id = 1,
+ .version_id = 2,
.minimum_version_id = 1,
+ .post_load = sd_vmstate_post_load,
.fields = (VMStateField[]) {
Any idea how I can easily test this? Sorry, I am not at all familiar with the
vmstate/migration stuff. Is there a good/known working device model to test
that uses SD? I guess I just do savevm on the old one and loadvm on the new one?
Thanks,
Andrew
- [Qemu-devel] [PATCH v2 3/3] hw/sd: use guest error logging rather than fprintf to stderr, (continued)
- [Qemu-devel] [PATCH v2 3/3] hw/sd: use guest error logging rather than fprintf to stderr, Andrew Baumann, 2015/12/16
- [Qemu-devel] [PATCH v2 1/3] hw/sd: implement CMD23 (SET_BLOCK_COUNT) for MMC compatibility, Andrew Baumann, 2015/12/16
- [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug, Andrew Baumann, 2015/12/16
- Re: [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug, Peter Crosthwaite, 2015/12/21
- Re: [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug, Peter Maydell, 2015/12/23
- Re: [Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug, Andrew Baumann, 2015/12/23