[Qemu-devel] [PATCH v3 0/2] add ahci migration

[Qemu-devel] [PATCH v3 0/2] add ahci migration

[Jason Baron]

Hi,

Add migration bits for ahci. This allows q35 to be migratable. I also
have been working on some qtest migration tests, to show that this does
something. I will re-post those separately.

Thanks,

-Jason

Jason Baron (2):
  ahci: remove unused AHCIDevice fields
  ahci: add migration support

 hw/ide/ahci.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 hw/ide/ahci.h |   12 ++++++-
 hw/ide/ich.c  |   11 +++++--
 3 files changed, 99 insertions(+), 12 deletions(-)

[Qemu-devel] [PATCH v3 1/2] ahci: remove unused AHCIDevice fields

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

'dma_status' and 'dma_cb' are written to, but never read.
Remove these fields in preparation for AHCI migration bits.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 hw/ide/ahci.c |    8 ++------
 hw/ide/ahci.h |    2 --
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index d072449..72cd1c8 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1035,11 +1035,10 @@ out:
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
                            BlockDriverCompletionFunc *dma_cb)
 {
+#ifdef DEBUG_AHCI
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
-
+#endif
     DPRINTF(ad->port_no, "\n");
-    ad->dma_cb = dma_cb;
-    ad->dma_status |= BM_STATUS_DMAING;
     s->io_buffer_offset = 0;
     dma_cb(s, 0);
 }
@@ -1095,7 +1094,6 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit)
 static int ahci_dma_add_status(IDEDMA *dma, int status)
 {
     AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
-    ad->dma_status |= status;
     DPRINTF(ad->port_no, "set status: %x\n", status);
 
     if (status & BM_STATUS_INT) {
@@ -1114,8 +1112,6 @@ static int ahci_dma_set_inactive(IDEDMA *dma)
     /* update d2h status */
     ahci_write_fis_d2h(ad, NULL);
 
-    ad->dma_cb = NULL;
-
     if (!ad->check_bh) {
         /* maybe we still have something to process, check later */
         ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad);
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 1200a56..735b379 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -281,11 +281,9 @@ struct AHCIDevice {
     QEMUBH *check_bh;
     uint8_t *lst;
     uint8_t *res_fis;
-    int dma_status;
     int done_atapi_packet;
     int busy_slot;
     int init_d2h_sent;
-    BlockDriverCompletionFunc *dma_cb;
     AHCICmdHdr *cur_cmd;
     NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
 };
-- 
1.7.1

Re: [Qemu-devel] [PATCH v3 1/2] ahci: remove unused AHCIDevice fields

[Juan Quintela]

Jason Baron <jbaron@redhat.com> wrote:
> From: Jason Baron <jbaron@redhat.com>
>
> 'dma_status' and 'dma_cb' are written to, but never read.
> Remove these fields in preparation for AHCI migration bits.
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH v3 2/2] ahci: add migration support

[Jason Baron]

From: Jason Baron <jbaron@redhat.com>

I've tested these patches by migrating Windows 7 and Fedora 17 guests (while
uunder i/o) on both piix with ahci attached and on q35 (which has a built-in
ahci controller).

Changes from v2:
 -migrate all relevant ahci fields
 -flush any pending i/o in 'post_load'

Changes from v1:
 -extend Andreas Färber's patch

Signed-off-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Alexander Graf <agraf@suse.de>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/ide/ahci.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/ide/ahci.h |   10 +++++++
 hw/ide/ich.c  |   11 +++++--
 3 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 72cd1c8..96f224b 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1199,6 +1199,81 @@ void ahci_reset(AHCIState *s)
     }
 }
 
