Hi Oliver,
I have additional questions/comments.
> > +2.2. ATTRIBUTE: KVM_ARM_VCPU_TIMER_PHYS_OFFSET
> > +-----------------------------------------
> > +
> > +:Parameters: in kvm_device_attr.addr the address for the timer offset is a
> > + pointer to a __u64
> > +
> > +Returns:
> > +
> > + ======= ==================================
> > + -EFAULT Error reading/writing the provided
> > + parameter address
> > + -ENXIO Timer offsetting not implemented
> > + ======= ==================================
> > +
> > +Specifies the guest's counter-timer offset from the host's virtual counter.
> > +The guest's physical counter value is then derived by the following
> > +equation:
> > +
> > + guest_cntpct = host_cntvct - KVM_ARM_VCPU_TIMER_PHYS_OFFSET
> > +
> > +The guest's virtual counter value is derived by the following equation:
> > +
> > + guest_cntvct = host_cntvct - KVM_REG_ARM_TIMER_OFFSET
> > + - KVM_ARM_VCPU_TIMER_PHYS_OFFSET
Although KVM_REG_ARM_TIMER_OFFSET is available only if userspace
opt-in KVM_CAP_ARM_VTIMER_OFFSET, setting this attribute doesn't
depend on the capability, correct ?
> > +KVM does not allow the use of varying offset values for different vCPUs;
> > +the last written offset value will be broadcasted to all vCPUs in a VM.
What if a new vCPU is added after KVM_ARM_VCPU_TIMER_PHYS_OFFSET
is set (for other existing vCPUs) ? Don't we want to set the offset for
the newly created vCPU as well ?
I'm a bit concerned about extra cost during Live Migration of a VM
that has large number of vCPUs if setting this is done by each vCPU
because that invokes update_timer_offset(), which sets the offset
for every vCPU that is owned by the guest holding kvm->lock,
two times (e.g. for a VM who number of vCPUs is KVM_MAX_VCPUS,
which is 512, the offset setting is done 524288 times).
Userspace can be implemented to run this just for single vCPU though.
> In my understanding, the offset that the code below specifies
> to call update_vtimer_cntvoff() is (guest's virtual counter) offset
> from the host's counter, which is always same as guest's virtual
> counter offset from the guest's physical counter-timer before this patch.
>
> int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)
> {
> struct arch_timer_context *timer;
>
> switch (regid) {
> <...>
> case KVM_REG_ARM_TIMER_CNT:
> timer = vcpu_vtimer(vcpu);
> update_vtimer_cntvoff(vcpu, kvm_phys_timer_read() - value);
> break;
> <...>
>
> With this patch, since the guest's counter-timer offset from the host's
> counter can be set by userspace, doesn't the code need to specify
> guest's virtual counter offset (from guest's physical counter-timer) ?
Thanks,
Reiji
