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] [RESEND][PATCH 0/5]MTRR/PAT virtualization

To: "Su, Disheng" <disheng.su@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [RESEND][PATCH 0/5]MTRR/PAT virtualization
From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Date: Mon, 22 Oct 2007 10:35:04 +0100
Delivery-date: Mon, 22 Oct 2007 02:35:57 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <FF386CB4AE0E4648B0A96060EC00F36C41F621@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcgQmgE9aEEete+pQSeR9AQJGxn4+wD9NHiN
Thread-topic: [Xen-devel] [RESEND][PATCH 0/5]MTRR/PAT virtualization
User-agent: Microsoft-Entourage/11.3.6.070618
Some comments:

 * Do not steal the private mtrr structures out of arch/x86/cpu/mtrr/*.
Instead define your own new ones in include/asm-x86/hvm/mtrr.h. Yes, there
will be some overlap with the existing definitions, but the structures are
small, and also there are fields that are applicable to one case and not the
other (e.g., the new fields you add are not really needed by arch/x86/mtrr
code; and some of the existing fields (e.g., has_fixed) are not relevant to
the new HVM virtual-mtrr code). It's cleaner just to totally separate the
two imo.

 * Do not change the existing arch/x86/cpu/mtrr/* code. Some of that
naturally disappears since you will no longer be modifying the mtrr state
structures for that code. Also the shadow_blow_tables() call is imo overly
conservative and is in any case a bug since you will likely be calling it
from irq context, which is invalid. IMO, best to assume that host MTRRs are
set up *before* relevant HVM passthru guests are created. At least in this
initial patchset.

 * Clean up coding style. All new files should follow Xen coding style. The
new hvm/mtrr.c file is a bit of a stylistic mess. Also I think it is being
too clever for its own good. E.g., the logic for overlapping variable MTRRs
in get_mtrr_type() is freaking complicated, when the spec allows us to say
that any overlap implies UC. How hard should that be to implement, really?

 * The new hypercall should be a domctl(), not a memory_op(). It's only to
be used by dom0 for control of other guests.

That's it for now!

 -- Keir

On 17/10/07 09:45, "Su, Disheng" <disheng.su@xxxxxxxxx> wrote:

> Hi,
> The following 5 patches add the MTRR/PAT support into
> hypervisor. 
> 
> 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