+static const VMStateDescription vmstate_ahci_device = {
+    .name = "ahci port",
+    .version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_IDE_BUS(port, AHCIDevice),
+        VMSTATE_UINT32(port_state, AHCIDevice),
+        VMSTATE_UINT32(finished, AHCIDevice),
+        VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice),
+        VMSTATE_UINT32(port_regs.lst_addr_hi, AHCIDevice),
+        VMSTATE_UINT32(port_regs.fis_addr, AHCIDevice),
+        VMSTATE_UINT32(port_regs.fis_addr_hi, AHCIDevice),
+        VMSTATE_UINT32(port_regs.irq_stat, AHCIDevice),
+        VMSTATE_UINT32(port_regs.irq_mask, AHCIDevice),
+        VMSTATE_UINT32(port_regs.cmd, AHCIDevice),
+        VMSTATE_UINT32(port_regs.tfdata, AHCIDevice),
+        VMSTATE_UINT32(port_regs.sig, AHCIDevice),
+        VMSTATE_UINT32(port_regs.scr_stat, AHCIDevice),
+        VMSTATE_UINT32(port_regs.scr_ctl, AHCIDevice),
+        VMSTATE_UINT32(port_regs.scr_err, AHCIDevice),
+        VMSTATE_UINT32(port_regs.scr_act, AHCIDevice),
+        VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice),
+        VMSTATE_INT32(done_atapi_packet, AHCIDevice),
+        VMSTATE_INT32(busy_slot, AHCIDevice),
+        VMSTATE_INT32(init_d2h_sent, AHCIDevice),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static int ahci_state_post_load(void *opaque, int version_id)
+{
+    int i;
+    struct AHCIDevice *ad;
+    AHCIState *s = opaque;
+
+    for (i = 0; i < s->ports; i++) {
+        ad = &s->dev[i];
+        AHCIPortRegs *pr = &ad->port_regs;
+
+        map_page(&ad->lst,
+                 ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
+        map_page(&ad->res_fis,
+                 ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
+        /*
+         * All pending i/o should be flushed out on a migrate. However,
+         * we might not have cleared the busy_slot since this is done
+         * in a bh. Also, issue i/o against any slots that are pending.
+         */
+        if (ad->busy_slot != -1) {
+            pr->cmd_issue &= ~(1 << ad->busy_slot);
+            ad->busy_slot = -1;
+        }
+        check_cmd(s, i);
+    }
+
+    return 0;
+}
+
+const VMStateDescription vmstate_ahci = {
+    .name = "ahci",
+    .version_id = 1,
+    .post_load = ahci_state_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
+                                     vmstate_ahci_device, AHCIDevice),
+        VMSTATE_UINT32(control_regs.cap, AHCIState),
+        VMSTATE_UINT32(control_regs.ghc, AHCIState),
+        VMSTATE_UINT32(control_regs.irqstatus, AHCIState),
+        VMSTATE_UINT32(control_regs.impl, AHCIState),
+        VMSTATE_UINT32(control_regs.version, AHCIState),
+        VMSTATE_UINT32(idp_index, AHCIState),
+        VMSTATE_INT32(ports, AHCIState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 typedef struct SysbusAHCIState {
     SysBusDevice busdev;
     AHCIState ahci;
@@ -1207,7 +1282,10 @@ typedef struct SysbusAHCIState {
 
 static const VMStateDescription vmstate_sysbus_ahci = {
     .name = "sysbus-ahci",
-    .unmigratable = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_AHCI(ahci, AHCIPCIState),
+        VMSTATE_END_OF_LIST()
+    },
 };
 
 static void sysbus_ahci_reset(DeviceState *dev)
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 735b379..67b818f 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -305,6 +305,16 @@ typedef struct AHCIPCIState {
     AHCIState ahci;
 } AHCIPCIState;
 
+extern const VMStateDescription vmstate_ahci;
+
+#define VMSTATE_AHCI(_field, _state) {                               \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(AHCIState),                                 \
+    .vmsd       = &vmstate_ahci,                                     \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, AHCIState),   \
+}
+
 typedef struct NCQFrame {
     uint8_t fis_type;
     uint8_t c;
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index de39b30..0c4206a 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -79,9 +79,14 @@
 #define ICH9_IDP_INDEX          0x10
 #define ICH9_IDP_INDEX_LOG2     0x04
 
-static const VMStateDescription vmstate_ahci = {
+static const VMStateDescription vmstate_ich9_ahci = {
     .name = "ahci",
-    .unmigratable = 1,
+    .version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(card, AHCIPCIState),
+        VMSTATE_AHCI(ahci, AHCIPCIState),
+        VMSTATE_END_OF_LIST()
+    },
 };
 
 static void pci_ich9_reset(DeviceState *dev)
@@ -152,7 +157,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82801IR;
     k->revision = 0x02;
     k->class_id = PCI_CLASS_STORAGE_SATA;
-    dc->vmsd = &vmstate_ahci;
+    dc->vmsd = &vmstate_ich9_ahci;
     dc->reset = pci_ich9_reset;
 }
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH v3 2/2] ahci: add migration support

[Kevin Wolf]

Am 04.01.2013 20:44, schrieb Jason Baron:
> From: Jason Baron <jbaron@redhat.com>
> 
> I've tested these patches by migrating Windows 7 and Fedora 17 guests (while
> uunder i/o) on both piix with ahci attached and on q35 (which has a built-in
> ahci controller).
> 
> Changes from v2:
>  -migrate all relevant ahci fields
>  -flush any pending i/o in 'post_load'
> 
> Changes from v1:
>  -extend Andreas Färber's patch
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>

I have a few comments below, but in general this seems to get migration
right for AHCI in its current state.

Unfortunately I noticed only now that AHCI completely ignores -drive
werror/rerror=stop. Once we introduce this, we'll probably get some more
state that needs to be transferred. We'll have to introduce a subsection
then, which isn't nice, but I guess good enough that it shouldn't block
this patch.

