Re: [Xen-devel] [PATCH v14 11/17] pvh: Set up more PV stuff in set_info_guest

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.11.13 at 13:15, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> @@ -728,8 +740,21 @@ int arch_set_info_guest(
>  
>      if ( has_hvm_container_vcpu(v) )
>      {
> -        hvm_set_info_guest(v);
> -        goto out;
> +        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);

I'm afraid this isn't correct - so far gs_base_kernel didn't get used
for HVM guests, i.e. you're changing behavior here (even if only
in a - presumably - benign way).

> +
> +        if ( is_hvm_vcpu(v) || v->is_initialised )
> +            goto out;
> +
> +        cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[3]);

I'd recommend against using this PV construct - the 32-bit
counterpart won't be correct to be used here once 32-bit
support gets added.

> @@ -1426,6 +1426,11 @@ static void vmx_set_info_guest(struct vcpu *v)
>          __vmwrite(GUEST_INTERRUPTIBILITY_INFO, intr_shadow);
>      }
>  
> +    /* PVH 32bitfixme */
> +    if ( is_pvh_vcpu(v) )
> +        __vmwrite(GUEST_GS_BASE, gs_base_kernel);

Oh, I see, you suppress this here. I'd really suggest adjusting the
caller, then you don't need to do anything here afaict.

> +
> +

Or if you need to, please add just a single blank line here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel