Re: [Xen-devel] [PATCH RFC] xen: fix cpuid reporting on PVH Dom0

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/07/14 17:42, Roger Pau Monne wrote:
> dab11417d also caused some problems regarding HVM guest creation on
> PVH Dom0, mainly the CR4 mask returned by hvm_cr4_guest_reserved_bits
> changed from 0xfffffffffffff800 to 0xfffffffffffff893, which means HVM
> guests created from a PVH Dom0 are unable to set VME, PVI, PSE or PGE
> CR4 flags.
>
> This is because cpuid on PVH guests mask PSE, PGE, PSE36 and VME
> flags, so the white listing done in xc_cpuid_hvm_policy doesn't enable
> those features, and the guest ends up with a very restrictive cpuid
> policy.
>
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> TBH, I'm not sure what's the best way to fix this, the cpuid stuff is
> so convoluted and it's done in so many different places that I've
> probably missed something.

The whole cpuid setup is a mess which I plan to address once migration
v2 is sorted.

For now, my best suggestion is hack it until it works.

> ---
>  xen/arch/x86/traps.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 677074b..0a46f75 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -803,12 +803,17 @@ void pv_cpuid(struct cpu_user_regs *regs)
>      if ( (regs->eax & 0x7fffffff) == 0x00000001 )
>      {
>          /* Modify Feature Information. */
> -        __clear_bit(X86_FEATURE_VME, &d);
>          if ( !cpu_has_apic )
>              __clear_bit(X86_FEATURE_APIC, &d);
> -        __clear_bit(X86_FEATURE_PSE, &d);
> -        __clear_bit(X86_FEATURE_PGE, &d);
> -        __clear_bit(X86_FEATURE_PSE36, &d);
> +        if ( !is_pvh_vcpu(curr) || !is_control_domain(curr->domain) ||
> +             !is_hardware_domain(curr->domain) )

I am not really sure what the expected difference between
is_control_domain() and is_hardware_domain() actually is, but they are
synonymous for anyone not using hardware_dom=<something other than 0>

I suspect that is_control_domain() is irrelevant here however.

~Andrew

> +        {
> +            __clear_bit(X86_FEATURE_PSE, &d);
> +            __clear_bit(X86_FEATURE_PGE, &d);
> +            __clear_bit(X86_FEATURE_PSE36, &d);
> +            __clear_bit(X86_FEATURE_VME, &d);
> +        }
> +
>      }
>  
>      switch ( (uint32_t)regs->eax )


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