thr3ads.net - Xen devel - HVM performance improvements [Aug 2013]

If this information is useful, please help other people find it:
Share via:

Andrew Cooper

2013-Aug-06 13:48 UTC

HVM performance improvements

This is re-post of my patchset to reduce the number of VMexits caused by
RomBIOS.  These patches have been included in several releases of XenServer,
and have had a substantial improvement to VM bootstorm scalability.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Pasi Kärkkäinen <pasik@iki.fi>
CC: George Dunlap <george.dunlap@eu.citrix.com>



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

Andrew Cooper

2013-Aug-06 13:48 UTC

head link

[Patch 1/5] rombios/keyboard: Don''t needlessly poll the status register

Repeated polling of the status register is not going to change its value, so
don''t needlessly take 8192 traps to Qemu when 1 will do.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
---
 tools/firmware/rombios/rombios.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/rombios/rombios.c b/tools/firmware/rombios/rombios.c
index 80980b6..e364759 100644
--- a/tools/firmware/rombios/rombios.c
+++ b/tools/firmware/rombios/rombios.c
@@ -1805,12 +1805,12 @@ keyboard_init()
     while ( (inb(0x64) & 0x02) && (--max>0)) outb(0x80, 0x00);
 
     /* flush incoming keys */
-    max=0x2000;
+    max=2;
     while (--max > 0) {
         outb(0x80, 0x00);
         if (inb(0x64) & 0x01) {
             inb(0x60);
-            max = 0x2000;
+            max = 2;
             }
         }
 
-- 
1.7.10.4

Andrew Cooper

2013-Aug-06 13:48 UTC

head link

[Patch 2/5] rombios/ata: Do not wait for BSY to be set

After issuing a reset, the BSY bit is expected to be set.  This is not the
case for Qemu.

In SeaBIOS.git: 580e33293244fee4556e56ecc67b8bd877f3c496

this check was even replaced with a udelay(5), as enough real hardware ignored
the BSY bit as well.

As rombios does not have an equivalent udelay(), replace the wait with a write
to port 0x80 which is whitelisted by Xen for ''a small delay''.

This causes 42k fewer IO traps to Qemu.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
---
 tools/firmware/rombios/rombios.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/rombios/rombios.c b/tools/firmware/rombios/rombios.c
index e364759..a3efc61 100644
--- a/tools/firmware/rombios/rombios.c
+++ b/tools/firmware/rombios/rombios.c
@@ -2914,8 +2914,8 @@ Bit16u device;
 // 8.2.1 (a) -- set SRST in DC
   outb(iobase2+ATA_CB_DC, ATA_CB_DC_HD15 | ATA_CB_DC_NIEN | ATA_CB_DC_SRST);
 
-// 8.2.1 (b) -- wait for BSY
-  await_ide(BSY, iobase1, 20);
+// 8.2.1 (b) -- wait
+  outb(0x80, 0x00);
 
 // 8.2.1 (f) -- clear SRST
   outb(iobase2+ATA_CB_DC, ATA_CB_DC_HD15 | ATA_CB_DC_NIEN);
-- 
1.7.10.4

Andrew Cooper

2013-Aug-06 13:48 UTC

head link

[Patch 3/5] rombios/ata: Reading this status register has no relevant side effects

So taking two traps when one will do is pointless.  This removes 1 of 13
VMExits on the int 0x13 hotpath.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
---
 tools/firmware/rombios/rombios.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/firmware/rombios/rombios.c b/tools/firmware/rombios/rombios.c
index a3efc61..f555747 100644
--- a/tools/firmware/rombios/rombios.c
+++ b/tools/firmware/rombios/rombios.c
@@ -2540,7 +2540,6 @@ static int await_ide(when_done,base,timeout)
   Bit32u time=0,last=0;
   Bit16u status;
   Bit8u result;
