Re: [PATCH] qemu: define and use VIRTIO_PFN_SHIFT

[Anthony Liguori <anthony@codemonkey.ws>]

Hollis Blanchard wrote:
> # HG changeset patch
> # User Hollis Blanchard <hollisb@us.ibm.com>
> # Date 1225946837 21600
> # Node ID 43a111ea61b542d3823e2a11d017e7b06b7ec254
> # Parent  b63967268af119e0faa4adc3086cdef857815548
> qemu: define and use VIRTIO_PFN_SHIFT
>
> The virtio front and back ends must agree about how big a pfn really is. Since
> qemu has no idea what "page size" the guest may be using, it must be
> independent of TARGET_PAGE_BITS.
>
> This patch should have no functional effect on x86 or ia64, but I'd like an ack from the
> ia64 guys.
>   

Would be better to add a new header in target-XXX instead of using 
cpu.h.  Virtio is not part of the CPU ISA.

> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>
> diff --git a/qemu/hw/virtio.c b/qemu/hw/virtio.c
> --- a/qemu/hw/virtio.c
> +++ b/qemu/hw/virtio.c
> @@ -56,6 +56,10 @@
>   */
>  #define wmb() do { } while (0)
>  
> +#define VRING_PAGE_SIZE (1<<12)
> +
> +#define ALIGN(x, a)  (((x)+(a)-1) & ~((a)-1))
> +
>  /* virt queue functions */

Why is VRING_PAGE_SIZE not architecture specific?

Regards,

Anthony Liguori


>  
>  static void *virtio_map_gpa(target_phys_addr_t addr, size_t size)
> @@ -95,8 +99,8 @@ static void *virtio_map_gpa(target_phys_
>  
>  static size_t virtqueue_size(int num)
>  {
> -    return TARGET_PAGE_ALIGN((sizeof(VRingDesc) * num) +
> -			     (sizeof(VRingAvail) + sizeof(uint16_t) * num)) +
> +    return ALIGN((sizeof(VRingDesc) * num) + (sizeof(VRingAvail) +
> +		 sizeof(uint16_t) * num), VRING_PAGE_SIZE) +
>  	(sizeof(VRingUsed) + sizeof(VRingUsedElem) * num);
>  }
>  
> @@ -104,7 +108,7 @@ static void virtqueue_init(VirtQueue *vq
>  {
>      vq->vring.desc = p;
>      vq->vring.avail = p + vq->vring.num * sizeof(VRingDesc);
> -    vq->vring.used = (void *)TARGET_PAGE_ALIGN((unsigned long)&vq->vring.avail->ring[vq->vring.num]);
> +    vq->vring.used = (void *)ALIGN((unsigned long)&vq->vring.avail->ring[vq->vring.num], VRING_PAGE_SIZE);
>  }
>  
>  static unsigned virtqueue_next_desc(VirtQueue *vq, unsigned int i)
> @@ -241,7 +245,7 @@ static void virtio_ioport_write(void *op
>  	vdev->features = val;
>  	break;
>      case VIRTIO_PCI_QUEUE_PFN:
> -	pa = (ram_addr_t)val << TARGET_PAGE_BITS;
> +	pa = (ram_addr_t)val << VIRTIO_PFN_SHIFT;
>  	vdev->vq[vdev->queue_sel].pfn = val;
>  	if (pa == 0) {
>              virtio_reset(vdev);
> @@ -519,7 +523,7 @@ void virtio_load(VirtIODevice *vdev, QEM
>  	    size_t size;
>  	    target_phys_addr_t pa;
>  
> -	    pa = (ram_addr_t)vdev->vq[i].pfn << TARGET_PAGE_BITS;
> +	    pa = (ram_addr_t)vdev->vq[i].pfn << VIRTIO_PFN_SHIFT;
>  	    size = virtqueue_size(vdev->vq[i].vring.num);
>  	    virtqueue_init(&vdev->vq[i], virtio_map_gpa(pa, size));
>  	}
> diff --git a/qemu/target-i386/cpu.h b/qemu/target-i386/cpu.h
> --- a/qemu/target-i386/cpu.h
> +++ b/qemu/target-i386/cpu.h
> @@ -751,6 +751,8 @@ static inline int cpu_get_time_fast(void
>  
>  #define TARGET_PAGE_BITS 12
>  
> +#define VIRTIO_PFN_SHIFT 12
> +
>  #define CPUState CPUX86State
>  #define cpu_init cpu_x86_init
>  #define cpu_exec cpu_x86_exec
> diff --git a/qemu/target-ia64/cpu.h b/qemu/target-ia64/cpu.h
> --- a/qemu/target-ia64/cpu.h
> +++ b/qemu/target-ia64/cpu.h
> @@ -30,6 +30,8 @@
>  #define TARGET_LONG_BITS 64
>  
>  #define TARGET_PAGE_BITS 16
> +
> +#define VIRTIO_PFN_SHIFT 16
>  
>  #define ELF_MACHINE	EM_IA_64
>  
> diff --git a/qemu/target-ppc/cpu.h b/qemu/target-ppc/cpu.h
> --- a/qemu/target-ppc/cpu.h
> +++ b/qemu/target-ppc/cpu.h
> @@ -54,6 +54,8 @@
>  #endif /* defined(TARGET_PPCEMB) */
>  
>  #endif /* defined (TARGET_PPC64) */
> +
> +#define VIRTIO_PFN_SHIFT 10
>  
>  #include "cpu-defs.h"
>  
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>