Re: [PATCH 1/1] Enble a guest to access a device's memory mapped I/O regions directly.

[Avi Kivity <avi@qumranet.com>]

benami@il.ibm.com wrote:
> From: Ben-Ami Yassour <benami@il.ibm.com>
>
> Signed-off-by: Ben-Ami Yassour <benami@il.ibm.com>
> Signed-off-by: Muli Ben-Yehuda <muli@il.ibm.com>
> ---
>  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.

> 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.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
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