On Mon, Dec 03, 2018 at 03:23:41PM +0000, Marc Zyngier wrote:
> On 03/12/2018 15:16, Andrew Jones wrote:
> > On Mon, Dec 03, 2018 at 02:18:25PM +0000, Marc Zyngier wrote:
> >> On 03/12/2018 13:50, Andrew Jones wrote:
> >>> On Wed, Nov 28, 2018 at 02:45:16PM +0000, Steven Price wrote:
> >>>> +The structure pointed to by the PV_TIME_LPT hypercall is as follows:
> >>>> +
> >>>> + Field | Byte Length | Byte Offset | Description
> >>>> + --------------- | ----------- | ----------- | -------------------------------
> >>>> + Revision | 4 | 0 | Must be 0 for this revision
> >>>> + Attributes | 4 | 4 | Must be 0
> >>>> + sequence_number | 8 | 8 | Bit 0: reserved
> >>>> + | | | Bits 1:63 number of migrations
> >>>> + scale_mult | 8 | 16 | Multiplier to scale from native
> >>>> + | | | frequency to PV frequency
> >>>> + shift | 4 | 24 | Shift applied before multiplier
> >>>> + Reserved | 4 | 28 | Must be 0
> >>>> + Fn | 8 | 32 | Native frequency
> >>>> + Fpv | 8 | 40 | Paravirtualized frequency seen
> >>>> + | | | by guest
> >>>> + div_by_fpv_mult | 8 | 48 | Multiplier to implement fast
> >>>> + | | | divide by Fpv
> >>>
> >>> Here's kvmclock's struct
> >>>
> >>> struct pvclock_vcpu_time_info {
> >>> u32 version;
> >>> u32 pad0;
> >>> u64 tsc_timestamp;
> >>> u64 system_time;
> >>> u32 tsc_to_system_mul;
> >>> s8 tsc_shift;
> >>> u8 flags;
> >>> u8 pad[2];
> >>> }
> >>>
> >>> Revision =>
> >>> Attributes =>
> >>> sequence_number => version
> >>> scale_mult => tsc_to_system_mul (this is reversed, but OK)
> >>> shift => tsc_shift (also reversed)
> >>> Reserved =>
> >>> Fn => (pvclock doesn't have, but does have system_time)
> >>> Fpv =>
> >>> div_by_fpv_mult =>
> >>>
> >>> I haven't thought about this enough yet to be sure kvmclock's fields
> >>> are sufficient, but several look close - although the 'tsc' naming
> >>> isn't nice. Also, the pvclock struct could be extended by adding an
> >>> 'extended' flag to 'flags', and then appending more fields.
> >>
> >> One important thing is that this is not a KVM PV time implementation.
> >> This is something generic for the ARM architecture. Any ARM hypervisor
> >> should be able to implement this.
> >>
> >
> > I agree with that, but xen also uses the same pvclock structure on x86.
>
> Which makes sense.
>
> Each architecture has a way to express its PV time based on its own
> requirements, and I'd expect Xen on arm64 to use the ARM data structure
> (/me eyes the Xen/ARM maintainer...). I thought that what you were
> advocating was to use the x86 layout on ARM, which didn't make complete
> sense to me.
I sort of was. I was wondering if pvclock shouldn't be improved in a
backward compatible way by using the flags field to extend it. Field names
could change and new fields can be appended. Then, when x86 kvm and xen
are updated to use the extended pvclock struct, there may be more
opportunity for code sharing across architectures - perhaps much of
kvmclock could be shared. Of course I'm just thinking out loud, and doing
more of the out loud part than the thinking...
Hmm, I do see that arm-xen has already adopted the x86 layouts without
modification and also has a TODO saying the struct definitions should put
somewhere arch neutral.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
