Re: [PATCH 1/1] xen:vtd: missing RMRR mapping while share EPT

["Jan Beulich" <JBeulich@suse.com>]

>>> On 23.07.14 at 11:35, <tiejun.chen@intel.com> wrote:
> intel_iommu_map_page() does nothing if VT-d shares EPT page table.
> So rmrr_identity_mapping() never create RMRR mapping but in some
> cases like some GFX drivers it still need to access RMRR.
> 
> Here we will create those RMRR mappings even in shared EPT case.

Technically this is fine, but there are mechanical issues:

> @@ -1867,7 +1869,19 @@ static int rmrr_identity_mapping(struct domain *d,
>  
>      while ( base_pfn < end_pfn )
>      {
> -        if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> +        if ( iommu_use_hap_pt(d) ) {
> +            dprintk(XENLOG_DEBUG VTDPREFIX,
> +                    "Set RMRR mapping: pfn:0x%lx mfn:0x%lx.\n",
> +                    base_pfn, mfn_x(_mfn(base_pfn)));

Do we really need this message, even more so not at guest level?
And if you're convinced we need it, please use %#lx instead of
0x%lx.

> +            p2m_lock(p2m);
> +            if ( p2m_set_entry(p2m, base_pfn, _mfn(base_pfn), PAGE_ORDER_4K,
> +                    p2m_mmio_direct, p2m_access_rw) ) {
> +                p2m_unlock(p2m);
> +                return -1;
> +            }
> +            p2m_unlock(p2m);
> +        }
> +        else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
>                                    IOMMUF_readable|IOMMUF_writable) )
>              return -1;
>          base_pfn++;

Apart from the above there are several coding style issues here.

Jan