Re: [Xen-devel] [v6][PATCH 2/2] xen:vtd: missing RMRR mapping while share EPT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.07.14 at 03:36, <tiejun.chen@xxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1867,8 +1867,14 @@ static int rmrr_identity_mapping(struct domain *d,
>  
>      while ( base_pfn < end_pfn )
>      {
> -        if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> -                                  IOMMUF_readable|IOMMUF_writable) )
> +        if ( iommu_use_hap_pt(d) )
> +        {
> +            ASSERT(!iommu_passthrough || !is_hardware_domain(d));
> +            if ( set_identity_p2m_entry(d, base_pfn) )
> +                return -1;
> +        }
> +        else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> +                                       IOMMUF_readable|IOMMUF_writable) )
>              return -1;
>          base_pfn++;
>      }

So I gave this a try on the one box I have which exposes RMRRs
(since those are for USB devices I also used your patch to drop
the USB special casing as done in your later patch series, and I
further had to fiddle with vtd_ept_page_compatible() in order to
get page table sharing to actually work on that box [I'll send the
resulting patch later]) - with the result that passing through an
affected USB controller (as expected) doesn't work anymore. Which
raises the question why the two patches alone would work at all.
Could you please share information on the address ranges specified
by the RMRR(s) in your case? I simply wonder whether things just
happen to work for you on the particular system(s) you're testing
on, as I'd generally expect an address space collision to be possible
for any RMRR.

I think you understand the consequences: If the series here has no
way of reliably working without the other one, "iommu=no-sharept"
is going to be the solution for you, at once being one more argument
towards dropping page table sharing altogether. The one argument
in favor of the two patches here would be that they at least detect
the collision now, thus forcing people to suppress page table sharing.

But what's worse, I can't see how the non-sharing case is being
handled correctly either (independent of the series here):
rmrr_identity_mapping() blindly overwrites what may already be
in the page tables, breaking consistency with the CPU-side P2M
(iiuc this is a problem even for PV, including Dom0). Plus there's
nothing being done to prevent subsequent overwriting of these
1:1 entries by "normal" P2M manipulations. All in all another
argument not to allow (at least by default) passing through of
devices associated with one or more RMRRs.

Jan


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