Jeremy Fitzhardinge wrote: >> +static unsigned pvclock_get_time_values(struct pvclock_shadow_time *dst, >> + struct kvm_vcpu_time_info *src) >> > > I think the kvm_* types should be renamed. xen_* would make some sense > since the ABI originated with Xen, but something generic would be OK > too. The important thing to get across is that the structure defines a > guest<->host ABI which happens to be shared by Xen and KVM, and it isn't > an in-kernel API like the rest of paravirt_ops. Fine with me, but see below. > And having defined a common structure, we may as well use it in the > hypervisor-specific code/headers so there's no strange casting going on. Hmm, what is the state of include/xen/interface/? It is a straight copy of the xen public header files, right? Is it really ok to modify them? We have to do that to make the cast actually go away ... >> +/* >> + * This is our read_clock function. The host puts an tsc timestamp >> each time >> + * it updates a new time. Without the tsc adjustment, we can have a >> situation >> + * in which a vcpu starts to run earlier (smaller system_time), but >> probes >> + * time later (compared to another vcpu), leading to backwards time >> + */ >> > > This comment is confusing. Are you explaining why the tsc is read > within the loop? I think it can be clarified. This was just copyed over from somewhere else (kvmclock I think). >> +++ b/include/asm-x86/pvclock.h >> @@ -0,0 +1,6 @@ >> +#include <linux/clocksource.h> >> +#include <asm/kvm_para.h> >> +cycle_t pvclock_clocksource_read(struct kvm_vcpu_time_info *src); >> +void pvclock_read_wallclock(struct kvm_wall_clock *wall, >> + struct kvm_vcpu_time_info *vcpu, >> + struct timespec *ts); >> > > No #ifdef guards? Oh yes, I should better add some ... thanks, Gerd -- http://kraxel.fedorapeople.org/xenner/ _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization
