Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.08.15 at 17:49, <malcolm.crossley@xxxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
> bool_t map,
>  
>              while ( base_pfn < end_pfn )
>              {
> -                if ( clear_identity_p2m_entry(d, base_pfn) )
> -                    ret = -ENXIO;
> +                if ( iommu_use_hap_pt(d) )
> +                {
> +                    if ( clear_identity_p2m_entry(d, base_pfn) )
> +                            ret = -ENXIO;
> +                }
> +                else
> +                {
> +                    if ( intel_iommu_unmap_page(d, base_pfn) )
> +                        ret = -ENXIO;
> +                }
>                  base_pfn++;
>              }
>  
> @@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
> bool_t map,
>  
>      while ( base_pfn < end_pfn )
>      {
> -        int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
> +        int err;
> +        if ( iommu_use_hap_pt(d) )
> +        {
> +            err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
> +        }
> +        else
> +        {
> +            err = intel_iommu_map_page(d, base_pfn, base_pfn,
> +                                       IOMMUF_readable|IOMMUF_writable);
> +        }
>  
>          if ( err )
>              return err;

Considering that {set,clear}_identity_p2m_entry are really clones of
guest_physmap_{add,remove}_page(), I think it would be more
logical to follow their model of invoking iommu_{,un}map_page()
instead of having the current (and possible future) callers take care
of this for themselves. Which of course then raises the question of
the right predicate: The guest_physmap_* functions check
!paging_mode_translate(), and the !shared-ept case for HVM/PVH
is being covered by {p2m_pt,ept}_set_entry(). Surely asymmetry
here would be at least suspicious.

And which raises a second question (to the VT-d maintainers): Why
is it that intel_iommu_unmap_page() doesn't use
iommu_use_hap_pt() just like intel_iommu_map_page() as well as
amd_iommu_unmap_page() do?

Jan


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