All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
@ 2017-06-29  4:26 Peter Xu
  2017-06-29 13:35 ` Michael S. Tsirkin
  2017-06-30  3:12 ` Jason Wang
  0 siblings, 2 replies; 4+ messages in thread
From: Peter Xu @ 2017-06-29  4:26 UTC (permalink / raw)
  To: qemu-devel
  Cc: Juan Quintela, peterx, Jason Wang, Michael S . Tsirkin,
	Laurent Vivier, Dr . David Alan Gilbert

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
  2017-06-29  4:26 [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch Peter Xu
@ 2017-06-29 13:35 ` Michael S. Tsirkin
  2017-06-30  6:30   ` Peter Xu
  2017-06-30  3:12 ` Jason Wang
  1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2017-06-29 13:35 UTC (permalink / raw)
  To: Peter Xu
  Cc: qemu-devel, Juan Quintela, Jason Wang, Laurent Vivier,
	Dr . David Alan Gilbert

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
  2017-06-29  4:26 [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch Peter Xu
  2017-06-29 13:35 ` Michael S. Tsirkin
@ 2017-06-30  3:12 ` Jason Wang
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Wang @ 2017-06-30  3:12 UTC (permalink / raw)
  To: Peter Xu, qemu-devel
  Cc: Laurent Vivier, Michael S . Tsirkin, Juan Quintela,
	Dr . David Alan Gilbert



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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch
  2017-06-29 13:35 ` Michael S. Tsirkin
@ 2017-06-30  6:30   ` Peter Xu
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Xu @ 2017-06-30  6:30 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, Juan Quintela, Jason Wang, Laurent Vivier,
	Dr . David Alan Gilbert

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-06-30  6:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29  4:26 [Qemu-devel] [PATCH] intel_iommu: fix migration breakage on mr switch Peter Xu
2017-06-29 13:35 ` Michael S. Tsirkin
2017-06-30  6:30   ` Peter Xu
2017-06-30  3:12 ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.