[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v15 05/19] pvh prep: Introduce pv guest type and has_hvm_container macros
On 11/12/2013 01:34 PM, Jan Beulich wrote: On 11.11.13 at 15:57, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:--- a/xen/arch/x86/acpi/suspend.c +++ b/xen/arch/x86/acpi/suspend.c @@ -85,7 +85,7 @@ void restore_rest_processor_state(void) BUG(); /* Maybe load the debug registers. */ - BUG_ON(is_hvm_vcpu(curr)); + BUG_ON(!is_pv_vcpu(curr)); if ( !is_idle_vcpu(curr) && curr->arch.debugreg[7] ) { write_debugreg(0, curr->arch.debugreg[0]);This is certainly fine for now, but I guess in the long run (i.e. when supporting Dom0) this will need changing from BUG_ON() to integrating the condition with the if(). Yes -- the BUG_ON() was to make sure someone thinks carefully about what the right thing is to do here. I suppose there should have been a fixme here as well, however. @@ -1246,8 +1246,7 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c) bool_t compat = is_pv_32on64_domain(v->domain); #define c(fld) (!compat ? (c.nat->fld) : (c.cmp->fld)) - if ( is_hvm_vcpu(v) ) - memset(c.nat, 0, sizeof(*c.nat)); + memset(c.nat, 0, sizeof(*c.nat));Removing the conditional is both dangerous and unmotivated: Dangerous because you now assume that the space allocated is always covering the (larger) native size (this currently happens to be that way in the only caller, but isn't guaranteed). And unmotivated because switching just the conditional (as done throughout the rest of the patch) would have sufficed here. Indeed, I can't think of why I didn't change it to "!is_pv_vcpu()" here. -George _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |