Carsten Otte wrote: > Zhang, Xiantao wrote: >> +static struct kvm_vcpu *lid_to_vcpu(struct kvm *kvm, unsigned long >> id, + unsigned long eid) +{ >> + ia64_lid_t lid; >> + int i; >> + >> + for (i = 0; i < KVM_MAX_VCPUS; i++) { >> + if (kvm->vcpus[i]) { >> + lid.val = VCPU_LID(kvm->vcpus[i]); >> + if (lid.id == id && lid.eid == eid) >> + return kvm->vcpus[i]; >> + } >> + } >> + >> + return NULL; >> +} >> + >> +static int handle_ipi(struct kvm_vcpu *vcpu, struct kvm_run >> *kvm_run) +{ + struct exit_ctl_data *p = kvm_get_exit_data(vcpu); >> + struct kvm_vcpu *target_vcpu; >> + struct kvm_pt_regs *regs; >> + ia64_ipi_a addr = p->u.ipi_data.addr; >> + ia64_ipi_d data = p->u.ipi_data.data; >> + >> + target_vcpu = lid_to_vcpu(vcpu->kvm, addr.id, addr.eid); + if >> (!target_vcpu) + return handle_vm_error(vcpu, kvm_run); >> + >> + if (!target_vcpu->arch.launched) { >> + regs = vcpu_regs(target_vcpu); >> + >> + regs->cr_iip = vcpu->kvm->arch.rdv_sal_data.boot_ip; >> + regs->r1 = vcpu->kvm->arch.rdv_sal_data.boot_gp; + >> + target_vcpu->arch.mp_state = VCPU_MP_STATE_RUNNABLE; >> + if (waitqueue_active(&target_vcpu->wq)) >> + wake_up_interruptible(&target_vcpu->wq); >> + } else { >> + vcpu_deliver_ipi(target_vcpu, data.dm, data.vector); + if >> (target_vcpu != vcpu) + kvm_vcpu_kick(target_vcpu); >> + } >> + >> + return 1; >> +} > *Shrug*. This looks highly racy to me. You do access various values in > target_vcpu without any lock! I know that taking the target vcpu's > lock does'nt work because that one is held all the time during > KVM_VCPU_RUN. My solution to that was struct local_interrupt, which > has its own lock, and has the waitqueue plus everything I need to send > a sigp [that's our flavor of ipi]. > >> +int kvm_emulate_halt(struct kvm_vcpu *vcpu) >> +{ >> + >> + ktime_t kt; >> + long itc_diff; >> + unsigned long vcpu_now_itc; >> + >> + unsigned long expires; >> + struct hrtimer *p_ht = &vcpu->arch.hlt_timer; > That makes me jealous, I'd love to have hrtimer on s390 for this. I've > got to round up to the next jiffie. *Sigh* > >> +int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, + struct >> kvm_sregs *sregs) +{ >> + printk(KERN_WARNING"kvm:kvm_arch_vcpu_ioctl_set_sregs >> called!!\n"); >> + return 0; >> +} >> + >> +int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu, + struct >> kvm_sregs *sregs) +{ >> + printk(KERN_WARNING"kvm:kvm_arch_vcpu_ioctl_get_sregs >> called!!\n"); >> + return 0; >> + >> +} > Suggestion: if get/set sregs does'nt seem useful on ia64, why not > return -EINVAL? In that case, you could also not print a kern warning, > the user will either handle that situation or complain. > >> +int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) >> +{ > <snip> >> + /*FIXME:Need to removed it later!!\n*/ >> + vcpu->arch.apic = kzalloc(sizeof(struct kvm_lapic), GFP_KERNEL); >> + vcpu->arch.apic->vcpu = vcpu; > Fixme! Removed! >> +static int vti_vcpu_setup(struct kvm_vcpu *vcpu, int id) +{ >> + unsigned long psr; >> + int r; >> + >> + local_irq_save(psr); >> + r = kvm_insert_vmm_mapping(vcpu); >> + if (r) >> + goto fail; >> + r = kvm_vcpu_init(vcpu, vcpu->kvm, id); >> + if (r) >> + goto fail; > Maybe change to return r, rather then goto fail? It should be same. >> +int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct >> kvm_fpu *fpu) +{ >> + printk(KERN_WARNING"kvm:IA64 doesn't need to export" + "fpu to >> userspace!\n"); + return 0; >> +} >> + >> +int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct >> kvm_fpu *fpu) +{ >> + printk(KERN_WARNING"kvm:IA64 doesn't need to export" >> + "fpu to userspace !\n"); >> + return 0; >> +} > maybe -EINVAL? Good suggestion! >> +static int find_highest_bits(int *dat) >> +{ >> + u32 bits, bitnum; >> + int i; >> + >> + /* loop for all 256 bits */ >> + for (i = 7; i >= 0 ; i--) { >> + bits = dat[i]; >> + if (bits) { >> + bitnum = fls(bits); >> + return i * 32 + bitnum - 1; >> + } >> + } >> + >> + return -1; >> +} > Should be in asm/bitops.h. Look at find_first_bit() and friends, this > is duplicate. Seems find_first_bit only can be used to find lowest bit? Xiantao -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |