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

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> "Wei, Gang" <gang.wei@xxxxxxxxx> 30.04.08 05:27 >>>
>Revising done according to Jan's comments. Resend.

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


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