[PATCH v4] libxl: Add AHCI support for upstream qemu

[PATCH v4] libxl: Add AHCI support for upstream qemu

[Fabio Fantoni]

Usage:
hdtype=ide|ahci (default=ide)

If hdtype=ahci adds ich9 disk controller in ahci mode and uses it with
upstream qemu to emulate disks instead of ide.
It doesn't support cdroms which still using ide (cdroms will use
"-device ide-cd" as new qemu parameter)
Ahci requires new qemu parameter but for now other emulated disks cases
remains with old ones (I did it in other patch, not needed by this one)
I did it as libxl parameter disabled by default to avoid possible
problems:
- with save/restore/migration (restoring with ahci a domU that was with
ide instead)
- windows < 8 without pv drivers (a registry key change is needed for
AHCI<->IDE change FWIK to avoid possible blue screen)
- windows XP or older that many not support ahci by default.
Setting AHCI with libxl parameter and default to disabled seems the best
solution.
AHCI increase hvm domUs boot performance. On linux hvm domU I saw up to
only 20% of the previous total boot time, whereas boot time decrease a
lot on W7 domUs for most of boots I have done. Small difference in boot
time compared to ide mode on W8 and newer (probably other xen
improvements or fixes are needed not ahci related)

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>

---

Changes in v4:
- libxl.h: LIBXL_HAVE_AHCI->LIBXL_HAVE_HVM_AHCI
- xl_cmdimpl.c: improved using libxl_hdtype_from_string

Changes in v3:
- changed ahci=0|1 to hdtype=ide|ahci
- small doc entry and code improvements as suggested by Ian Campbell

Changes in v2:
- libxl_dm.c: small code style fix
- added vbd-interface.txt changes
---
 docs/man/xl.cfg.pod.5       | 10 ++++++++++
 docs/misc/vbd-interface.txt |  6 ++++--
 tools/libxl/libxl.h         | 10 ++++++++++
 tools/libxl/libxl_create.c  |  3 +++
 tools/libxl/libxl_dm.c      | 11 ++++++++++-
 tools/libxl/libxl_types.idl |  6 ++++++
 tools/libxl/xl_cmdimpl.c    |  7 +++++++
 7 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a3e0e2e..ef7d558 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -904,6 +904,16 @@ default is B<cd>.
 
 =back
 
+=item B<hdtype="STRING">
+
+Select the hd disk type (ide|ahci).
+If hdtype=ahci adds ich9 disk controller in AHCI mode and uses it with
+upstream qemu to emulate disks instead of IDE. It decreases boot time
+but may not be supported by default in Windows xp and older Windows.
+The default is ide.
+
+=back
+
 =head3 Paging
 
 The following options control the mechanisms used to virtualise guest
diff --git a/docs/misc/vbd-interface.txt b/docs/misc/vbd-interface.txt
index f873db0..1c996bf 100644
--- a/docs/misc/vbd-interface.txt
+++ b/docs/misc/vbd-interface.txt
@@ -3,18 +3,20 @@ Xen guest interface
 
 A Xen guest can be provided with block devices.  These are always
 provided as Xen VBDs; for HVM guests they may also be provided as
-emulated IDE or SCSI disks.
+emulated IDE, AHCI or SCSI disks.
 
 The abstract interface involves specifying, for each block device:
 
  * Nominal disk type: Xen virtual disk (aka xvd*, the default); SCSI
-   (sd*); IDE (hd*).
+   (sd*); IDE or AHCI (hd*).
 
    For HVM guests, each whole-disk hd* and and sd* device is made
    available _both_ via emulated IDE resp. SCSI controller, _and_ as a
    Xen VBD.  The HVM guest is entitled to assume that the IDE or SCSI
    disks available via the emulated IDE controller target the same
    underlying devices as the corresponding Xen VBD (ie, multipath).
