On Thu, 03 Mar 2022 12:01:10 +0000,
Sebastian Ene <sebastianene@xxxxxxxxxx> wrote:
>
> > > +int kvm_cpu__setup_pvtime(struct kvm_cpu *vcpu)
> > > +{
> > > + int ret;
> > > + bool has_stolen_time;
> > > + u64 pvtime_guest_addr = ARM_PVTIME_MMIO_BASE + vcpu->cpu_id *
> > > + ARM_PVTIME_STRUCT_SIZE;
> > > + struct kvm_config *kvm_cfg = NULL;
> > > + struct kvm_device_attr pvtime_attr = (struct kvm_device_attr) {
> > > + .group = KVM_ARM_VCPU_PVTIME_CTRL,
> > > + .addr = KVM_ARM_VCPU_PVTIME_IPA
> > > + };
> > > +
> > > + kvm_cfg = &vcpu->kvm->cfg;
> > > + if (kvm_cfg->no_pvtime)
> > > + return 0;
> > > +
> > > + if (!pvtime_data.is_supported)
> > > + return -ENOTSUP;
> >
> > It is a bit odd to have this hard failure if running on a system that
> > doesn't have pvtime. It forces the user to alter their command-line,
> > which is a bit annoying. I'd rather have a soft-fail here.
> >
>
> The flag 'is_supported' is set to false when we support pvtime but we
> fail to configure it. We verify that we support pvtime by calling the check
> extension KVM_CAP_STEAL_TIME. I think the naming is odd here for the
> flag name. It should be : 'is_failed_cfg'.
Ah, I see. Yes, the name is misleading.
>
> > > +
> > > + has_stolen_time = kvm__supports_extension(vcpu->kvm,
> > > + KVM_CAP_STEAL_TIME);
> > > + if (!has_stolen_time)
> > > + return 0;
Here, you could force no_pvtime to 1, and avoid checking for each vcpu
once you detected that the host is not equipped to deal with it.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
