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
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
