WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] [PATCH 0/2] MTRR/PAT virtualization

To: "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>, "Su, Disheng" <disheng.su@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH 0/2] MTRR/PAT virtualization
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Mon, 8 Oct 2007 23:29:41 +0800
Delivery-date: Mon, 08 Oct 2007 08:30:32 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <C32FB165.E882%Keir.Fraser@xxxxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C32FA17F.E879%Keir.Fraser@xxxxxxxxxxxx> <C32FB165.E882%Keir.Fraser@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcgJfbiHYf5t3raCT5qrRGbrSVAc4gAAyqLhAAJeet0ADAbXoA==
Thread-topic: [Xen-devel] [PATCH 0/2] MTRR/PAT virtualization

xen-devel-bounces@xxxxxxxxxxxxxxxxxxx <> wrote:
> A couple of other comments:
> 1. I don't see any changes to shadow code to look at the _PAGE_PAT
bit;
> only _PAGE_PCD and _PAGE_PWT. Is this correct?

I think the changes to shadow code is done in following hunk, which will
update the shadow flag according to guest flag.

@@ -703,6 +713,17 @@ _sh_propagate(struct vcpu *v,
         pass_thru_flags |= _PAGE_NX_BIT;
     sflags = gflags & pass_thru_flags;
 
+    if ( level == 1 ) {
+        /*XXXX:Don't change pat attributes for VRAM spaces,
+         * as it is shared between qemu and domain*/
+        sflags |= get_pat_flags(&v->arch.hvm_vcpu.mtrr,
+                                NULL,
+                                v->arch.hvm_vcpu.pat_cr,
+                                gflags,
+                                guest_l1e_get_paddr(*gp),
+                                target_mfn<<PAGE_SHIFT);
+    }

As the changes to _PAGE_PCD and _PAGE_PWT, not sure if you are talking
about following hunk, which is for flags in super pages.
@@ -267,6 +267,10 @@ guest_walk_tables(struct vcpu *v, unsign
          * us reflect l2 changes later without touching the l1s. */
         int flags = (_PAGE_PRESENT|_PAGE_USER|_PAGE_RW|
                      _PAGE_ACCESSED|_PAGE_DIRTY);
+        if ( (guest_l2e_get_flags(*gw->l2e) & _PAGE_PWT) )
+            flags |= _PAGE_PWT;
+        if ( (guest_l2e_get_flags(*gw->l2e) & _PAGE_PCD) )
+            flags |= _PAGE_PCD


> 2. Introducing non-WB types for normal RAM is potentially a
> problem due to
> attribute mismatches (E.g., mappings in Xen and mappings in
> qemu-dm). Do you
> disallow non-WB types for RAM, or handle this in some other
> way? Disallow
> would probably work fine except for a few cases like AGP GART.
> 
> Given that this is probably only safe for non-RAM pages, and given
that
> usually such mappings will be UC anyway, is there any advantage to
this
> large patch except accelerated access to passed-through video
framebuffer
> access? The current 'collapse all memory types to UC for
> non-RAM mappings'
> is I believe always safe, and video framebuffer access is the
> only case I
> can think of where there would be a performance loss that we might
care
> about. We could probably fix that with a much smaller patch
> with a higher
> chance of acceptance for 3.2.0.

For RAM assigned to guest, the patch will allow non-WB types. The reason
is for following scenerio: A  PCI-E device setting the non-snoop bit to
1 in TLP header when doing memory access transaction to RAM. and the
driver/OS will access that RAM with UC attribute. 

In current implementation without this patch, WB type will be used by
guest  , then PCI-E device may get wrong data, becaues the data updated
by CPU may still in cache, and the PCI-E device's access is not snooped.
This patch will virtualize the cache attribute through attribute in
shadow page table.
Not sure if this is similar to AGP GART.

Of course, some issues left for this method:
1) For qemu's cirrus VGA VRAM, even if guest set that memory range to be
non-WB type like UC, the attribute in shadow table will be WB
(precisely, it will be the type that domain0/qemu-dm will use) , since
domain0 and guest will access them at the same time. This is commented
in the patch, and we have a internal patch under review.
2) If Xen/domain0 try to map guest's RAM  that guest has set to be UC,
problem may happen. This issue do exists, but we think it is safe
currently, considering guest OS will usually only change DMA target
RAM's attribute, and Xen/domain0 should not try map those guest RAM. Of
course, we need to consider this still.

Yunhong Jiang

> 
> -- Keir
> 
> On 8/10/07 08:57, "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx> wrote:
> 
>> The second patch is a bit of a hack-and-slash job. New memory-type
>> virtualisation code should go in a new file, instead of being put in
host
>> MTRR management files (which are mostly unmodified from Linux anyway,
and
>> shouldn't deviate more than necessary). There are various other
one-line
>> changes in various MTRR source files that are unexplained (e.g., mask
>> changes from |0xf00 to |0x7ff). Initialisation of guest MTRR state
will
>> need to be moved to hvmloader -- we do not call setvcpucontext for
HVM
>> guests any more, so the current hack won't work with current
xen-unstable.
>> Coding style needs cleaning up (looks like there is some spaces vs.
tabs
>> mixing up going on). 
>> 
>>   -- Keir
>> 
>> On 8/10/07 08:34, "Su, Disheng" <disheng.su@xxxxxxxxx> wrote:
>> 
>>> Hi,
>>> The following 2 patches add basic MTRR/PAT support into
>>> hypervisor. When vt-d enabled, direct IO and RAM
>>> could be mapped to different cache attribute, such as UC or WC,
which
>>> will bring some trouble. xen.patch: some data structures of MTRR in
xen
>>> are exported. mtrr_pat.patch: The basic idea is listed below:
>>>           a. MTRR/PAT MSRs are per vcpu. The value of guest MTRR is
>>> initialized in host, after guest E820 table is build. The value of
guest
>>> PAT is initialized as default. The host PAT is initialized to cover
all
>>>           the page attributes. b. The guest page attribute is
virtualized
>>> through shadow page attribute. First, get the effective guest page
>>> attribute, by calculating from the combination of guest MTRR/PAT.
Then
>>> the shadow page attribute is calculated from effective guest page
>>> attribute and host MTRR. If conflict occurs(e.g effective guest page
>>> attribute is WB, host MTRR of this range is UC, can't set this page
>>> attribute as guest required), set this range as UC. Some lookup
tables
>>> are added to accelerate above procedure. 
>>> 
>>> Signed-off-by: Disheng Su <disheng.su@xxxxxxxxx>
>>> 
>>> Best Regards,
>>> Disheng, Su
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel