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
