Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/01/17 15:41, Roger Pau Monné wrote:
> On Tue, Jan 24, 2017 at 08:10:56AM -0700, Jan Beulich wrote:
>>>>> On 24.01.17 at 15:38, <roger.pau@xxxxxxxxxx> wrote:
>>> On Wed, Jan 18, 2017 at 07:40:53PM +0000, Andrew Cooper wrote:
>>>> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
>>>> mask, but nothing thusfar has prevented the features being visible in
>>>> HVM-based control domains (where there is no toolstack decision to hide the
>>>> features).
>>>>
>>>> As a side effect of calling nestedhvm_enabled() earlier during domain
>>>> creation, it needs to cope with the params[] array array not having been
>>>> allocated.
>>>>
>>>> Reported-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> ---
>>>>  xen/arch/x86/cpuid.c         | 25 ++++++++++++++++++-------
>>>>  xen/arch/x86/hvm/nestedhvm.c |  3 ++-
>>>>  2 files changed, 20 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
>>>> index eb829d7..7b9af1b 100644
>>>> --- a/xen/arch/x86/cpuid.c
>>>> +++ b/xen/arch/x86/cpuid.c
>>>> @@ -3,6 +3,7 @@
>>>>  #include <xen/sched.h>
>>>>  #include <asm/cpuid.h>
>>>>  #include <asm/hvm/hvm.h>
>>>> +#include <asm/hvm/nestedhvm.h>
>>>>  #include <asm/hvm/vmx/vmcs.h>
>>>>  #include <asm/processor.h>
>>>>  #include <asm/xstate.h>
>>>> @@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
>>>>      cpuid_policy_to_featureset(p, fs);
>>>>      cpuid_policy_to_featureset(max, max_fs);
>>>>  
>>>> -    /*
>>>> -     * HVM domains using Shadow paging have further restrictions on their
>>>> -     * available paging features.
>>>> -     */
>>>> -    if ( is_hvm_domain(d) && !hap_enabled(d) )
>>>> +    if ( is_hvm_domain(d) )
>>> This should be has_hvm_container_domain or else classic PVH is broken, but I
>>> don't know how much we care about classic PVH any longer.
>> The old check excluded PVHv1 (due to it depending on HAP), as
>> does the new check (in a more explicit way), so I don't see what's
>> wrong here.
> Right, I guess this is caused by e94ce5, which did:
>
>      case EXIT_REASON_CPUID:
>      {
> -        int rc;
> -
> -        if ( is_pvh_vcpu(v) )
> -        {
> -            pv_cpuid(regs);
> -            rc = 0;
> -        }
> -        else
> -            rc = vmx_do_cpuid(regs);
> +        int rc = vmx_do_cpuid(regs);
>
> Which removed the special casing for the PVH CPUID, and I assume pv_cpuid used
> to remove the VT-x extensions from the output of CPUID?

PVH guests still enter pv_cpuid() via the legacy path in guest_cpuid().

However, PVH cpuid handling was quite broken to start with.  I am not
deliberately trying to make it worse, so your original suggestion should
probably be made (if anyone actually cares).

~Andrew

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