RE: [Xen-devel] [PATCH 1/9] Add cpu idle pwr mgmt to xen

["Wei, Gang" <gang.wei@xxxxxxxxx>]

Revised according to below comments. Resend.

Jimmy

On Wednesday, April 30, 2008 3:22 PM, Jan Beulich wrote:
> Thanks. Unfortunately you now use a static (but not per-CPU) variable
-
> while I understand that it is expected that the call is done just
once, I
> don't think this is a good thing to do.
> 
> Further, xen_processor_csd_t seems to not need translation, so you
> could simply add a check for the type to xen/include/xlat.lst and copy
> the handle rather than what it points to. This would reduce size
> constraints on the xlat area and also simplify the code.
> 
> As another suggestion - could you use uint32_t for the bitfield
> declarations, making it more obvious that the remaining bits in the
> 32-bit quantity are reserved? Alternatively, could you use an
> explicit padding field after the flags member of struct
> xen_processor_power?
> 
> Also, I think there's error checking missing on copy_from_guest*
> throughout the patch. And I think I saw non-C89 constructs (loop
> variables declared inside for() statements).
> 
> Jan

Attachment: xen-1-cx_base-0501.patch
Description: xen-1-cx_base-0501.patch

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