* Philippe Mathieu-Daudé (phi...@redhat.com) wrote: > On 12/5/19 10:36 AM, Damien Hedde wrote: > > On 12/4/19 9:34 PM, Philippe Mathieu-Daudé wrote: > > > On 12/4/19 5:40 PM, Damien Hedde wrote: > > > > On 12/2/19 5:15 PM, Peter Maydell wrote: > > > > > > > > > > The one topic I think we could do with discussing is whether > > > > > a simple uint64_t giving the frequency of the clock in Hz is > > > > > the right representation. In particular in your patch 9 the > > > > > board has a clock frequency that's not a nice integer number > > > > > of Hz. I think Philippe also mentioned on irc some board where > > > > > the UART clock ends up at a weird frequency. Since the > > > > > representation of the frequency is baked into the migration > > > > > format it's going to be easier to get it right first rather > > > > > than trying to change it later. > > > > > > Important precision for Damien, IIUC we can not migrate float/double > > > types. > > > > > > > > So what should the representation be? Some random thoughts: > > > > > > > > > > 1) ptimer internally uses a 'period plus fraction' representation: > > > > > int64_t period is the integer part of the period in nanoseconds, > > > > > uint32_t period_frac is the fractional part of the period > > > > > (if you like you can think of this as "96-bit integer > > > > > period measured in units of one-2^32nd of a nanosecond"). > > > > > However its only public interfaces for setting the frequency > > > > > are (a) set the frequency in Hz (uint32_t) or (b) set > > > > > the period in nanoseconds (int64_t); the period_frac part > > > > > is used to handle frequencies which don't work out to > > > > > a nice whole number of nanoseconds per cycle. > > > > > > This is very clear, thanks Peter! > > > > > > The period+period_frac split allow us to migrate the 96 bits: > > > > > > VMSTATE_UINT32(period_frac, ptimer_state), > > > VMSTATE_INT64(period, ptimer_state), > > > > > > > > 2) I hear that SystemC uses "value plus a time unit", with > > > > > the smallest unit being a picosecond. (I think SystemC > > > > > also lets you specify the duty cycle, but we definitely > > > > > don't want to get into that!) > > > > > > > > The "value" is internally stored in a 64bits unsigned integer. > > > > > > > > > > > > > > 3) QEMUTimers are basically just nanosecond timers > > > > > > Similarly to SystemC, the QEMUTimers macro use a 'scale' unit, of: > > > > > > #define SCALE_MS 1000000 > > > #define SCALE_US 1000 > > > #define SCALE_NS 1 > > > > > > > > > > > > > 4) The MAME emulator seems to work with periods of > > > > > 96-bit attoseconds (represented internally by a > > > > > 32-bit count of seconds plus a 64-bit count of > > > > > attoseconds). One attosecond is 1e-18 seconds. > > > > > > > > > > Does anybody else have experience with other modelling > > > > > or emulator technology and how it represents clocks ? > > > > > > > > 5) In linux, a clock rate is an "unsigned long" representing Hz. > > > > > > > > > > > > > > I feel we should at least be able to represent clocks > > > > > with the same accuracy that ptimer has. > > > > > > > > Then is a maybe a good idea to store the period and not the frequency in > > > > clocks so that we don't loose anything when we switch from a clock to a > > > > ptimer ? > > > > > > I think storing the period as an integer type is a good idea. > > > > > > However if we store the period in nanoseconds, we get at most 1GHz > > > frequency. > > > > > > The attosecond granularity feels overkill. > > > > > > If we use a 96-bit integer to store picoseconds and use similar SCALE > > > macros we get to 1THz. > > > > > > Regardless the unit chosen, as long it is integer, we can migrate it. > > > If can migrate the period, we don't need to migrate the frequency. > > > We can then use the float type in with the timer API to pass frequencies > > > (which in the modeled hardware are ratios, likely not integers). > > > > > > So we could use set_freq(100e6 / 3), set_freq(40e6 / 5.5) directly. > > > > > > > Regarding the clock, I don't see any strong obstacle to switch > > > > internally to a period based value. > > > > The only things we have to choose is how to represent a disabled clock. > > > > Since putting a "0" period to a ptimer will disable the timer in > > > > ptimer_reload(). We can choose that (and it's a good value because we > > > > can multiply or divide it, it stays the same). > > > > > > > > We could use the same representation as a ptimer. But if we don't keep a > > > > C number representation, then computation of frequencies/periods will be > > > > complicated at best and error prone. > > > > > > > > From that point of view, if we could stick to a 64bits integer (or > > > > floating point number) it would be great. Can we use a sub nanosecond > > > > unit that fit our needs ? > > > > > > > > I did some test with a unit of 2^-32 of nanoseconds on 64bits (is that > > > > the unit of the ptimer fractional part ?) and if I'm not mistaken > > > > + we have a frequency range from ~0.2Hz up to 10^18Hz > > > > + the resolution is decreasing with the frequency (but at 100Mhz we have > > > > a ~2.3mHz resolution, at 1GHz it's ~0.23Hz and at 10GHz ~23Hz > > > > resolution). We hit 1Hz resolution around 2GHz. > > > > > > > > So it sounds to me we have largely enough resolution to model clocks in > > > > the range of frequencies we will have to handle. What do you think ? > > > > > > Back to your series, I wonder why you want to store the frequency in > > > ClockIn. ClockIn shouldn't be aware at what frequency it is clocked. > > > What matters is ClockOut, and each device exposing ClockOuts has a > > > (migrated) state of the output frequencies (rather in fields, or encoded > > > in registers). Once migrated, after the state is loaded back into the > > > device, we call post_load(). Isn't it a good place to call > > > clock_set_frequency(ClockOut[]) which will correctly set each ClockIn > > > frequency. > > > > > > IOW I don't think ClockIn/ClockOut require to migrate a frequency field. > > > > > > > I agree it is more logical to store the frequency in clock out. But, > > regarding migration constraints, we have no choice I think because a > > device cannot rely on values that are migrated by another device for > > restoring its state. (when I checked, I add the impression that > > post_load()s are called on a per device migration basis not all at the > > end of migration). > > Cc'ing David to clear that out.
That's pretty much right; the 'post_load' is called on a structure at the end of loading the data for that structure. You can do things at the end of migration; one way is to register a vm change state handler (search for qemu_add_vm_change_state_handler) and that means you get a kick when the VM starts running or a timer set in virtual time (not wall clock time because that becomes sensitive to the speed of the host). Somewhere ^^^ it says we can't migrate fp values; I'm not sure that's true; we've got a VMSTATE_FLOAT64 macro but I don't see it's used anywhere. Dave > > So we could store the frequency in clock out and migrate things there. > > But since we have no way to ensure all clock out states are migrated > > before some device fetch a ClockIn: we'll have to say "don't fetch one > > of your ClockIn frequency during migration and migrate the value > > yourself if you need it", pretty much like gpios. > > > > So we will probably migrate all ClockOut and almost all ClockIn. > > > > It would nice if we had a way to ensure clocks are migrated before > > devices try to use them. But I don't think this is possible. > > > > -- > > Damien > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK