On Mon, 2010-06-21 at 16:56 +0300, Gleb Natapov wrote: > On Mon, Jun 21, 2010 at 05:31:43PM +0800, Zhang, Yanmin wrote: > > The 3rd patch is to implement para virt perf at host kernel. > > > > Signed-off-by: Zhang Yanmin <yanmin_zhang@xxxxxxxxxxxxxxx> > > <snip> > > + > > +static void kvm_copy_event_to_guest(struct kvm_vcpu *vcpu, > > + struct perf_event *host_event) > > +{ > > + struct host_perf_shadow *shadow = host_event->host_perf_shadow; > > + struct guest_perf_event counter; > > + int ret; > > + s32 overflows; > > + > > + ret = kvm_read_guest(vcpu->kvm, shadow->guest_event_addr, > > + &counter, sizeof(counter)); > > + if (ret < 0) > > + return; > > + > > +again: > > + overflows = atomic_read(&shadow->counter.overflows); > > + if (atomic_cmpxchg(&shadow->counter.overflows, overflows, 0) != > > + overflows) > > + goto again; > > + > > + counter.count = shadow->counter.count; > > + atomic_add(overflows, &counter.overflows); > > + > > + kvm_write_guest(vcpu->kvm, > > + shadow->guest_event_addr, > > + &counter, > > + sizeof(counter)); > Those kind of interfaces worry me since the can cause bugs that are > very hard to catch. What if guest enables some events and crashes into > kdump kernel (or kexec new kernel) without reseting HW. Now host may > write over guest memory without guest expecting it. Do you handle this > scenario in a guest side? I think you need to register reboot notify > and disable events from there. Sorry for missing your comments. My patch could take care of dead guest os by cleaning up all events in function kvm_arch_destroy_vm, so all events are closed if host user kills the guest qemu process. As for your scenario, I will register reboot notify and add a new pv perf hypercall interface to vmexit to host kernel to do cleanup. Thanks, Yanmin -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html