-  status = inb(base + ATA_CB_STAT); // for the times you''re supposed
to throw one away
   for(;;) {
     status = inb(base+ATA_CB_STAT);
     time++;
-- 
1.7.10.4

Andrew Cooper

2013-Aug-06 13:48 UTC

head link

[Patch 4/5] rombios/ata Remove another needless trap from the int 0x13 hotpath

The return value from await_ide() is always ignored, and most calls to
await_ide()
immediately reread the status register.

Therefore, making await_ide() return the last value of the status register
removes
a further two traps on the int 0x13 path.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
---
 tools/firmware/rombios/rombios.c |   32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/tools/firmware/rombios/rombios.c b/tools/firmware/rombios/rombios.c
index f555747..057aced 100644
--- a/tools/firmware/rombios/rombios.c
+++ b/tools/firmware/rombios/rombios.c
@@ -2531,14 +2531,14 @@ void ata_init( )
 
 #define IDE_TIMEOUT 32000u //32 seconds max for IDE ops
 
-int await_ide();
-static int await_ide(when_done,base,timeout)
+Bit8u await_ide();
+static Bit8u await_ide(when_done,base,timeout)
   Bit8u when_done;
   Bit16u base;
   Bit16u timeout;
 {
   Bit32u time=0,last=0;
-  Bit16u status;
+  Bit8u status;
   Bit8u result;
   for(;;) {
     status = inb(base+ATA_CB_STAT);
@@ -2556,7 +2556,7 @@ static int await_ide(when_done,base,timeout)
     else if (when_done == TIMEOUT)
       result = 0;
 
-    if (result) return 0;
+    if (result) return status;
     if (time>>16 != last) // mod 2048 each 16 ms
     {
       last = time >>16;
@@ -2565,12 +2565,12 @@ static int await_ide(when_done,base,timeout)
     if (status & ATA_CB_STAT_ERR)
     {
       BX_DEBUG_ATA("await_ide: ERROR
(TIMEOUT,BSY,!BSY,!BSY_DRQ,!BSY_!DRQ,!BSY_RDY) %d time= %ld timeout=
%d\n",when_done,time>>11, timeout);
-      return -1;
+      return status;
     }
     if ((timeout == 0) || ((time>>11) > timeout)) break;
   }
   BX_INFO("IDE time out\n");
-  return -1;
+  return status;
 }
 
 // ---------------------------------------------------------------------------
@@ -3016,8 +3016,7 @@ Bit32u lba_low, lba_high;
   outb(iobase1 + ATA_CB_DH, (slave ? ATA_CB_DH_DEV1 : ATA_CB_DH_DEV0) | (Bit8u)
head );
   outb(iobase1 + ATA_CB_CMD, command);
 
-  await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
-  status = inb(iobase1 + ATA_CB_STAT);
+  status = await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
 
   if (status & ATA_CB_STAT_ERR) {
     BX_DEBUG_ATA("ata_cmd_data_in : read error\n");
@@ -3077,8 +3076,7 @@ ASM_END
     current++;
     write_word(ebda_seg, &EbdaData->ata.trsfsectors,current);
     count--;
-    await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
-    status = inb(iobase1 + ATA_CB_STAT);
+    status = await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
     if (count == 0) {
       if ( (status & (ATA_CB_STAT_BSY | ATA_CB_STAT_RDY | ATA_CB_STAT_DRQ |
ATA_CB_STAT_ERR) )
           != ATA_CB_STAT_RDY ) {
@@ -3167,8 +3165,7 @@ Bit32u lba_low, lba_high;
   outb(iobase1 + ATA_CB_DH, (slave ? ATA_CB_DH_DEV1 : ATA_CB_DH_DEV0) | (Bit8u)
head );
   outb(iobase1 + ATA_CB_CMD, command);
 
-  await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
-  status = inb(iobase1 + ATA_CB_STAT);
+  status = await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
 
   if (status & ATA_CB_STAT_ERR) {
     BX_DEBUG_ATA("ata_cmd_data_out : read error\n");
@@ -3316,8 +3313,7 @@ Bit32u length;
   outb(iobase1 + ATA_CB_CMD, ATA_CMD_PACKET);
 
   // Device should ok to receive command
-  await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
-  status = inb(iobase1 + ATA_CB_STAT);
+  status = await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
 
   if (status & ATA_CB_STAT_ERR) {
     BX_DEBUG_ATA("ata_cmd_packet : error, status is %02x\n",status);
@@ -3353,8 +3349,7 @@ ASM_START
 ASM_END
 
   if (inout == ATA_DATA_NO) {
-    await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
-    status = inb(iobase1 + ATA_CB_STAT);
+    status = await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
     }
   else {
         Bit16u loops = 0;
@@ -3363,13 +3358,12 @@ ASM_END
 
       if (loops == 0) {//first time through
         status = inb(iobase2 + ATA_CB_ASTAT);
-        await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
+        status = await_ide(NOT_BSY_DRQ, iobase1, IDE_TIMEOUT);
       }
       else
-        await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
+        status = await_ide(NOT_BSY, iobase1, IDE_TIMEOUT);
       loops++;
 
-      status = inb(iobase1 + ATA_CB_STAT);
       sc = inb(iobase1 + ATA_CB_SC);
 
       // Check if command completed
-- 
1.7.10.4

Andrew Cooper

2013-Aug-06 13:48 UTC

head link

[Patch 5/5] rombios/debug: Reduce verbosity of rombios

Default builds of Qemu have the Bochs debug port logging #ifdef''d out,
so
remove all the completely wasted VMExits

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
---
 tools/firmware/rombios/rombios.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/rombios/rombios.h b/tools/firmware/rombios/rombios.h
index 93d12a4..0308a18 100644
--- a/tools/firmware/rombios/rombios.h
+++ b/tools/firmware/rombios/rombios.h
@@ -48,10 +48,11 @@
 // per-device basis. Debug info are sent only in debug mode
 #if DEBUG_ROMBIOS
 #  define BX_DEBUG(format, p...)  bios_printf(BIOS_PRINTF_INFO, format, ##p)
+#  define BX_INFO(format, p...)   bios_printf(BIOS_PRINTF_INFO, format, ##p)
 #else
 #  define BX_DEBUG(format, p...)
+#  define BX_INFO(format, p...)
 #endif
-#define BX_INFO(format, p...)   bios_printf(BIOS_PRINTF_INFO, format, ##p)
 #define BX_PANIC(format, p...)  bios_printf(BIOS_PRINTF_DEBHALT, format, ##p)
 
 #define ACPI_DATA_SIZE    0x00010000L
-- 
1.7.10.4

Keir Fraser

2013-Aug-06 14:36 UTC

head link

Re: HVM performance improvements

On 06/08/2013 14:48, "Andrew Cooper" <andrew.cooper3@citrix.com>
wrote:
> This is re-post of my patchset to reduce the number of VMexits caused by
> RomBIOS.  These patches have been included in several releases of
XenServer,
> and have had a substantial improvement to VM bootstorm scalability.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Pasi Kärkkäinen <pasik@iki.fi>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
Aren''t we primarily using seabios now anyway? Anyhow these looked sane
to
me.

Acked-by: Keir Fraser <keir@xen.org>
>

Andrew Cooper

2013-Aug-06 14:41 UTC

head link

Re: HVM performance improvements

On 06/08/13 15:36, Keir Fraser wrote:> On 06/08/2013 14:48, "Andrew Cooper"
<andrew.cooper3@citrix.com> wrote:
>
>> This is re-post of my patchset to reduce the number of VMexits caused
by
>> RomBIOS.  These patches have been included in several releases of
XenServer,
>> and have had a substantial improvement to VM bootstorm scalability.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Pasi Kärkkäinen <pasik@iki.fi>
>> CC: George Dunlap <george.dunlap@eu.citrix.com>
> Aren''t we primarily using seabios now anyway? Anyhow these looked
sane to
> me.
Upstream perhaps, but there will still be substantial use of
qemu-traditional.  XenServer for example will be maintaining both
qemu''s
for the forseeable future, and I expect other larger distros will.

(Also, these patches have been pending since early 4.2 without specific
concerns against them, but I clearly didn''t badger enough to get them
committed)
>
> Acked-by: Keir Fraser <keir@xen.org>
>
>

Ian Campbell

2013-Aug-08 12:47 UTC

head link

Re: HVM performance improvements

On Tue, 2013-08-06 at 15:36 +0100, Keir Fraser wrote:> On 06/08/2013 14:48, "Andrew Cooper"
<andrew.cooper3@citrix.com> wrote:
> 
> > This is re-post of my patchset to reduce the number of VMexits caused
by
> > RomBIOS.  These patches have been included in several releases of
XenServer,
> > and have had a substantial improvement to VM bootstorm scalability.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > CC: Keir Fraser <keir@xen.org>
> > CC: Pasi Kärkkäinen <pasik@iki.fi>
> > CC: George Dunlap <george.dunlap@eu.citrix.com>
> 
> Aren't we primarily using seabios now anyway? Anyhow these looked sane
to
> me.
> 
> Acked-by: Keir Fraser <keir@xen.org>
I applied all five patches.

Ian.


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

Xen devel - Aug 2013 - HVM performance improvements

HVM performance improvements

[Patch 1/5] rombios/keyboard: Don''t needlessly poll the status register

[Patch 2/5] rombios/ata: Do not wait for BSY to be set

[Patch 3/5] rombios/ata: Reading this status register has no relevant side effects

[Patch 4/5] rombios/ata Remove another needless trap from the int 0x13 hotpath

[Patch 5/5] rombios/debug: Reduce verbosity of rombios

Re: HVM performance improvements

Re: HVM performance improvements

Re: HVM performance improvements