[PATCH v3 0/3] Fix xen crash when starting HVM guest due to missing io handler

[PATCH v3 0/3] Fix xen crash when starting HVM guest due to missing io handler

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Hi All,

Changes from V2:
  * Use assert instead of sanity check before count increment in
    the hvm_next_io_handler().
  * Post-pone iommu_domain_init() and add proper error handling code
    to destroy hvm in case of failure.
  * Split out sanity check in guest_iommu_init() into a separate patch.

OVERVIEW:
 
On systems with iommu v2 enabled, the hypervisor crashes when trying
to start up an HVM guest. 

Investigating shows that the guest_iommu_init() is called before the
HVM domain is initialized. It then tries to register_mmio_handler()
causing the hvm_next_io_handler() to increment the io_handler_count.
However, the registration fails silently and left the I/O handler
uninitialized.

At later time, hvm_find_io_handler() is called and iterate through
the registered handlered, but then resulting in referencing NULL
pointers.

This patch series proposes fix for this issue.

Thanks,
Suravee

Suravee Suthikulpanit (3):
  x86/hvm: Add check when register io handler
  svm: iommu: Only call guest_iommu_init() after initialized HVM domain
  AMD IOMMU: Check io_handler before registering mmio handler

 xen/arch/x86/domain.c                     | 9 ++++++---
 xen/arch/x86/hvm/intercept.c              | 2 ++
 xen/drivers/passthrough/amd/iommu_guest.c | 6 ++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
1.9.1


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

[PATCH v3 1/3] x86/hvm: Add check when register io handler

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

At the time of registering HVM I/O handler, the HVM domain might
not have been initialized, which means the hvm_domain.io_handler
would be NULL. In the hvm_next_io_handler(), this should be asserted.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/arch/x86/hvm/intercept.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c
index fc757d0..2f8d57f 100644
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -258,6 +258,8 @@ struct hvm_io_handler *hvm_next_io_handler(struct domain *d)
 {
     unsigned int i = d->arch.hvm_domain.io_handler_count++;
 
+    ASSERT( d->arch.hvm_domain.io_handler );
+
     if ( i == NR_IO_HANDLERS )
     {
         domain_crash(d);
-- 
1.9.1


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

[PATCH v3 2/3] svm: iommu: Only call guest_iommu_init() after initialized HVM domain

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

The guest_iommu_init() is currently called by the following code path:

    arch/x86/domain.c: arch_domain_create()
      ]- drivers/passthrough/iommu.c: iommu_domain_init()
        |- drivers/passthrough/amd/pci_amd_iommu.c: amd_iommu_domain_init();
          |- drivers/passthrough/amd/iommu_guest.c: guest_iommu_init()

At this point, the hvm_domain_initialised() has not been called.
So register_mmio_handler() in guest_iommu_init() silently fails.
This patch moves the iommu_domain_init() to a later point after the
hvm_domain_intialise() instead.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/arch/x86/domain.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 5af2cc5..0260e01 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -642,9 +642,6 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
 
         if ( (rc = init_domain_irq_mapping(d)) != 0 )
             goto fail;
-
-        if ( (rc = iommu_domain_init(d)) != 0 )
-            goto fail;
     }
     spin_lock_init(&d->arch.e820_lock);
 
@@ -660,6 +657,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
         /* 64-bit PV guest by default. */
         d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
 
+    if ( !is_idle_domain(d) && (rc = iommu_domain_init(d)) != 0 )
+        goto fail_1;
+
     /* initialize default tsc behavior in case tools don't */
     tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
     spin_lock_init(&d->arch.vtsc_lock);
@@ -675,6 +675,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
 
     return 0;
 
+ fail_1:
+    if ( has_hvm_container_domain(d) )
+        hvm_domain_destroy(d);
  fail:
     d->is_dying = DOMDYING_dead;
     psr_domain_free(d);
-- 
1.9.1


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

Re: [PATCH v3 0/3] Fix xen crash when starting HVM guest due to missing io handler

[Suravee Suthikulpanit]

+ Rüdiger

This patch series should help fixing the issue you are seeing.

Thanks,
Suravee

On 05/21/2016 06:42 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>
> Hi All,
>
> Changes from V2:
>    * Use assert instead of sanity check before count increment in
>      the hvm_next_io_handler().
>    * Post-pone iommu_domain_init() and add proper error handling code
>      to destroy hvm in case of failure.
>    * Split out sanity check in guest_iommu_init() into a separate patch.
>
> OVERVIEW:
>
> On systems with iommu v2 enabled, the hypervisor crashes when trying
> to start up an HVM guest.
>
> Investigating shows that the guest_iommu_init() is called before the
> HVM domain is initialized. It then tries to register_mmio_handler()
> causing the hvm_next_io_handler() to increment the io_handler_count.
> However, the registration fails silently and left the I/O handler
> uninitialized.
>
> At later time, hvm_find_io_handler() is called and iterate through
> the registered handlered, but then resulting in referencing NULL
> pointers.
>
> This patch series proposes fix for this issue.
>
> Thanks,
> Suravee
>
> Suravee Suthikulpanit (3):
>    x86/hvm: Add check when register io handler
>    svm: iommu: Only call guest_iommu_init() after initialized HVM domain
>    AMD IOMMU: Check io_handler before registering mmio handler
>
>   xen/arch/x86/domain.c                     | 9 ++++++---
>   xen/arch/x86/hvm/intercept.c              | 2 ++
>   xen/drivers/passthrough/amd/iommu_guest.c | 6 ++++++
>   3 files changed, 14 insertions(+), 3 deletions(-)
>

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

[PATCH v3 0/3] Fix xen crash when starting HVM guest due to missing io handler

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Hi All,

Changes from V2:
  * Use assert instead of sanity check before count increment in
    the hvm_next_io_handler().
  * Post-pone iommu_domain_init() and add proper error handling code
    to destroy hvm in case of failure.
  * Split out sanity check in guest_iommu_init() into a separate patch.

OVERVIEW:
 
On systems with iommu v2 enabled, the hypervisor crashes when trying
to start up an HVM guest. 

Investigating shows that the guest_iommu_init() is called before the
HVM domain is initialized. It then tries to register_mmio_handler()
causing the hvm_next_io_handler() to increment the io_handler_count.
However, the registration fails silently and left the I/O handler
uninitialized.

At later time, hvm_find_io_handler() is called and iterate through
the registered handlered, but then resulting in referencing NULL
pointers.

This patch series proposes fix for this issue.

Thanks,
Suravee

Suravee Suthikulpanit (3):
  x86/hvm: Add check when register io handler
  svm: iommu: Only call guest_iommu_init() after initialized HVM domain
  AMD IOMMU: Check io_handler before registering mmio handler

 xen/arch/x86/domain.c                     | 9 ++++++---
 xen/arch/x86/hvm/intercept.c              | 2 ++
 xen/drivers/passthrough/amd/iommu_guest.c | 6 ++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
1.9.1


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