On Fri, Apr 18, 2008 at 06:56:41PM +0300, Avi Kivity wrote: > [EMAIL PROTECTED] wrote: > > From: Ben-Ami Yassour <[EMAIL PROTECTED]> > > > > Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]> > > Signed-off-by: Muli Ben-Yehuda <[EMAIL PROTECTED]> > > --- > > libkvm/libkvm.c | 24 ++++++++---- > > qemu/hw/pci-passthrough.c | 89 > > +++++++++++---------------------------------- > > qemu/hw/pci-passthrough.h | 2 + > > 3 files changed, 40 insertions(+), 75 deletions(-) > > > > diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c > > index de91328..8c02af9 100644 > > --- a/libkvm/libkvm.c > > +++ b/libkvm/libkvm.c > > @@ -400,7 +400,7 @@ void *kvm_create_userspace_phys_mem(kvm_context_t kvm, > > unsigned long phys_start, > > { > > int r; > > int prot = PROT_READ; > > - void *ptr; > > + void *ptr = NULL; > > struct kvm_userspace_memory_region memory = { > > .memory_size = len, > > .guest_phys_addr = phys_start, > > @@ -410,16 +410,24 @@ void *kvm_create_userspace_phys_mem(kvm_context_t > > kvm, unsigned long phys_start, > > if (writable) > > prot |= PROT_WRITE; > > > > - ptr = mmap(NULL, len, prot, MAP_ANONYMOUS | MAP_SHARED, -1, 0); > > - if (ptr == MAP_FAILED) { > > - fprintf(stderr, "create_userspace_phys_mem: %s", > > strerror(errno)); > > - return 0; > > - } > > + if (len > 0) { > > + ptr = mmap(NULL, len, prot, MAP_ANONYMOUS | MAP_SHARED, -1, 0); > > + if (ptr == MAP_FAILED) { > > + fprintf(stderr, "create_userspace_phys_mem: %s", > > + strerror(errno)); > > + return 0; > > + } > > > > - memset(ptr, 0, len); > > + memset(ptr, 0, len); > > + } > > > > memory.userspace_addr = (unsigned long)ptr; > > - memory.slot = get_free_slot(kvm); > > + > > + if (len > 0) > > + memory.slot = get_free_slot(kvm); > > + else > > + memory.slot = get_slot(phys_start); > > + > > r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory); > > if (r == -1) { > > fprintf(stderr, "create_userspace_phys_mem: %s", > > strerror(errno)); > > > > This looks like support for zero-length memory slots? Why is it > needed? > > It needs to be in a separate patch.
We need an interface to remove a memslot. When the guest writes to a direct assigned device's BAR and changes an MMIO region, we need to remove the old slot and establish a new one. The kernel side treats 0-sized memslot as "remove", but the userspace side didn't quite handle it properly. Personally I would've preferred a proper "remove" interface, rather than shoehorning it into kvm_create_userspace_phys_mem and a 0-sized slot. If that's acceptable, we'll put together a patch. > > diff --git a/qemu/hw/pci-passthrough.c b/qemu/hw/pci-passthrough.c > > index 7ffcc7b..a5894d9 100644 > > --- a/qemu/hw/pci-passthrough.c > > +++ b/qemu/hw/pci-passthrough.c > > @@ -25,18 +25,6 @@ typedef __u64 resource_size_t; > > extern kvm_context_t kvm_context; > > extern FILE *logfile; > > > > -CPUReadMemoryFunc *pt_mmio_read_cb[3] = { > > - pt_mmio_readb, > > - pt_mmio_readw, > > - pt_mmio_readl > > -}; > > - > > -CPUWriteMemoryFunc *pt_mmio_write_cb[3] = { > > - pt_mmio_writeb, > > - pt_mmio_writew, > > - pt_mmio_writel > > -}; > > - > > > > There's at least one use case for keeping mmio in userspace: > reverse-engineering a device driver. So if it doesn't cause too much > trouble, please keep this an option. I don't think it's a big deal to support both, although I'm not sure how useful it would be (especially considering mmiotrace). Did you have a user-interface for specifying it in mind? Cheers, Muli ------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel