| Message ID | 20170811164320.92899-5-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 11.08.17 at 18:43, <roger.pau@citrix.com> 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, &v->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
On Tue, Aug 22, 2017 at 06:37:15AM -0600, Jan Beulich wrote: > >>> On 11.08.17 at 18:43, <roger.pau@citrix.com> 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, &v->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.
>>> On 22.08.17 at 16:05, <roger.pau@citrix.com> wrote: > On Tue, Aug 22, 2017 at 06:37:15AM -0600, Jan Beulich wrote: >> >>> On 11.08.17 at 18:43, <roger.pau@citrix.com> 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, &v->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
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, &v->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, &entry, &start_info); if ( rc )
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é <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- 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(-)