Re: [Xen-devel] [PATCH v2 4/4] x86/dom0: re-order DMA remapping enabling for PVH Dom0

2017-08-23 Thread Jan Beulich 
>>> On 22.08.17 at 16:05,  wrote:
> On Tue, Aug 22, 2017 at 06:37:15AM -0600, Jan Beulich wrote:
>> >>> On 11.08.17 at 18:43,  wrote:
>> > Make sure the reserved regions are setup before enabling the DMA
>> > remapping in the IOMMU, by calling dom0_setup_permissions before
>> > iommu_hwdom_init.
>> 
>> I can't match up this part with ...
>> 
>> > --- a/xen/arch/x86/hvm/dom0_build.c
>> > +++ b/xen/arch/x86/hvm/dom0_build.c
>> > @@ -605,13 +605,6 @@ static int __init pvh_setup_cpus(struct domain *d, 
>> > paddr_t entry,
>> >  return rc;
>> >  }
>> >  
>> > -rc = dom0_setup_permissions(d);
>> > -if ( rc )
>> > -{
>> > -panic("Unable to setup Dom0 permissions: %d\n", rc);
>> > -return rc;
>> > -}
>> > -
>> >  update_domain_wallclock_time(d);
>> >  
>> >  clear_bit(_VPF_down, >pause_flags);
>> > @@ -1059,7 +1052,12 @@ int __init dom0_construct_pvh(struct domain *d, 
>> > const module_t *image,
>> >  
>> >  printk("** Building a PVH Dom0 **\n");
>> >  
>> > -iommu_hwdom_init(d);
>> > +rc = dom0_setup_permissions(d);
>> > +if ( rc )
>> > +{
>> > +printk("Unable to setup Dom0 permissions: %d\n", rc);
>> > +return rc;
>> > +}
>> >  
>> >  rc = pvh_setup_p2m(d);
>> >  if ( rc )
>> > @@ -1068,6 +1066,8 @@ int __init dom0_construct_pvh(struct domain *d, 
>> > const module_t *image,
>> >  return rc;
>> >  }
>> >  
>> > +iommu_hwdom_init(d);
>> 
>> ... you not changing the relative order between these two function
>> calls. As to the other half I'm inclined to also wait for better
>> understanding of what's going on here, as said in reply to patch 3.
> 
> Why not?

Oh, I'm sorry - I should have looked at function names in the
hunk headers instead of just the sequence of hunks.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 4/4] x86/dom0: re-order DMA remapping enabling for PVH Dom0

2017-08-22 Thread Roger Pau Monne 
On Tue, Aug 22, 2017 at 06:37:15AM -0600, Jan Beulich wrote:
> >>> On 11.08.17 at 18:43,  wrote:
> > Make sure the reserved regions are setup before enabling the DMA
> > remapping in the IOMMU, by calling dom0_setup_permissions before
> > iommu_hwdom_init.
> 
> I can't match up this part with ...
> 
> > --- a/xen/arch/x86/hvm/dom0_build.c
> > +++ b/xen/arch/x86/hvm/dom0_build.c
> > @@ -605,13 +605,6 @@ static int __init pvh_setup_cpus(struct domain *d, 
> > paddr_t entry,
> >  return rc;
> >  }
> >  
> > -rc = dom0_setup_permissions(d);
> > -if ( rc )
> > -{
> > -panic("Unable to setup Dom0 permissions: %d\n", rc);
> > -return rc;
> > -}
> > -
> >  update_domain_wallclock_time(d);
> >  
> >  clear_bit(_VPF_down, >pause_flags);
> > @@ -1059,7 +1052,12 @@ int __init dom0_construct_pvh(struct domain *d, 
> > const module_t *image,
> >  
> >  printk("** Building a PVH Dom0 **\n");
> >  
> > -iommu_hwdom_init(d);
> > +rc = dom0_setup_permissions(d);
> > +if ( rc )
> > +{
> > +printk("Unable to setup Dom0 permissions: %d\n", rc);
> > +return rc;
> > +}
> >  
> >  rc = pvh_setup_p2m(d);
> >  if ( rc )
> > @@ -1068,6 +1066,8 @@ int __init dom0_construct_pvh(struct domain *d, const 
> > module_t *image,
> >  return rc;
> >  }
> >  
> > +iommu_hwdom_init(d);
> 
> ... you not changing the relative order between these two function
> calls. As to the other half I'm inclined to also wait for better
> understanding of what's going on here, as said in reply to patch 3.

Why not?

dom0_setup_permissions was called from pvh_setup_cpus, while
iommu_hwdom_init was the first function called in
dom0_construct_pvh.

After this patch dom0_setup_permissions is always called before
iommu_hwdom_init.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 4/4] x86/dom0: re-order DMA remapping enabling for PVH Dom0

