diff mbox series

[v5,28/65] i386/tdx: Disable pmu for TD guest

Message ID 20240229063726.610065-29-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series QEMU Guest memfd + QEMU TDX support | expand

Commit Message

Xiaoyao Li Feb. 29, 2024, 6:36 a.m. UTC
Current KVM doesn't support PMU for TD guest. It returns error if TD is
created with PMU bit being set in attributes.

Disable PMU for TD guest on QEMU side.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/tdx.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Chenyi Qiang April 16, 2024, 8:32 a.m. UTC | #1
On 2/29/2024 2:36 PM, Xiaoyao Li wrote:
> Current KVM doesn't support PMU for TD guest. It returns error if TD is
> created with PMU bit being set in attributes.
> 
> Disable PMU for TD guest on QEMU side.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  target/i386/kvm/tdx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 262e86fd2c67..1c12cda002b8 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -496,6 +496,8 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>      g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
>      int r = 0;
>  
> +    object_property_set_bool(OBJECT(cpu), "pmu", false, &error_abort);

Is it necessary to output some prompt if the user wants to enable pmu by
"-cpu host,pmu=on"? As in patch 27, it mentions PMU is configured by
x86cpu->enable_pmu, but PMU is actually not support in this series and
will be disabled silently.

> +
>      QEMU_LOCK_GUARD(&tdx_guest->lock);
>      if (tdx_guest->initialized) {
>          return r;
Xiaoyao Li April 16, 2024, 8:55 a.m. UTC | #2
On 4/16/2024 4:32 PM, Chenyi Qiang wrote:
> 
> 
> On 2/29/2024 2:36 PM, Xiaoyao Li wrote:
>> Current KVM doesn't support PMU for TD guest. It returns error if TD is
>> created with PMU bit being set in attributes.
>>
>> Disable PMU for TD guest on QEMU side.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   target/i386/kvm/tdx.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
>> index 262e86fd2c67..1c12cda002b8 100644
>> --- a/target/i386/kvm/tdx.c
>> +++ b/target/i386/kvm/tdx.c
>> @@ -496,6 +496,8 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>>       g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
>>       int r = 0;
>>   
>> +    object_property_set_bool(OBJECT(cpu), "pmu", false, &error_abort);
> 
> Is it necessary to output some prompt if the user wants to enable pmu by
> "-cpu host,pmu=on"? As in patch 27, it mentions PMU is configured by
> x86cpu->enable_pmu, but PMU is actually not support in this series and
> will be disabled silently.

We do this in QEMU is mainly for KVM, because KVM will fail to init TD 
if ATTRIBUTE.PERFMON is set.

It's expected that KVM reports PERFMON in attributes.fixed0 when KVM 
cannot provide support of it. Then QEMU will print error message 
automatically when validate the attributes.

For QEMU part, next version is going to set the default value of "pmu" 
to false in kvm_cpu_max_instance_init(), so that "-cpu host" will not 
enable pmu for TDX VMs by default.

I suppose both the KVM and QEMU change will show up in the next version.

>> +
>>       QEMU_LOCK_GUARD(&tdx_guest->lock);
>>       if (tdx_guest->initialized) {
>>           return r;
diff mbox series

Patch

diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 262e86fd2c67..1c12cda002b8 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -496,6 +496,8 @@  int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
     g_autofree struct kvm_tdx_init_vm *init_vm = NULL;
     int r = 0;
 
+    object_property_set_bool(OBJECT(cpu), "pmu", false, &error_abort);
+
     QEMU_LOCK_GUARD(&tdx_guest->lock);
     if (tdx_guest->initialized) {
         return r;