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

[Kevin Wolf <kwolf@redhat.com>]

Am 20.05.2015 um 10:06 hat Peter Maydell geschrieben:
> On 20 May 2015 at 08:54, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 19.05.2015 um 22:57 hat Peter Maydell geschrieben:
> >> Yeah, if there's genuinely an underlying state machine that's
> >> not completely visible in registers you need to actually model it.
> >> You should probably then model the register bits by calculating
> >> them from the state rather than by changing them as you go along
> >> in parallel with moving the state machine around.
> >
> > I think the combination of registers is actually enough to reconstruct
> > what state we're in, so it is derived (otherwise I would have fixed a
> > bug that I'm not aware of). Adding logic to derive it in a post-load
> > handler should be good enough.
> 
> 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.

In fact, I'm not even sure if we would still be able to derive the phase
from registers alone if we ever converted the FDC to use AIO instead of
synchronous I/O.

Kevin