2017-08-22 Thread Jan Beulich 
>>> On 11.08.17 at 18:43,  wrote:
> Make sure the reserved regions are setup before enabling the DMA
> remapping in the IOMMU, by calling dom0_setup_permissions before
> iommu_hwdom_init.

I can't match up this part with ...

> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -605,13 +605,6 @@ static int __init pvh_setup_cpus(struct domain *d, 
> paddr_t entry,
>  return rc;
>  }
>  
> -rc = dom0_setup_permissions(d);
> -if ( rc )
> -{
> -panic("Unable to setup Dom0 permissions: %d\n", rc);
> -return rc;
> -}
> -
>  update_domain_wallclock_time(d);
>  
>  clear_bit(_VPF_down, >pause_flags);
> @@ -1059,7 +1052,12 @@ int __init dom0_construct_pvh(struct domain *d, const 
> module_t *image,
>  
>  printk("** Building a PVH Dom0 **\n");
>  
> -iommu_hwdom_init(d);
> +rc = dom0_setup_permissions(d);
> +if ( rc )
> +{
> +printk("Unable to setup Dom0 permissions: %d\n", rc);
> +return rc;
> +}
>  
>  rc = pvh_setup_p2m(d);
>  if ( rc )
> @@ -1068,6 +1066,8 @@ int __init dom0_construct_pvh(struct domain *d, const 
> module_t *image,
>  return rc;
>  }
>  
> +iommu_hwdom_init(d);

... you not changing the relative order between these two function
calls. As to the other half I'm inclined to also wait for better
understanding of what's going on here, as said in reply to patch 3.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 4/4] x86/dom0: re-order DMA remapping enabling for PVH Dom0

2017-08-11 Thread Roger Pau Monne 
Make sure the reserved regions are setup before enabling the DMA
remapping in the IOMMU, by calling dom0_setup_permissions before
iommu_hwdom_init. Also, in order to workaround IOMMU issues seen on
pre-Haswell Intel hardware, as described in patch "introduce a PVH
implementation of iommu_inclusive_mapping" make sure the DMA remapping
is enabled after populating Dom0 p2m.

Signed-off-by: Roger Pau Monné 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
Changes since RFC:
 - Expand commit message to reference patch #3.
---
 xen/arch/x86/hvm/dom0_build.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 020c355faf..0e7d06be95 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -605,13 +605,6 @@ static int __init pvh_setup_cpus(struct domain *d, paddr_t 
entry,
 return rc;
 }
 
-rc = dom0_setup_permissions(d);
-if ( rc )
-{
-panic("Unable to setup Dom0 permissions: %d\n", rc);
-return rc;
-}
-
 update_domain_wallclock_time(d);
 
 clear_bit(_VPF_down, >pause_flags);
@@ -1059,7 +1052,12 @@ int __init dom0_construct_pvh(struct domain *d, const 
module_t *image,
 
 printk("** Building a PVH Dom0 **\n");
 
-iommu_hwdom_init(d);
+rc = dom0_setup_permissions(d);
+if ( rc )
+{
+printk("Unable to setup Dom0 permissions: %d\n", rc);
+return rc;
+}
 
 rc = pvh_setup_p2m(d);
 if ( rc )
@@ -1068,6 +1066,8 @@ int __init dom0_construct_pvh(struct domain *d, const 
module_t *image,
 return rc;
 }
 
+iommu_hwdom_init(d);
+
 rc = pvh_load_kernel(d, image, image_headroom, initrd, 
bootstrap_map(image),
  cmdline, , _info);
 if ( rc )
-- 
2.11.0 (Apple Git-81)


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel