On 22 June 2018 at 14:50, Aaron Lindsay <[email protected]> wrote: > On Apr 20 11:17, Peter Maydell wrote: >> On 17 April 2018 at 21:37, Aaron Lindsay <[email protected]> wrote: >> > pmccntr_read and pmccntr_write contained duplicate code that was already >> > being handled by pmccntr_sync. Consolidate the duplicated code into two >> > functions: pmccntr_op_start and pmccntr_op_finish. Add a companion to >> > c15_ccnt in CPUARMState so that we can simultaneously save both the >> > architectural register value and the last underlying cycle count - this >> > ensure time isn't lost and will also allow us to access the 'old' >> > architectural register value in order to detect overflows in later >> > patches. >> > >> > Signed-off-by: Aaron Lindsay <[email protected]>
>> > - /* If the counter is enabled, this stores the last time the >> > counter >> > - * was reset. Otherwise it stores the counter value >> > + /* Stores the architectural value of the counter *the last time >> > it was >> > + * updated* by pmccntr_op_start. Accesses should always be >> > surrounded >> > + * by pmccntr_op_start/pmccntr_op_finish to guarantee the latest >> > + * architecturally-corect value is being read/set. >> > */ >> > uint64_t c15_ccnt; >> > + /* Stores the delta between the architectural value and the >> > underlying >> > + * cycle count during normal operation. It is used to update >> > c15_ccnt >> > + * to be the correct architectural value before accesses. During >> > + * accesses, c15_ccnt_delta contains the underlying count being >> > used >> > + * for the access, after which it reverts to the delta value in >> > + * pmccntr_op_finish. >> > + */ >> > + uint64_t c15_ccnt_delta; >> >> So the key question here is: how does this work for VM migration? > > To be honest, I'm not sure I fully understand the things I need to be > looking out for with VM migration. > > My guess, though, is that this current implementation is not sufficient. > Perhaps there needs to be logic to ensure that c15_ccnt is the current > architectural value before migration and also to setup c15_ccnt_delta to > be the delta between that architectural value and the underlying cycle > count upon inbound migration. Does that sound like an approach which > would fit well within the rest of the migration framework? You need to deal with two different situations: (1) migration from an older QEMU which doesn't have this patchset (2) migration from a QEMU with this patchset to one with this patchset Either: (a) all the architectural state can be expressed in our existing state fields in whatever the previous format was -- in this case you just need to ensure that cpu_pre_save() and cpu_post_load() put the state there and unpack it again (b) we were missing some architectural state and really do need to transfer more over the wire than we were before -- in this case you need to add a new subsection to the vmstate which has the fields that contain that new state, and give the subsection a suitable 'needed' function to indicate when the subsection should be transferred plus pre_load and post_load functions that allow us to cope correctly with the case of the older QEMU that doesn't send the subsection. thanks -- PMM
