[Patch] Buffer disk I/O requests

[Patch] Buffer disk I/O requests

[Han, Weidong]

Disk I/O of VM is slow compared to native, I/O port access consumes time
significantly, which could be partly improved with buffered I/O.  
The attached patch partly improves the performance of disk I/O through
buffering Disk I/O requests. 
 
Signed-off-by: Zhiteng Huang <zhiteng.huang@intel.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>

RE: [Patch] Buffer disk I/O requests

[Han, Weidong]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Sorry, attach the patch :)

--Weidong

Han, Weidong wrote:
> Disk I/O of VM is slow compared to native, I/O port access consumes
> time significantly, which could be partly improved with buffered I/O.
> The attached patch partly improves the performance of disk I/O through
> buffering Disk I/O requests.
> 
> Signed-off-by: Zhiteng Huang <zhiteng.huang@intel.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


[-- Attachment #2: buffer_disk_io.patch --]
[-- Type: application/octet-stream, Size: 9909 bytes --]

diff -r 3ecf51689671 tools/ioemu/hw/ide.c
--- a/tools/ioemu/hw/ide.c	Sun May 13 10:04:34 2007 +0100
+++ b/tools/ioemu/hw/ide.c	Mon May 14 08:50:01 2007 +0800
@@ -2519,6 +2519,9 @@ static void bmdma_map(PCIDevice *pci_dev
     PCIIDEState *d = (PCIIDEState *)pci_dev;
     int i;
 
+    /* hypercall to set start address of bus master */
+    xc_hvm_set_bmdma_base(xc_handle, domid, addr);
+
     for(i = 0;i < 2; i++) {
         BMDMAState *bm = &d->bmdma[i];
         d->ide_if[2 * i].bmdma = bm;
diff -r 3ecf51689671 tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Sun May 13 10:04:34 2007 +0100
+++ b/tools/libxc/xc_domain.c	Fri May 11 16:59:50 2007 +0800
@@ -696,6 +696,25 @@ int xc_get_hvm_param(int handle, domid_t
     return rc;
 }
 
+int xc_hvm_set_bmdma_base(
+    int handle, domid_t dom, unsigned long start_addr)
+{
+    DECLARE_HYPERCALL;
+    xen_hvm_set_bmdma_base_t arg;
+    int rc;
+
+    hypercall.op     = __HYPERVISOR_hvm_op;
+    hypercall.arg[0] = HVMOP_set_bmdma_base;
+    hypercall.arg[1] = (unsigned long)&arg;
+    arg.domid = dom;
+    arg.start_addr = start_addr;
+    if ( lock_pages(&arg, sizeof(arg)) != 0 )
+        return -1;
+    rc = do_xen_hypercall(handle, &hypercall);
+    unlock_pages(&arg, sizeof(arg));
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r 3ecf51689671 tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Sun May 13 10:04:34 2007 +0100
+++ b/tools/libxc/xenctrl.h	Fri May 11 16:59:50 2007 +0800
@@ -843,6 +843,10 @@ int xc_set_hvm_param(int handle, domid_t
 int xc_set_hvm_param(int handle, domid_t dom, int param, unsigned long value);
 int xc_get_hvm_param(int handle, domid_t dom, int param, unsigned long *value);
 
+/* set start address of bus master DMA */
+int xc_hvm_set_bmdma_base(
+    int handle, domid_t dom, unsigned long start_addr);
+
 /* PowerPC specific. */
 int xc_alloc_real_mode_area(int xc_handle,
                             uint32_t domid,
diff -r 3ecf51689671 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Sun May 13 10:04:34 2007 +0100
+++ b/xen/arch/x86/hvm/hvm.c	Mon May 14 08:50:03 2007 +0800
@@ -992,6 +992,38 @@ static int hvmop_set_pci_link_route(
 
     rc = 0;
     hvm_set_pci_link_route(d, op.link, op.isa_irq);
+
+ out:
+    rcu_unlock_domain(d);
+    return rc;
+}
+
+static int hvmop_set_bmdma_base(
+    XEN_GUEST_HANDLE(xen_hvm_set_bmdma_base_t) uop)
+{
+    struct xen_hvm_set_bmdma_base op;
+    struct domain *d;
+    int rc;
+    struct hvm_buffered_io_range *perdom_portio_ranges;
+
+    if ( copy_from_guest(&op, uop, 1) )
+        return -EFAULT;
+
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
+
+    d = rcu_lock_domain_by_id(op.domid);
+    if ( d == NULL )
+        return -ESRCH;
+
+    rc = -EINVAL;
+    if ( !is_hvm_domain(d) )
+        goto out;
+
+    rc = 0;
+    perdom_portio_ranges = d->arch.hvm_domain.buffered_portio_ranges;
+    perdom_portio_ranges[HVM_PERDOM_BMDMA_INDEX].start_addr= op.start_addr;
+    perdom_portio_ranges[HVM_PERDOM_BMDMA_INDEX].length = BMDMA_IO_PORT_NR;
 
  out:
     rcu_unlock_domain(d);
@@ -1090,13 +1122,15 @@ long do_hvm_op(unsigned long op, XEN_GUE
         rc = hvmop_set_pci_link_route(
             guest_handle_cast(arg, xen_hvm_set_pci_link_route_t));
         break;
+    case HVMOP_set_bmdma_base:
+        rc = hvmop_set_bmdma_base(
+            guest_handle_cast(arg, xen_hvm_set_bmdma_base_t));
+        break;
 
     default:
-    {
         gdprintk(XENLOG_WARNING, "Bad HVM op %ld.\n", op);
         rc = -ENOSYS;
         break;
-    }
     }
 
     return rc;
diff -r 3ecf51689671 xen/arch/x86/hvm/intercept.c
--- a/xen/arch/x86/hvm/intercept.c	Sun May 13 10:04:34 2007 +0100
+++ b/xen/arch/x86/hvm/intercept.c	Mon May 14 13:18:05 2007 +0800
@@ -44,18 +44,31 @@ static struct hvm_mmio_handler *hvm_mmio
     &vioapic_mmio_handler
 };
 
-struct hvm_buffered_io_range {
-    unsigned long start_addr;
-    unsigned long length;
+#define HVM_BUF_MMIO_RANGE_NR 1
+
+static struct hvm_buffered_io_range buffered_stdvga_range = {0xA0000, 0x20000};
+
+static struct hvm_buffered_io_range
+*hvm_buf_mmio_ranges[HVM_BUF_MMIO_RANGE_NR] =
+{
+    &buffered_stdvga_range
 };
 
-#define HVM_BUFFERED_IO_RANGE_NR 1
-
-static struct hvm_buffered_io_range buffered_stdvga_range = {0xA0000, 0x20000};
+#define HVM_BUF_PORTIO_RANGE_NR 4
+
+static struct hvm_buffered_io_range buffered_pri_ide_range = {0x1f0, 0x8};
+static struct hvm_buffered_io_range buffered_sec_ide_range = {0x170, 0x8};
+static struct hvm_buffered_io_range buffered_pri_status_range = {0x3f6, 0x1};
+static struct hvm_buffered_io_range buffered_sec_status_range = {0x376, 0x1};
+
 static struct hvm_buffered_io_range
-*hvm_buffered_io_ranges[HVM_BUFFERED_IO_RANGE_NR] =
-{
-    &buffered_stdvga_range
+*hvm_buf_portio_ranges[HVM_BUF_PORTIO_RANGE_NR] =
+{
+    &buffered_pri_ide_range,
+    &buffered_sec_ide_range,
+    &buffered_pri_status_range,
+    &buffered_sec_status_range
 };
 
 static inline void hvm_mmio_access(struct vcpu *v,
@@ -169,6 +182,18 @@ int hvm_buffered_io_send(ioreq_t *p)
     struct vcpu *v = current;
     struct hvm_ioreq_page *iorp = &v->domain->arch.hvm_domain.buf_ioreq;
     buffered_iopage_t *pg = iorp->va;
+    struct hvm_buffered_io_range bmdma_range =
+        v->domain->arch.hvm_domain.buffered_portio_ranges[HVM_PERDOM_BMDMA_INDEX];
+    unsigned long mask;
+
+    /* Don't intercept WRITE non-zero access to primary and secondary
+     * command registers of bus master, let it trigger DMA
+     */
+    mask = ~0UL << (8 * (sizeof(mask) - p->size));
+    if ( (p->addr == bmdma_range.start_addr ||
+          p->addr == bmdma_range.start_addr + 8) &&
+         (p->data & ~mask) != 0x0 )
+        return 0;
 
     spin_lock(&iorp->lock);
 
@@ -194,22 +219,41 @@ int hvm_buffered_io_intercept(ioreq_t *p
 int hvm_buffered_io_intercept(ioreq_t *p)
 {
     int i;
+    struct vcpu *v = current;
+    struct hvm_buffered_io_range *perdom_buf_portio_ranges =
+        v->domain->arch.hvm_domain.buffered_portio_ranges;
 
     /* ignore READ ioreq_t! */
     if ( p->dir == IOREQ_READ )
         return 0;
 
-    for ( i = 0; i < HVM_BUFFERED_IO_RANGE_NR; i++ ) {
-        if ( p->addr >= hvm_buffered_io_ranges[i]->start_addr &&
-             p->addr + p->size - 1 < hvm_buffered_io_ranges[i]->start_addr +
-                                     hvm_buffered_io_ranges[i]->length )
-            break;
-    }
-
-    if ( i == HVM_BUFFERED_IO_RANGE_NR )
-        return 0;
-
-    return hvm_buffered_io_send(p);
+    if ( p->type != IOREQ_TYPE_PIO )
+    {
+        for ( i = 0; i < HVM_BUF_MMIO_RANGE_NR; i++ ) {
+            if ( p->addr >= hvm_buf_mmio_ranges[i]->start_addr &&
+                 p->addr + p->size - 1 < hvm_buf_mmio_ranges[i]->start_addr +
+                                         hvm_buf_mmio_ranges[i]->length )
+                return hvm_buffered_io_send(p);
+        }
+    }
+    else
+    {
+        for ( i = 0; i < HVM_BUF_PORTIO_RANGE_NR; i++ ) {
+            if ( p->addr >= hvm_buf_portio_ranges[i]->start_addr &&
+                 p->addr + p->size - 1 < hvm_buf_portio_ranges[i]->start_addr +
+                                         hvm_buf_portio_ranges[i]->length )
+                return hvm_buffered_io_send(p);
+        }
+
+        for ( i = 0; i < HVM_PERDOM_BUF_PORTIO_RANGE_NR; i++ ) {
+            if ( p->addr >= perdom_buf_portio_ranges[i].start_addr &&
+                 p->addr + p->size - 1 < perdom_buf_portio_ranges[i].start_addr +
+                                         perdom_buf_portio_ranges[i].length )
+                return hvm_buffered_io_send(p);
+        }
+    }
+
+    return 0;
 }
 
 int hvm_mmio_intercept(ioreq_t *p)
diff -r 3ecf51689671 xen/arch/x86/hvm/platform.c
--- a/xen/arch/x86/hvm/platform.c	Sun May 13 10:04:34 2007 +0100
+++ b/xen/arch/x86/hvm/platform.c	Fri May 11 16:59:50 2007 +0800
@@ -875,7 +875,7 @@ void send_pio_req(unsigned long port, un
 
     p->data = value;
 
-    if ( hvm_portio_intercept(p) )
+    if ( hvm_portio_intercept(p) || hvm_buffered_io_intercept(p) )
     {
         p->state = STATE_IORESP_READY;
         hvm_io_assist();
diff -r 3ecf51689671 xen/include/asm-x86/hvm/domain.h
--- a/xen/include/asm-x86/hvm/domain.h	Sun May 13 10:04:34 2007 +0100
+++ b/xen/include/asm-x86/hvm/domain.h	Fri May 11 16:59:50 2007 +0800
@@ -28,6 +28,20 @@
 #include <public/hvm/params.h>
 #include <public/hvm/save.h>
 
+/* The number of per domain buffered portio ranges */
+#define HVM_PERDOM_BUF_PORTIO_RANGE_NR  1
+
+/* The index of per domain buffered_portio_ranges[] for bus master DMA */
+#define HVM_PERDOM_BMDMA_INDEX  0
+
+/* The number of I/O ports used by bus master DMA */
+#define BMDMA_IO_PORT_NR    16
+
+struct hvm_buffered_io_range {
+    unsigned long start_addr;
+    unsigned long length;
+};
+
 struct hvm_ioreq_page {
     spinlock_t lock;
     struct page_info *page;
@@ -55,6 +69,10 @@ struct hvm_domain {
     spinlock_t             pbuf_lock;
 
     uint64_t               params[HVM_NR_PARAMS];
+
+    /* The array of buffered portio ranges */
+    struct hvm_buffered_io_range
+        buffered_portio_ranges[HVM_PERDOM_BUF_PORTIO_RANGE_NR];
 };
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
diff -r 3ecf51689671 xen/include/public/hvm/hvm_op.h
--- a/xen/include/public/hvm/hvm_op.h	Sun May 13 10:04:34 2007 +0100
+++ b/xen/include/public/hvm/hvm_op.h	Fri May 11 16:59:50 2007 +0800
@@ -70,4 +70,13 @@ typedef struct xen_hvm_set_pci_link_rout
 typedef struct xen_hvm_set_pci_link_route xen_hvm_set_pci_link_route_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_link_route_t);
 
+/* set start address of bus master DMA. */
+#define HVMOP_set_bmdma_base    5
+struct xen_hvm_set_bmdma_base {
+    domid_t domid;
+    uint64_t start_addr;
+};
+typedef struct xen_hvm_set_bmdma_base xen_hvm_set_bmdma_base_t;
+DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_bmdma_base_t);
+
 #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [Patch] Buffer disk I/O requests

[Keir Fraser]

What sort of improvement does it achieve? It's a pretty hacky thing to have
to add... :-(

 -- Keir


On 14/5/07 10:36, "Han, Weidong" <weidong.han@intel.com> wrote:

> Disk I/O of VM is slow compared to native, I/O port access consumes time
> significantly, which could be partly improved with buffered I/O.
> The attached patch partly improves the performance of disk I/O through
> buffering Disk I/O requests.
>  
> Signed-off-by: Zhiteng Huang <zhiteng.huang@intel.com>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [Patch] Buffer disk I/O requests

[Han, Weidong]

Buffered disk I/O improves disk I/O port access performance. It's
similar to buffered vga which is mmio. We ran SysBench random file I/O
test and found it reached about 10% improvement.

--Weidong

Keir Fraser wrote:
> What sort of improvement does it achieve? It's a pretty hacky thing
> to have to add... :-(
> 
>  -- Keir
> 
> 
> On 14/5/07 10:36, "Han, Weidong" <weidong.han@intel.com> wrote:
> 
>> Disk I/O of VM is slow compared to native, I/O port access consumes
>> time significantly, which could be partly improved with buffered I/O.
>> The attached patch partly improves the performance of disk I/O
>> through buffering Disk I/O requests. 
>> 
>> Signed-off-by: Zhiteng Huang <zhiteng.huang@intel.com>
>> Signed-off-by: Weidong Han <weidong.han@intel.com>
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel

RE: [Patch] Buffer disk I/O requests

[Ian Pratt]

> Buffered disk I/O improves disk I/O port access performance. It's
> similar to buffered vga which is mmio. We ran SysBench random file I/O
> test and found it reached about 10% improvement.

How does it compare to just using the SCSI HBA support that got checked
in a few days ago (in the qemu-dm 0.9.0 upgrade)?

If we're going to add support for enabling buffering of ioport accesses
beyond what we currently special case for the VGA it should be via a
generic interface used by qemu to register sets of ports with xen and
configure how they will be handled. 

Ian

RE: [Patch] Buffer disk I/O requests

[Han, Weidong]

Ian Pratt wrote:
>> Buffered disk I/O improves disk I/O port access performance. It's
>> similar to buffered vga which is mmio. We ran SysBench random file
>> I/O test and found it reached about 10% improvement.
> 
> How does it compare to just using the SCSI HBA support that got
> checked in a few days ago (in the qemu-dm 0.9.0 upgrade)?
> 

In our test,  the performance of SCSI HBA is better than our patch
performance in qemu 0.9.0, But we find the total I/O preformance
downgrade a lot after upgrade to qemu 0.9.0. We suspect there may be
some issues in qemu 0.9.0.

> If we're going to add support for enabling buffering of ioport
> accesses beyond what we currently special case for the VGA it should
> be via a generic interface used by qemu to register sets of ports
> with xen and configure how they will be handled.
> 
> Ian

Yes, if there are many these buffering cases, using a generic interface
is a final solution.

-- Weidong
 

RE: [Patch] Buffer disk I/O requests

[Ian Pratt]

> > How does it compare to just using the SCSI HBA support that got
> > checked in a few days ago (in the qemu-dm 0.9.0 upgrade)?
> 
> In our test,  the performance of SCSI HBA is better than our patch
> performance in qemu 0.9.0, 

Thanks for running the tests.

> But we find the total I/O preformance
> downgrade a lot after upgrade to qemu 0.9.0. We suspect there may be
> some issues in qemu 0.9.0.

Please can you explain in more detail. 

> > If we're going to add support for enabling buffering of ioport
> > accesses beyond what we currently special case for the VGA it should
> > be via a generic interface used by qemu to register sets of ports
> > with xen and configure how they will be handled.
>
> Yes, if there are many these buffering cases, using a generic
interface
> is a final solution.

I'd like to see this generic mechanism introduced for more than just
whether writes are buffered or not -- it would be very useful to
register ranges of port or mmio space for handling in different
fashions, e.g.:
 * read: forward to handler domain X channel Y
 * read: read as zeros
 * write: forward to handler domain X channel Y (and flush any buffered)
 * write: buffer and forward to domain X channel Y
 * write: ignore writes
 
These hooks would also be very useful for adding debugging/tracing. I
severely dislike our current  approach of forwarding anything that
doesn't get picked up in Xen to a single qemu-dm rather than registering
explicit ranges.

Best,
Ian



 



 

RE: [Patch] Buffer disk I/O requests

[Han, Weidong]

Ian Pratt wrote:
>>> How does it compare to just using the SCSI HBA support that got
>>> checked in a few days ago (in the qemu-dm 0.9.0 upgrade)?
>> 
>> In our test,  the performance of SCSI HBA is better than our patch
>> performance in qemu 0.9.0,
> 
> Thanks for running the tests.
> 
>> But we find the total I/O preformance
>> downgrade a lot after upgrade to qemu 0.9.0. We suspect there may be
>> some issues in qemu 0.9.0.
> 
> Please can you explain in more detail.
> 
We just found the preformance is down when we run the same test cases,
but now we don't know why.

>>> If we're going to add support for enabling buffering of ioport
>>> accesses beyond what we currently special case for the VGA it should
>>> be via a generic interface used by qemu to register sets of ports
>>> with xen and configure how they will be handled.
>> 
>> Yes, if there are many these buffering cases, using a generic
>> interface is a final solution.
> 
> I'd like to see this generic mechanism introduced for more than just
> whether writes are buffered or not -- it would be very useful to
> register ranges of port or mmio space for handling in different
> fashions, e.g.:
>  * read: forward to handler domain X channel Y
>  * read: read as zeros
>  * write: forward to handler domain X channel Y (and flush any
> buffered) 
>  * write: buffer and forward to domain X channel Y
>  * write: ignore writes
> 
> These hooks would also be very useful for adding debugging/tracing. I
> severely dislike our current  approach of forwarding anything that
> doesn't get picked up in Xen to a single qemu-dm rather than
> registering explicit ranges.
> 
> Best,
> Ian

Agree. A generic mechanism should be introduced in future, because we
have found buffering I/O port or MMIO is valuable. However I think our
patch is still useful now, after all it obviously improves performance
of IDE emulation disk I/O, 

Best regards,
--Weidong

Re: [Patch] Buffer disk I/O requests

[Keir Fraser]

On 20/5/07 14:57, "Han, Weidong" <weidong.han@intel.com> wrote:

>> These hooks would also be very useful for adding debugging/tracing. I
>> severely dislike our current  approach of forwarding anything that
>> doesn't get picked up in Xen to a single qemu-dm rather than
>> registering explicit ranges.
> 
> Agree. A generic mechanism should be introduced in future, because we
> have found buffering I/O port or MMIO is valuable. However I think our
> patch is still useful now, after all it obviously improves performance
> of IDE emulation disk I/O,

It's too ugly for a 10% win. It's not like the vga acceleration, which gave
a much bigger win and was also a less ugly change to the hypervisor
interfaces. It would be far more interesting to find out where the
(presumably larger than 10%) performance loss in the move to qemu 0.9.0
comes from.

 -- Keir

Re: [Patch] Buffer disk I/O requests

[Anthony Liguori]

Ian Pratt wrote:
>>> How does it compare to just using the SCSI HBA support that got
>>> checked in a few days ago (in the qemu-dm 0.9.0 upgrade)?
>> In our test,  the performance of SCSI HBA is better than our patch
>> performance in qemu 0.9.0, 
> 
> Thanks for running the tests.
> 
>> But we find the total I/O preformance
>> downgrade a lot after upgrade to qemu 0.9.0. We suspect there may be
>> some issues in qemu 0.9.0.
> 
> Please can you explain in more detail. 

IDE emulation is largely bound by how many requests you can get our per 
second.  In 0.8.2, DMA completion happened immediately after the IO 
request finished.  In 0.9.0, DMA completion is triggered by a AIO 
completion of the event.  This implies a trip through the main event loop.

By default, QEMU only allows glibc AIO to use a single thread which 
basically turns all AIO requests into synchronous requests.  You'll get 
some performance back by changing that to allow multiple threads.

I haven't looked at the QEMU 0.9.0 port just yet but I suspect that's 
the problem since I've seen this behavior in normal QEMU.

Regards,

Anthony Liguori

>>> If we're going to add support for enabling buffering of ioport
>>> accesses beyond what we currently special case for the VGA it should
>>> be via a generic interface used by qemu to register sets of ports
>>> with xen and configure how they will be handled.
>> Yes, if there are many these buffering cases, using a generic
> interface
>> is a final solution.
> 
> I'd like to see this generic mechanism introduced for more than just
> whether writes are buffered or not -- it would be very useful to
> register ranges of port or mmio space for handling in different
> fashions, e.g.:
>  * read: forward to handler domain X channel Y
>  * read: read as zeros
>  * write: forward to handler domain X channel Y (and flush any buffered)
>  * write: buffer and forward to domain X channel Y
>  * write: ignore writes
>  
> These hooks would also be very useful for adding debugging/tracing. I
> severely dislike our current  approach of forwarding anything that
> doesn't get picked up in Xen to a single qemu-dm rather than registering
> explicit ranges.
> 
> Best,
> Ian
> 
> 
> 
>  
> 
> 
> 
>