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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.14 at 04:43, <yang.z.zhang@xxxxxxxxx> wrote:
> Jan Beulich wrote on 2014-09-18:
>>>>> 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.
> 
> This is another issue that current RMRR handling logic is not right which I 
> think we have discussed long time ago.
> http://lists.xen.org/archives/html/xen-devel/2014-07/msg03249.html 
> 
> Obviously, current RMRR handling logic is totally wrong. It is not a simple 
> task to do it which I think Tiejun already spent about two months but we 
> still have some divergences. From my point, this patch will mitigate this 
> issue. At least, it doesn't make thing worse.

In fact it does (as pointed out by the tests I conducted for that
very reason) cause a regression (at least a perceived one), which
iirc was already mentioned earlier: Devices associated with RMRRs
where the RMRRs aren't actually getting accessed post-boot can
currently be passed through fine (and without other than a purely
theoretical security implication) now, but won't with the patch in
place.

Apart from that, the absolute minimum of extra work needed here
would imo be to make the separate-pt case detect address space
collisions the same way as the shared-pt case does with the two
patches here in place, so that there's no divergence in behavior.

Yet again - for 4.5 I'd favor disallowing assignment of devices
associated with RMRRs altogether.

Jan


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