+   In hd* case with hdtype=ahci, disk will be AHCI via emulated
+   ich9 disk controller.
 
    For PV guests every device is made available to the guest only as a
    Xen VBD.  For these domains the type is advisory, for use by the
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a1c5d15..3a4edba 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -596,6 +596,16 @@ typedef struct libxl__ctx libxl_ctx;
 #define LIBXL_HAVE_SPICE_STREAMINGVIDEO 1
 
 /*
+ * LIBXL_HAVE_HVM_AHCI
+ *
+ * If defined, then the u.hvm structure will contain a enum type:
+ * hdtype. This value defines if ahci support is present.
+ *
+ * If this is not defined, the ahci support is ignored.
+ */
+#define LIBXL_HAVE_HVM_AHCI 1
+
+/*
  * LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS 1
  *
  * If this is defined, libxl_domain_create_restore()'s API has changed to
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f366a09..290303b 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -250,6 +250,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         if (!b_info->u.hvm.vga.kind)
             b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
 
+        if (!b_info->u.hvm.hdtype)
+            b_info->u.hvm.hdtype = LIBXL_HDTYPE_IDE;
+
         switch (b_info->device_model_version) {
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
             switch (b_info->u.hvm.vga.kind) {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 317a8eb..ad434f0 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -818,6 +818,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
 
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+        if (b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI)
+            flexarray_append_pair(dm_args, "-device", "ahci,id=ahci0");
         for (i = 0; i < num_disks; i++) {
             int disk, part;
             int dev_number =
@@ -872,7 +874,14 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                     drive = libxl__sprintf
                         (gc, "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
                          pdev_path, disk, format);
-                else if (disk < 4)
+                else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) {
+                    flexarray_vappend(dm_args, "-drive",
+                        GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
+                        pdev_path, disk, format),
+                        "-device", GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
+                        disk, disk), NULL);
+                    continue;
+                } else if (disk < 4)
                     drive = libxl__sprintf
                         (gc, "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback",
                          pdev_path, disk, format);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index e1632fa..39874d3 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -201,6 +201,11 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [
     (3, "reference_tsc"),
     ])
 
+libxl_hdtype = Enumeration("hdtype", [
+    (1, "IDE"),
+    (2, "AHCI"),
+    ], init_val = "LIBXL_HDTYPE_IDE")
+
 #
 # Complex libxl types
 #
@@ -442,6 +447,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("nested_hvm",       libxl_defbool),
                                        ("smbios_firmware",  string),
                                        ("acpi_firmware",    string),
+                                       ("hdtype",           libxl_hdtype),
                                        ("nographic",        libxl_defbool),
                                        ("vga",              libxl_vga_interface_info),
                                        ("vnc",              libxl_vnc_info),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c858068..55f694d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2146,6 +2146,13 @@ skip_vfb:
             b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
                                          LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
 
+        if (!xlu_cfg_get_string(config, "hdtype", &buf, 0) &&
+            libxl_hdtype_from_string(buf, &b_info->u.hvm.hdtype)) {
+                fprintf(stderr, "ERROR: invalid value \"%s\" for \"hdtype\"\n",
+                    buf);
+                exit (1);
+        }
+
         xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
         xlu_cfg_get_defbool (config, "spice", &b_info->u.hvm.spice.enable, 0);
         if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
-- 
1.9.1

Re: [PATCH v4] libxl: Add AHCI support for upstream qemu

[Ian Campbell]

On Wed, 2015-07-08 at 16:31 +0200, Fabio Fantoni wrote:
> Usage:
> hdtype=ide|ahci (default=ide)
> 
> If hdtype=ahci adds ich9 disk controller in ahci mode and uses it with
> upstream qemu to emulate disks instead of ide.
> It doesn't support cdroms which still using ide (cdroms will use
> "-device ide-cd" as new qemu parameter)
> Ahci requires new qemu parameter but for now other emulated disks cases
> remains with old ones (I did it in other patch, not needed by this one)
> I did it as libxl parameter disabled by default to avoid possible
> problems:
> - with save/restore/migration (restoring with ahci a domU that was with
> ide instead)
> - windows < 8 without pv drivers (a registry key change is needed for
> AHCI<->IDE change FWIK to avoid possible blue screen)
> - windows XP or older that many not support ahci by default.
> Setting AHCI with libxl parameter and default to disabled seems the best
> solution.
> AHCI increase hvm domUs boot performance. On linux hvm domU I saw up to
> only 20% of the previous total boot time, whereas boot time decrease a
> lot on W7 domUs for most of boots I have done. Small difference in boot
> time compared to ide mode on W8 and newer (probably other xen
> improvements or fixes are needed not ahci related)
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> 
> ---
> 
> Changes in v4:
> - libxl.h: LIBXL_HAVE_AHCI->LIBXL_HAVE_HVM_AHCI

In retrospect this should have been LIBXL_HAVE_HVM_HDTYPE. Since the
above was my suggestion if you agree I can make that simple change as I
commit.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Re: [PATCH v4] libxl: Add AHCI support for upstream qemu

[Fabio Fantoni]

Il 09/07/2015 12:26, Ian Campbell ha scritto:
> On Wed, 2015-07-08 at 16:31 +0200, Fabio Fantoni wrote:
>> Usage:
>> hdtype=ide|ahci (default=ide)
>>
>> If hdtype=ahci adds ich9 disk controller in ahci mode and uses it with
>> upstream qemu to emulate disks instead of ide.
>> It doesn't support cdroms which still using ide (cdroms will use
>> "-device ide-cd" as new qemu parameter)
>> Ahci requires new qemu parameter but for now other emulated disks cases
>> remains with old ones (I did it in other patch, not needed by this one)
>> I did it as libxl parameter disabled by default to avoid possible
>> problems:
>> - with save/restore/migration (restoring with ahci a domU that was with
>> ide instead)
>> - windows < 8 without pv drivers (a registry key change is needed for
>> AHCI<->IDE change FWIK to avoid possible blue screen)
>> - windows XP or older that many not support ahci by default.
>> Setting AHCI with libxl parameter and default to disabled seems the best
>> solution.
>> AHCI increase hvm domUs boot performance. On linux hvm domU I saw up to
>> only 20% of the previous total boot time, whereas boot time decrease a
>> lot on W7 domUs for most of boots I have done. Small difference in boot
>> time compared to ide mode on W8 and newer (probably other xen
>> improvements or fixes are needed not ahci related)
>>
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>
>> ---
>>
>> Changes in v4:
>> - libxl.h: LIBXL_HAVE_AHCI->LIBXL_HAVE_HVM_AHCI
> In retrospect this should have been LIBXL_HAVE_HVM_HDTYPE. Since the
> above was my suggestion if you agree I can make that simple change as I
> commit.

Yes you can change it directly as this change not risk introduce bug, 
thanks.
Is also Stabellini Acked-by (he did in v2) needed for qemu parameters 
part (not changed in v3 and v4)?
http://lists.xen.org/archives/html/xen-devel/2015-06/msg03551.html
I not added in latest posted version because I don't know if can be 
still valid.

>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Ian.
>

Re: [PATCH v4] libxl: Add AHCI support for upstream qemu

[Ian Campbell]

On Thu, 2015-07-09 at 12:36 +0200, Fabio Fantoni wrote:
> Il 09/07/2015 12:26, Ian Campbell ha scritto:
> > On Wed, 2015-07-08 at 16:31 +0200, Fabio Fantoni wrote:
> >> Usage:
> >> hdtype=ide|ahci (default=ide)
> >>
> >> If hdtype=ahci adds ich9 disk controller in ahci mode and uses it with
> >> upstream qemu to emulate disks instead of ide.
> >> It doesn't support cdroms which still using ide (cdroms will use
> >> "-device ide-cd" as new qemu parameter)
> >> Ahci requires new qemu parameter but for now other emulated disks cases
> >> remains with old ones (I did it in other patch, not needed by this one)
> >> I did it as libxl parameter disabled by default to avoid possible
> >> problems:
> >> - with save/restore/migration (restoring with ahci a domU that was with
> >> ide instead)
> >> - windows < 8 without pv drivers (a registry key change is needed for
> >> AHCI<->IDE change FWIK to avoid possible blue screen)
> >> - windows XP or older that many not support ahci by default.
> >> Setting AHCI with libxl parameter and default to disabled seems the best
> >> solution.
> >> AHCI increase hvm domUs boot performance. On linux hvm domU I saw up to
> >> only 20% of the previous total boot time, whereas boot time decrease a
> >> lot on W7 domUs for most of boots I have done. Small difference in boot
> >> time compared to ide mode on W8 and newer (probably other xen
> >> improvements or fixes are needed not ahci related)
> >>
> >> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> >>
> >> ---
> >>
> >> Changes in v4:
> >> - libxl.h: LIBXL_HAVE_AHCI->LIBXL_HAVE_HVM_AHCI
> > In retrospect this should have been LIBXL_HAVE_HVM_HDTYPE. Since the
> > above was my suggestion if you agree I can make that simple change as I
> > commit.
> 
> Yes you can change it directly as this change not risk introduce bug, 
> thanks.

Thanks I'll do this.

> Is also Stabellini Acked-by (he did in v2) needed for qemu parameters 
> part (not changed in v3 and v4)?
> http://lists.xen.org/archives/html/xen-devel/2015-06/msg03551.html
> I not added in latest posted version because I don't know if can be 
> still valid.

Right. I had been planning to take that ack on v2 into consideration (as
validating the set of qemu parameters used) but in the absence of him
having specified that constraint explicitly or having acked the latest
version I wasn't going to add his Ack and I think you did the right
thing by dropping it.

I'll apply shortly.

Ian.

Re: [PATCH v4] libxl: Add AHCI support for upstream qemu

[Ian Campbell]

On Wed, 2015-07-08 at 16:31 +0200, Fabio Fantoni wrote:
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a3e0e2e..ef7d558 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -904,6 +904,16 @@ default is B<cd>.
>  
>  =back
>  
> +=item B<hdtype="STRING">
> +
> +Select the hd disk type (ide|ahci).
> +If hdtype=ahci adds ich9 disk controller in AHCI mode and uses it with
> +upstream qemu to emulate disks instead of IDE. It decreases boot time
> +but may not be supported by default in Windows xp and older Windows.
> +The default is ide.
> +
> +=back

While applying one of my test architectures (arm32) complained:
man/xl.cfg.pod.5 around line 885: '=item' outside of any '=over'
POD document had syntax errors at /usr/bin/pod2man line 71.


I'm going to add this little fixup:
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 6ab9a6e..ded3f78 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -882,6 +882,10 @@ default is B<cd>.
 
 =back
 
+=head 3 Emulated disk controller type
+
+=over 4
+
 =item B<hdtype="STRING">
 
 Select the hd disk type (ide|ahci).

Ian.

Re: [PATCH v4] libxl: Add AHCI support for upstream qemu

[Ian Campbell]

On Thu, 2015-07-09 at 12:36 +0200, Fabio Fantoni wrote:
> Il 09/07/2015 12:26, Ian Campbell ha scritto:
> > On Wed, 2015-07-08 at 16:31 +0200, Fabio Fantoni wrote:
> >> Usage:
> >> hdtype=ide|ahci (default=ide)
> >>
> >> If hdtype=ahci adds ich9 disk controller in ahci mode and uses it with
> >> upstream qemu to emulate disks instead of ide.
> >> It doesn't support cdroms which still using ide (cdroms will use
> >> "-device ide-cd" as new qemu parameter)
> >> Ahci requires new qemu parameter but for now other emulated disks cases
> >> remains with old ones (I did it in other patch, not needed by this one)
> >> I did it as libxl parameter disabled by default to avoid possible
> >> problems:
> >> - with save/restore/migration (restoring with ahci a domU that was with
> >> ide instead)
> >> - windows < 8 without pv drivers (a registry key change is needed for
> >> AHCI<->IDE change FWIK to avoid possible blue screen)
> >> - windows XP or older that many not support ahci by default.
> >> Setting AHCI with libxl parameter and default to disabled seems the best
> >> solution.
> >> AHCI increase hvm domUs boot performance. On linux hvm domU I saw up to
> >> only 20% of the previous total boot time, whereas boot time decrease a
> >> lot on W7 domUs for most of boots I have done. Small difference in boot
> >> time compared to ide mode on W8 and newer (probably other xen
> >> improvements or fixes are needed not ahci related)
> >>
> >> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> >>
> >> ---
> >>
> >> Changes in v4:
> >> - libxl.h: LIBXL_HAVE_AHCI->LIBXL_HAVE_HVM_AHCI
> > In retrospect this should have been LIBXL_HAVE_HVM_HDTYPE. Since the
> > above was my suggestion if you agree I can make that simple change as I
> > commit.
> 
> Yes you can change it directly as this change not risk introduce bug, 
> thanks.

Done, pod doc fixed and applied, thanks.

Ian.