Hi Fuad, On Tue, Feb 1, 2022 at 6:14 AM Fuad Tabba <tabba@xxxxxxxxxx> wrote: > > Hi Reiji, > > ... > > > > > diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c > > > > index 0a06d0648970..28d9bf0e178c 100644 > > > > --- a/arch/arm64/kvm/vgic/vgic-init.c > > > > +++ b/arch/arm64/kvm/vgic/vgic-init.c > > > > @@ -116,6 +116,11 @@ int kvm_vgic_create(struct kvm *kvm, u32 type) > > > > else > > > > INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions); > > > > > > > > + if (type == KVM_DEV_TYPE_ARM_VGIC_V3) > > > > + /* Set ID_AA64PFR0_EL1.GIC to 1 */ > > > > + (void)kvm_set_id_reg_feature(kvm, SYS_ID_AA64PFR0_EL1, > > > > + ID_AA64PFR0_GIC3, ID_AA64PFR0_GIC_SHIFT); > > > > + > > > > > > If this fails wouldn't it be better to return the error? > > > > This should never fail because kvm_vgic_create() prevents > > userspace from running the first KVM_RUN for any vCPUs > > while it calls kvm_set_id_reg_feature(). > > So, I am thinking of adding WARN_ON_ONCE() for the return value > > rather than adding an unnecessary error handling. > > Consider this to be a nit from my part, as I don't have any strong > feelings about this, but kvm_vgic_create() already returns an error if > there's a problem. So I don't think that that would be imposing any > additional error handling. That's true. But, since this failure case cannot be caused by userspace (but KVM's bug), I would still prefer using WARN_ON_ONCE. Thanks, Reiji _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
