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

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 19 May 2016 10:00
> To: Paul Durrant
> Cc: Suravee.Suthikulpanit@amd.com; George Dunlap; xen-
> devel@lists.xen.org; Keir (Xen.org)
> Subject: RE: [PATCH V2 2/2] svm: iommu: Only call guest_iommu_init() after
> initialized HVM domain
> 
> >>> On 19.05.16 at 09:52, <Paul.Durrant@citrix.com> wrote:
> >> >--- a/xen/arch/x86/domain.c
> >> >+++ b/xen/arch/x86/domain.c
> >> >@@ -625,24 +625,21 @@ 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);
> >> >
> >> >if ( has_hvm_container_domain(d) )
> >> >{
> >> >if ( (rc = hvm_domain_initialise(d)) != 0 )
> >> >-        {
> >> >-            iommu_domain_destroy(d);
> >> >goto fail;
> >> >-        }
> >> >}
> >>       >else
> >> >/* 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;
> >>
> >> This would in the error case fail to undo what hvm_domain_initialise() did.
> >> There was a fix very recently dealing with a similar issue. There really
> >> shouldn't be anything that can fail after hvm_domain_initialise().
> >
> > Why is that? There are many failure paths within hvm_domain_initialise().
> > What's wrong with calling hvm_domain_destroy() to undo the whole
> thing?
> 
> Well, I simply didn't check whether hvm_domain_destroy() would be
> suitable to be called in that case, but yes, it looks like it would be.
> 
> > (Although I do notice that the io_bitmap would appear to leak in that
> case...
> > but that looks like a bug to me).
> 
> That's a hardware domain only aspect, and failure to construct
> the hardware domain would be fatal to the system anyway.
>

Ok. No problem then.

  Paul

 > Jan


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