> ---
>  hw/ide/ahci.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/ide/ahci.h |   10 +++++++
>  hw/ide/ich.c  |   11 +++++--
>  3 files changed, 97 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 72cd1c8..96f224b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1199,6 +1199,81 @@ void ahci_reset(AHCIState *s)
>      }
>  }
>  
> +static const VMStateDescription vmstate_ahci_device = {
> +    .name = "ahci port",
> +    .version_id = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_IDE_BUS(port, AHCIDevice),
> +        VMSTATE_UINT32(port_state, AHCIDevice),
> +        VMSTATE_UINT32(finished, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.lst_addr, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.lst_addr_hi, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.fis_addr, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.fis_addr_hi, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.irq_stat, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.irq_mask, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.cmd, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.tfdata, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.sig, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.scr_stat, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.scr_ctl, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.scr_err, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.scr_act, AHCIDevice),
> +        VMSTATE_UINT32(port_regs.cmd_issue, AHCIDevice),
> +        VMSTATE_INT32(done_atapi_packet, AHCIDevice),

You should change the type of the struct field from int to int32_t then,
I guess.

> +        VMSTATE_INT32(busy_slot, AHCIDevice),
> +        VMSTATE_INT32(init_d2h_sent, AHCIDevice),

For these two as well.

> +        VMSTATE_END_OF_LIST()
> +    },
> +};

Fields from the struct not being saved are:

- dma: Immutable, ok
- port_no: Immutable, ok
- hba: Immutable, ok
- check_bh: Handled in post_load, ok
- lst: Handled in post_load, ok
- res_fis: Handled in post_load, ok
- cur_cmd: Handled in post_load by check_cmd(), probably ok
- ncq_tfs: AIO is flushed before migration, so it's unused, ok

