Hi Reiji,
...
> > > @@ -2862,11 +3077,12 @@ void set_default_id_regs(struct kvm *kvm)
> > > u32 id;
> > > const struct sys_reg_desc *rd;
> > > u64 val;
> > > + struct id_reg_info *idr;
> > >
> > > for (i = 0; i < ARRAY_SIZE(sys_reg_descs); i++) {
> > > rd = &sys_reg_descs[i];
> > > if (rd->access != access_id_reg)
> > > - /* Not ID register, or hidden/reserved ID register */
> > > + /* Not ID register or hidden/reserved ID register */
> > > continue;
> > >
> > > id = reg_to_encoding(rd);
> > > @@ -2874,7 +3090,8 @@ void set_default_id_regs(struct kvm *kvm)
> > > /* Shouldn't happen */
> > > continue;
> > >
> > > - val = read_sanitised_ftr_reg(id);
> > > - kvm->arch.id_regs[IDREG_IDX(id)] = val;
> > > + idr = GET_ID_REG_INFO(id);
> > > + val = idr ? idr->vcpu_limit_val : read_sanitised_ftr_reg(id);
> > > + (void)write_kvm_id_reg(kvm, id, val);
> >
> > Rather than ignoring the return value of write_kvm_id_reg(), wouldn't
> > it be better if set_default_id_regs were to propagate it back to
> > kvm_arch_init_vm in case there's a problem?
>
> Since write_kvm_id_reg() should never return an error for this
> case, returning an error to kvm_arch_init_vm() adds a practically
> unnecessary error handling, which I would like to avoid.
> So, how about putting WARN_ON_ONCE on its return value ?
I think this makes sense in this case.
Thanks,
/fuad
> Thanks,
> Reiji
