1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch

Peter Xu posted 1 patch 6 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1498710417-7807-1-git-send-email-peterx@redhat.com
There is a newer version of this series
hw/i386/intel_iommu.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
Posted by Peter Xu 6 years, 10 months ago 
Migration is broken after the vfio integration work:

qemu-kvm: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
qemu-kvm: Failed to load ich9_ahci:ahci
qemu-kvm: error while loading state for instance 0x0 of device '0000:00:1f.2/ich9_ahci'
qemu-kvm: load of migration failed: Operation not permitted

The problem is that vfio work introduced dynamic memory region
switching (actually it is also used for future PT mode), and this memory
region layout is not properly delivered to destination when migration
happens. Solution is to rebuild the layout in post_load.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1459906
Fixes: 558e0024 ("intel_iommu: allow dynamic switch of IOMMU region")
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index a9b59bd..36b231d 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2332,11 +2332,28 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
     }
 }
 
+static int vtd_post_load(void *opaque, int version_id)
+{
+    IntelIOMMUState *iommu = opaque;
+
+    /*
+     * Since commit 558e00 ("intel_iommu: allow dynamic switch of
+     * IOMMU region"), memory regions are dynamically turned on/off
+     * depending on context entry configurations from the guest. After
+     * migration, we need to make sure the memory regions are still
+     * correct.
+     */
+    vtd_switch_address_space_all(iommu);
+
+    return 0;
+}
+
 static const VMStateDescription vtd_vmstate = {
     .name = "iommu-intel",
     .version_id = 1,
     .minimum_version_id = 1,
     .priority = MIG_PRI_IOMMU,
+    .post_load = vtd_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT64(root, IntelIOMMUState),
         VMSTATE_UINT64(intr_root, IntelIOMMUState),
-- 
2.7.4
Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
Posted by Michael S. Tsirkin 6 years, 10 months ago 
On Thu, Jun 29, 2017 at 12:26:57PM +0800, Peter Xu wrote:
> Migration is broken after the vfio integration work:
> 
> qemu-kvm: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
> qemu-kvm: Failed to load ich9_ahci:ahci
> qemu-kvm: error while loading state for instance 0x0 of device '0000:00:1f.2/ich9_ahci'
> qemu-kvm: load of migration failed: Operation not permitted
> 
> The problem is that vfio work introduced dynamic memory region
> switching (actually it is also used for future PT mode), and this memory
> region layout is not properly delivered to destination when migration
> happens. Solution is to rebuild the layout in post_load.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1459906
> Fixes: 558e0024 ("intel_iommu: allow dynamic switch of IOMMU region")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/i386/intel_iommu.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index a9b59bd..36b231d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2332,11 +2332,28 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
>      }
>  }
>  
> +static int vtd_post_load(void *opaque, int version_id)
> +{
> +    IntelIOMMUState *iommu = opaque;
> +
> +    /*
> +     * Since commit 558e00 ("intel_iommu: allow dynamic switch of
> +     * IOMMU region"),

I would drop this part of the comment. It isn't of interest
to people reading code directly, while people reading git
history see the commit log.

> memory regions are dynamically turned on/off
> +     * depending on context entry configurations from the guest. After
> +     * migration, we need to make sure the memory regions are still
> +     * correct.
> +     */
> +    vtd_switch_address_space_all(iommu);
> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vtd_vmstate = {
>      .name = "iommu-intel",
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .priority = MIG_PRI_IOMMU,
> +    .post_load = vtd_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT64(root, IntelIOMMUState),
>          VMSTATE_UINT64(intr_root, IntelIOMMUState),
> -- 
> 2.7.4
Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
Posted by Peter Xu 6 years, 9 months ago 
On Thu, Jun 29, 2017 at 04:35:04PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 29, 2017 at 12:26:57PM +0800, Peter Xu wrote:
> > Migration is broken after the vfio integration work:
> > 
> > qemu-kvm: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
> > qemu-kvm: Failed to load ich9_ahci:ahci
> > qemu-kvm: error while loading state for instance 0x0 of device '0000:00:1f.2/ich9_ahci'
> > qemu-kvm: load of migration failed: Operation not permitted
> > 
> > The problem is that vfio work introduced dynamic memory region
> > switching (actually it is also used for future PT mode), and this memory
> > region layout is not properly delivered to destination when migration
> > happens. Solution is to rebuild the layout in post_load.
> > 
> > Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1459906
> > Fixes: 558e0024 ("intel_iommu: allow dynamic switch of IOMMU region")
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  hw/i386/intel_iommu.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index a9b59bd..36b231d 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2332,11 +2332,28 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> >      }
> >  }
> >  
> > +static int vtd_post_load(void *opaque, int version_id)
> > +{
> > +    IntelIOMMUState *iommu = opaque;
> > +
> > +    /*
> > +     * Since commit 558e00 ("intel_iommu: allow dynamic switch of
> > +     * IOMMU region"),
> 
> I would drop this part of the comment. It isn't of interest
> to people reading code directly, while people reading git
> history see the commit log.

Ok. Will resend. Thanks.

-- 
Peter Xu
Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
Posted by Jason Wang 6 years, 9 months ago 

On 2017年06月29日 12:26, Peter Xu wrote:
> Migration is broken after the vfio integration work:
>
> qemu-kvm: AHCI: Failed to start FIS receive engine: bad FIS receive buffer address
> qemu-kvm: Failed to load ich9_ahci:ahci
> qemu-kvm: error while loading state for instance 0x0 of device '0000:00:1f.2/ich9_ahci'
> qemu-kvm: load of migration failed: Operation not permitted
>
> The problem is that vfio work introduced dynamic memory region
> switching (actually it is also used for future PT mode), and this memory
> region layout is not properly delivered to destination when migration
> happens. Solution is to rebuild the layout in post_load.
>
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1459906
> Fixes: 558e0024 ("intel_iommu: allow dynamic switch of IOMMU region")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/i386/intel_iommu.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index a9b59bd..36b231d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2332,11 +2332,28 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
>       }
>   }
>   
> +static int vtd_post_load(void *opaque, int version_id)
> +{
> +    IntelIOMMUState *iommu = opaque;
> +
> +    /*
> +     * Since commit 558e00 ("intel_iommu: allow dynamic switch of
> +     * IOMMU region"), memory regions are dynamically turned on/off
> +     * depending on context entry configurations from the guest. After
> +     * migration, we need to make sure the memory regions are still
> +     * correct.
> +     */
> +    vtd_switch_address_space_all(iommu);
> +
> +    return 0;
> +}
> +
>   static const VMStateDescription vtd_vmstate = {
>       .name = "iommu-intel",
>       .version_id = 1,
>       .minimum_version_id = 1,
>       .priority = MIG_PRI_IOMMU,
> +    .post_load = vtd_post_load,
>       .fields = (VMStateField[]) {
>           VMSTATE_UINT64(root, IntelIOMMUState),
>           VMSTATE_UINT64(intr_root, IntelIOMMUState),

Reviewed-by: Jason Wang <jasowang@redhat.com>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.