> +
> +static int ahci_state_post_load(void *opaque, int version_id)
> +{
> +    int i;
> +    struct AHCIDevice *ad;
> +    AHCIState *s = opaque;
> +
> +    for (i = 0; i < s->ports; i++) {
> +        ad = &s->dev[i];
> +        AHCIPortRegs *pr = &ad->port_regs;
> +
> +        map_page(&ad->lst,
> +                 ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
> +        map_page(&ad->res_fis,
> +                 ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
> +        /*
> +         * All pending i/o should be flushed out on a migrate. However,
> +         * we might not have cleared the busy_slot since this is done
> +         * in a bh. Also, issue i/o against any slots that are pending.
> +         */
> +        if (ad->busy_slot != -1) {

The original condition in ahci_check_cmd_bh was:

    if ((ad->busy_slot != -1) &&
        !(ad->port.ifs[0].status & (BUSY_STAT|DRQ_STAT))) {

Under the assumption that no I/O is in flight, I guess omitting the
condition isn't wrong. If it doesn't hurt, I'd prefer to keep it around,
though, because I think the assumption won't hold true in the long term
when werror/rerror support is introduced.

> +            pr->cmd_issue &= ~(1 << ad->busy_slot);
> +            ad->busy_slot = -1;
> +        }
> +        check_cmd(s, i);
> +    }
> +
> +    return 0;
> +}
> +
> +const VMStateDescription vmstate_ahci = {
> +    .name = "ahci",
> +    .version_id = 1,
> +    .post_load = ahci_state_post_load,
> +    .fields = (VMStateField []) {
> +        VMSTATE_STRUCT_VARRAY_POINTER_INT32(dev, AHCIState, ports,
> +                                     vmstate_ahci_device, AHCIDevice),
> +        VMSTATE_UINT32(control_regs.cap, AHCIState),
> +        VMSTATE_UINT32(control_regs.ghc, AHCIState),
> +        VMSTATE_UINT32(control_regs.irqstatus, AHCIState),
> +        VMSTATE_UINT32(control_regs.impl, AHCIState),
> +        VMSTATE_UINT32(control_regs.version, AHCIState),
> +        VMSTATE_UINT32(idp_index, AHCIState),
> +        VMSTATE_INT32(ports, AHCIState),

Another int -> int32_t

> +        VMSTATE_END_OF_LIST()
> +    },

Fields from the struct not being saved are:

- mem, idp: Immutable, ok
- idp_offset: Immutable, ok
- irq: Immutable, ok
- dma_context: Immutable, ok

> +};
> +
>  typedef struct SysbusAHCIState {
>      SysBusDevice busdev;
>      AHCIState ahci;
> @@ -1207,7 +1282,10 @@ typedef struct SysbusAHCIState {
>  
>  static const VMStateDescription vmstate_sysbus_ahci = {
>      .name = "sysbus-ahci",
> -    .unmigratable = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_AHCI(ahci, AHCIPCIState),
> +        VMSTATE_END_OF_LIST()
> +    },
>  };
>  
>  static void sysbus_ahci_reset(DeviceState *dev)

Kevin

Re: [Qemu-devel] [PATCH v3 2/2] ahci: add migration support

[Juan Quintela]

Jason Baron <jbaron@redhat.com> wrote:
> From: Jason Baron <jbaron@redhat.com>
>
> I've tested these patches by migrating Windows 7 and Fedora 17 guests (while
> uunder i/o) on both piix with ahci attached and on q35 (which has a built-in
> ahci controller).
>
> Changes from v2:
>  -migrate all relevant ahci fields
>  -flush any pending i/o in 'post_load'
>
> Changes from v1:
>  -extend Andreas Färber's patch
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
> ---
>  hw/ide/ahci.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/ide/ahci.h |   10 +++++++
>  hw/ide/ich.c  |   11 +++++--
>  3 files changed, 97 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 72cd1c8..96f224b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1199,6 +1199,81 @@ void ahci_reset(AHCIState *s)
>      }
>  }
>  
> +static const VMStateDescription vmstate_ahci_device = {
> +    .name = "ahci port",

I will try to be consistent here, vmstate_ahci_port or "ahci device"

> index de39b30..0c4206a 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -79,9 +79,14 @@
>  #define ICH9_IDP_INDEX          0x10
>  #define ICH9_IDP_INDEX_LOG2     0x04
>  
> -static const VMStateDescription vmstate_ahci = {
> +static const VMStateDescription vmstate_ich9_ahci = {
>      .name = "ahci",
> -    .unmigratable = 1,


I will also preffer here to be consistent between name of the variable
ad name of the section.

> +    .version_id = 1,
> +    .fields = (VMStateField []) {
> +        VMSTATE_PCI_DEVICE(card, AHCIPCIState),
> +        VMSTATE_AHCI(ahci, AHCIPCIState),
> +        VMSTATE_END_OF_LIST()
> +    },
>  };
>  
>  static void pci_ich9_reset(DeviceState *dev)
> @@ -152,7 +157,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data)
>      k->device_id = PCI_DEVICE_ID_INTEL_82801IR;
>      k->revision = 0x02;
>      k->class_id = PCI_CLASS_STORAGE_SATA;
> -    dc->vmsd = &vmstate_ahci;
> +    dc->vmsd = &vmstate_ich9_ahci;


Both comments are stylist, nothing big.  Change if you have to
resent/whoever merge it.

Kevin?

Later, Juan.

Re: [Qemu-devel] [PATCH v3 2/2] ahci: add migration support

[Kevin Wolf]

Am 15.01.2013 15:54, schrieb Juan Quintela:
> Jason Baron <jbaron@redhat.com> wrote:
>> From: Jason Baron <jbaron@redhat.com>
>>
>> I've tested these patches by migrating Windows 7 and Fedora 17 guests (while
>> uunder i/o) on both piix with ahci attached and on q35 (which has a built-in
>> ahci controller).
>>
>> Changes from v2:
>>  -migrate all relevant ahci fields
>>  -flush any pending i/o in 'post_load'
>>
>> Changes from v1:
>>  -extend Andreas Färber's patch
>>
>> Signed-off-by: Jason Baron <jbaron@redhat.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
>> ---
>>  hw/ide/ahci.c |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  hw/ide/ahci.h |   10 +++++++
>>  hw/ide/ich.c  |   11 +++++--
>>  3 files changed, 97 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 72cd1c8..96f224b 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -1199,6 +1199,81 @@ void ahci_reset(AHCIState *s)
>>      }
>>  }
>>  
>> +static const VMStateDescription vmstate_ahci_device = {
>> +    .name = "ahci port",
> 
> I will try to be consistent here, vmstate_ahci_port or "ahci device"

Hm, I think calling it port makes much clearer what it really is, but
the struct is called AHCIDevice. Not sure if renaming it is worth it,
but if you all think it is, I can do it.

>> index de39b30..0c4206a 100644
>> --- a/hw/ide/ich.c
>> +++ b/hw/ide/ich.c
>> @@ -79,9 +79,14 @@
>>  #define ICH9_IDP_INDEX          0x10
>>  #define ICH9_IDP_INDEX_LOG2     0x04
>>  
>> -static const VMStateDescription vmstate_ahci = {
>> +static const VMStateDescription vmstate_ich9_ahci = {
>>      .name = "ahci",
>> -    .unmigratable = 1,
> 
> 
> I will also preffer here to be consistent between name of the variable
> ad name of the section.

Ok, I'll change this one.

> Both comments are stylist, nothing big.  Change if you have to
> resent/whoever merge it.
> 
> Kevin?

I'm addressing my own comments right now and will send a v4.

Kevin