From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [v6][PATCH 2/2] xen:vtd: missing RMRR mapping while
	share EPT
Date: Wed, 30 Jul 2014 16:59:52 +0800
Message-ID: <53D8B408.1010409@intel.com>
References: <1406684186-12788-1-git-send-email-tiejun.chen@intel.com>
	<1406684186-12788-2-git-send-email-tiejun.chen@intel.com>
	<53D8CAC402000078000278E9@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53D8CAC402000078000278E9@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: yang.z.zhang@intel.com, kevin.tian@intel.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 2014/7/30 16:36, Jan Beulich wrote:
>>>> On 30.07.14 at 03:36, <tiejun.chen@intel.com> 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 wonder how this plays together with
>
>      /* FIXME: Because USB RMRR conflicts with guest bios region,
>       * ignore USB RMRR temporarily.
>       */
>      seg = pdev->seg;
>      bus = pdev->bus;
>      if ( is_usb_device(seg, bus, pdev->devfn) )
>      {
>          ret = 0;
>          goto done;
>      }
>
> later in the same file (in intel_iommu_assign_device()). I.e. the
> improvement you claim won't be achieved for passed through USB

I think we can remove this chunk of codes since these two patches 
already check if they conflicts.

> devices afaict. One more aspect supporting my view that this
> needs fully addressing rather than any such partial solution.

If we're talking about fixing all RMRR issues completely, these patches 
should not be enough. But I think these patches shouldn't block that 
complete solution because they're just checking to make sure we can 
create those RMRR mapping if possible. I mean no matter if we address 
more on RMRR, we still need these two patches to double-check/create 
RMRR mapping in shared EPT case, even we will reserve those ranges in 
guest as some guys recommended previously.

Thanks
Tiejun