This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


RE: [Xen-devel] [PATCH][VTD] bug fix for EPT/VT-d table sharing

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH][VTD] bug fix for EPT/VT-d table sharing
From: "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
Date: Thu, 6 Jan 2011 18:49:31 -0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Delivery-date: Thu, 06 Jan 2011 18:50:35 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D258EB2020000780002AA9F@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <987664A83D2D224EAE907B061CE93D530193FC6195@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4D258EB2020000780002AA9F@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcutfcUcNoeZASpVRtmK+evhDbXKUAAlqyvw
Thread-topic: [Xen-devel] [PATCH][VTD] bug fix for EPT/VT-d table sharing
Thanks for your feedback.  I have incorporated all of them in the attached 
patch.  Hopefully my understanding of gfn is correct now.  Also I have found an 
pre-existing EPT 1GB page issue.  I have included the fix in this patch also:

-                if ( order == EPT_TABLE_ORDER )
+                if ( order > 0 )

I also remembered to use "hg diff -p" this time to make review easier.


-----Original Message-----
From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx] 
Sent: Thursday, January 06, 2011 12:43 AM
To: Kay, Allen M
Cc: Tim Deegan; xen-devel@xxxxxxxxxxxxxxxxxxx; Keir Fraser
Subject: Re: [Xen-devel] [PATCH][VTD] bug fix for EPT/VT-d table sharing

>>> On 05.01.11 at 20:36, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
>@@ -1729,13 +1732,15 @@
>     return 0;
> }
>-void iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte, int present)
>+void iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
>+                     u32 order, int present)

u32? Shouldn't fixed-size types only be used where really necessary?

> {
>     struct acpi_drhd_unit *drhd;
>     struct iommu *iommu = NULL;
>     struct hvm_iommu *hd = domain_hvm_iommu(d);
>     int flush_dev_iotlb;
>     int iommu_domid;
>+    int page_shift = (order == 0) ? PAGE_SHIFT_4K : PAGE_SHIFT_4K + order;

Why not simply 

    int page_shift = PAGE_SHIFT_4K + order;

>     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>@@ -1750,8 +1755,8 @@
>         if ( iommu_domid == -1 )
>             continue;
>         if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
>-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 1,
>-                                   !present, flush_dev_iotlb) )
>+                                   (paddr_t)gfn << page_shift, 1,

This certainly isn't correct, at least as long as "gfn" is what its
name says (and since the only caller simply adds an "order"
argument without adjusting the "gfn" one, I'm sure it is).

>+                                   page_shift, !present, flush_dev_iotlb) )
>             iommu_flush_write_buffer(iommu);
>     }
> }


Attachment: share0106.patch
Description: share0106.patch

Xen-devel mailing list