diff mbox

[for-4.8,2/2] x86/traps: Don't call hvm_hypervisor_cpuid_leaf() for PV guests

Message ID 1479121286-6390-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper Nov. 14, 2016, 11:01 a.m. UTC
Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
to execute in the context of a PV guest, but HVM-specific feature flags
shouldn't be visible to PV guests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/traps.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jan Beulich Nov. 14, 2016, 11:38 a.m. UTC | #1
>>> On 14.11.16 at 12:01, <andrew.cooper3@citrix.com> wrote:
> Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
> to execute in the context of a PV guest, but HVM-specific feature flags
> shouldn't be visible to PV guests.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit ...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -928,6 +928,11 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>          break;
>  
>      case 4:
> +        if ( !has_hvm_container_domain(currd) )
> +        {
> +            *eax = *ebx = *ecx = *edx = 0;
> +            break;
> +        }
>          hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
>          break;

... this being the last leaf, wouldn't we better limit the number of
leaves (reported in leaf 0) to 3 for PV?

Jan
Andrew Cooper Nov. 14, 2016, 1:18 p.m. UTC | #2
On 14/11/16 11:38, Jan Beulich wrote:
>>>> On 14.11.16 at 12:01, <andrew.cooper3@citrix.com> wrote:
>> Luckily, hvm_hypervisor_cpuid_leaf() and vmx_hypervisor_cpuid_leaf() are safe
>> to execute in the context of a PV guest, but HVM-specific feature flags
>> shouldn't be visible to PV guests.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -928,6 +928,11 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
>>          break;
>>  
>>      case 4:
>> +        if ( !has_hvm_container_domain(currd) )
>> +        {
>> +            *eax = *ebx = *ecx = *edx = 0;
>> +            break;
>> +        }
>>          hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
>>          break;
> ... this being the last leaf, wouldn't we better limit the number of
> leaves (reported in leaf 0) to 3 for PV?

I considered this, but decided not to.

The current max leaf handling is fragile, owing to some dubious control
from the toolstack, and the existence of XEN_CPUID_MAX_NUM_LEAVES in the
public API is absolutely broken.

I am going to need to rework this all anyway, and its not clear whether
we can/should report less than 4 leaves to PV guests.

~Andrew
diff mbox

Patch

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 14abb62..d56d76e 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -928,6 +928,11 @@  int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx,
         break;
 
     case 4:
+        if ( !has_hvm_container_domain(currd) )
+        {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
         hvm_hypervisor_cpuid_leaf(sub_idx, eax, ebx, ecx, edx);
         break;