[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase

From: John Snow
Subject: Re: [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
Date: Wed, 20 May 2015 07:55:49 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 05/20/2015 05:24 AM, Peter Maydell wrote:
> On 20 May 2015 at 09:43, Kevin Wolf <address@hidden> wrote:
>> Am 20.05.2015 um 10:06 hat Peter Maydell geschrieben:
>>> That handles migration, which is good. But I still think that
>>> storing the same information in two places in the device
>>> state (phase field and the register fields) is error-prone.
>> That's actually my point. The registers are accessed everywhere in the
>> code, whereas phase transitions are in very few well-defined places
>> (there are exactly four of them, iirc). If they get out of sync, chances
>> are that the bug is in the register value, not in the phase. When we
>> know what phase we're in, we can assert the bits and actually catch such
>> bugs.
>>> If we want to switch to having a phase field we should calculate
>>> the relevant register bits on demand based on the phase, rather
>>> than keeping both copies of the state in sync manually.
>> That doesn't work, unfortunately. Some register bits imply a specific
>> phase (assuming correct code), but you can't derive the exact bits just
>> from the phase.
> Having now dug out a copy of the 82078 spec, I agree that the state
> isn't derivable purely from the register values in the general case.
> The controller clearly has a state machine internally but it doesn't
> surface that in the register state except indirectly.
> -- PMM

So even if /currently/ we can reconstitute it from the register values,
we may eventually be unable to.

post_load will work for now, but I fear the case (in ten years) when
someone else cleans up FDC code but fails to realize that the phase is
not explicitly migrated.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]