diff mbox series

[v2,15/58] i386/tdx: Add property sept-ve-disable for tdx-guest object

Message ID 20230818095041.1973309-16-xiaoyao.li@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX QEMU support | expand

Commit Message

Xiaoyao Li Aug. 18, 2023, 9:49 a.m. UTC
Bit 28 of TD attribute, named SEPT_VE_DISABLE. When set to 1, it disables
EPT violation conversion to #VE on guest TD access of PENDING pages.

Some guest OS (e.g., Linux TD guest) may require this bit as 1.
Otherwise refuse to boot.

Add sept-ve-disable property for tdx-guest object, for user to configure
this bit.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi/qom.json         |  4 +++-
 target/i386/kvm/tdx.c | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé Aug. 21, 2023, 8:59 a.m. UTC | #1
On Fri, Aug 18, 2023 at 05:49:58AM -0400, Xiaoyao Li wrote:
> Bit 28 of TD attribute, named SEPT_VE_DISABLE. When set to 1, it disables
> EPT violation conversion to #VE on guest TD access of PENDING pages.
> 
> Some guest OS (e.g., Linux TD guest) may require this bit as 1.
> Otherwise refuse to boot.
> 
> Add sept-ve-disable property for tdx-guest object, for user to configure
> this bit.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi/qom.json         |  4 +++-
>  target/i386/kvm/tdx.c | 24 ++++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 2ca7ce7c0da5..cc08b9a98df9 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -871,10 +871,12 @@
>  #
>  # Properties for tdx-guest objects.
>  #
> +# @sept-ve-disable: bit 28 of TD attributes (default: 0)

This description isn't very useful as it forces the user to go off and
read the TDX specification to find out what bit 28 means. You've got a
more useful description in the commit message, so please use that
in the docs too. eg something like this

  @sept-ve-disable: toggle bit 28 of TD attributes to control disabling
                    of EPT violation conversion to #VE on guest
                    TD access of PENDING pages. Some guest OS (e.g.
		    Linux TD guest) may require this set, otherwise
                    they refuse to boot.

> +#
>  # Since: 8.2
>  ##
>  { 'struct': 'TdxGuestProperties',
> -  'data': { }}
> +  'data': { '*sept-ve-disable': 'bool' } }
>  
>  ##
>  # @ThreadContextProperties:
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 3d313ed46bd1..22130382c0c5 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -32,6 +32,8 @@
>                                       (1U << KVM_FEATURE_PV_SCHED_YIELD) | \
>                                       (1U << KVM_FEATURE_MSI_EXT_DEST_ID))
>  
> +#define TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE   BIT_ULL(28)
> +
>  #define TDX_ATTRIBUTES_MAX_BITS      64
>  
>  static FeatureMask tdx_attrs_ctrl_fields[TDX_ATTRIBUTES_MAX_BITS] = {
> @@ -501,6 +503,24 @@ out:
>      return r;
>  }
>  
> +static bool tdx_guest_get_sept_ve_disable(Object *obj, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    return !!(tdx->attributes & TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE);
> +}
> +
> +static void tdx_guest_set_sept_ve_disable(Object *obj, bool value, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    if (value) {
> +        tdx->attributes |= TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE;
> +    } else {
> +        tdx->attributes &= ~TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE;
> +    }
> +}
> +
>  /* tdx guest */
>  OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
>                                     tdx_guest,
> @@ -516,6 +536,10 @@ static void tdx_guest_init(Object *obj)
>      qemu_mutex_init(&tdx->lock);
>  
>      tdx->attributes = 0;
> +
> +    object_property_add_bool(obj, "sept-ve-disable",
> +                             tdx_guest_get_sept_ve_disable,
> +                             tdx_guest_set_sept_ve_disable);
>  }
>  
>  static void tdx_guest_finalize(Object *obj)
> -- 
> 2.34.1
> 

With regards,
Daniel
Markus Armbruster Aug. 22, 2023, 6:27 a.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Aug 18, 2023 at 05:49:58AM -0400, Xiaoyao Li wrote:
>> Bit 28 of TD attribute, named SEPT_VE_DISABLE. When set to 1, it disables
>> EPT violation conversion to #VE on guest TD access of PENDING pages.
>> 
>> Some guest OS (e.g., Linux TD guest) may require this bit as 1.
>> Otherwise refuse to boot.
>> 
>> Add sept-ve-disable property for tdx-guest object, for user to configure
>> this bit.
>> 
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  qapi/qom.json         |  4 +++-
>>  target/i386/kvm/tdx.c | 24 ++++++++++++++++++++++++
>>  2 files changed, 27 insertions(+), 1 deletion(-)
>> 
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 2ca7ce7c0da5..cc08b9a98df9 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -871,10 +871,12 @@
>>  #
>>  # Properties for tdx-guest objects.
>>  #
>> +# @sept-ve-disable: bit 28 of TD attributes (default: 0)
>
> This description isn't very useful as it forces the user to go off and
> read the TDX specification to find out what bit 28 means. You've got a

Seconded.

> more useful description in the commit message, so please use that
> in the docs too. eg something like this
>
>   @sept-ve-disable: toggle bit 28 of TD attributes to control disabling
>                     of EPT violation conversion to #VE on guest
>                     TD access of PENDING pages. Some guest OS (e.g.
>                     Linux TD guest) may require this set, otherwise
>                     they refuse to boot.

But please format like

# @sept-ve-disable: toggle bit 28 of TD attributes to control disabling
#     of EPT violation conversion to #VE on guest TD access of PENDING
#     pages.  Some guest OS (e.g. Linux TD guest) may require this to
#     be set, otherwise they refuse to boot.

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

>> +#
>>  # Since: 8.2
>>  ##
>>  { 'struct': 'TdxGuestProperties',
>> -  'data': { }}
>> +  'data': { '*sept-ve-disable': 'bool' } }
>>  
>>  ##
>>  # @ThreadContextProperties:

[...]
Xiaoyao Li Aug. 22, 2023, 8:39 a.m. UTC | #3
On 8/22/2023 2:27 PM, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
>> On Fri, Aug 18, 2023 at 05:49:58AM -0400, Xiaoyao Li wrote:
>>> Bit 28 of TD attribute, named SEPT_VE_DISABLE. When set to 1, it disables
>>> EPT violation conversion to #VE on guest TD access of PENDING pages.
>>>
>>> Some guest OS (e.g., Linux TD guest) may require this bit as 1.
>>> Otherwise refuse to boot.
>>>
>>> Add sept-ve-disable property for tdx-guest object, for user to configure
>>> this bit.
>>>
>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>>> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>   qapi/qom.json         |  4 +++-
>>>   target/i386/kvm/tdx.c | 24 ++++++++++++++++++++++++
>>>   2 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 2ca7ce7c0da5..cc08b9a98df9 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -871,10 +871,12 @@
>>>   #
>>>   # Properties for tdx-guest objects.
>>>   #
>>> +# @sept-ve-disable: bit 28 of TD attributes (default: 0)
>>
>> This description isn't very useful as it forces the user to go off and
>> read the TDX specification to find out what bit 28 means. You've got a
> 
> Seconded.
> 
>> more useful description in the commit message, so please use that
>> in the docs too. eg something like this
>>
>>    @sept-ve-disable: toggle bit 28 of TD attributes to control disabling
>>                      of EPT violation conversion to #VE on guest
>>                      TD access of PENDING pages. Some guest OS (e.g.
>>                      Linux TD guest) may require this set, otherwise
>>                      they refuse to boot.
> 
> But please format like
> 
> # @sept-ve-disable: toggle bit 28 of TD attributes to control disabling
> #     of EPT violation conversion to #VE on guest TD access of PENDING
> #     pages.  Some guest OS (e.g. Linux TD guest) may require this to
> #     be set, otherwise they refuse to boot.
>

Thank you, Daniel and Markus.

Will use above in the next version.

> to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
> to conform to current conventions).
>
>>> +#
>>>   # Since: 8.2
>>>   ##
>>>   { 'struct': 'TdxGuestProperties',
>>> -  'data': { }}
>>> +  'data': { '*sept-ve-disable': 'bool' } }
>>>   
>>>   ##
>>>   # @ThreadContextProperties:
> 
> [...]
>
diff mbox series

Patch

diff --git a/qapi/qom.json b/qapi/qom.json
index 2ca7ce7c0da5..cc08b9a98df9 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -871,10 +871,12 @@ 
 #
 # Properties for tdx-guest objects.
 #
+# @sept-ve-disable: bit 28 of TD attributes (default: 0)
+#
 # Since: 8.2
 ##
 { 'struct': 'TdxGuestProperties',
-  'data': { }}
+  'data': { '*sept-ve-disable': 'bool' } }
 
 ##
 # @ThreadContextProperties:
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 3d313ed46bd1..22130382c0c5 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -32,6 +32,8 @@ 
                                      (1U << KVM_FEATURE_PV_SCHED_YIELD) | \
                                      (1U << KVM_FEATURE_MSI_EXT_DEST_ID))
 
+#define TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE   BIT_ULL(28)
+
 #define TDX_ATTRIBUTES_MAX_BITS      64
 
 static FeatureMask tdx_attrs_ctrl_fields[TDX_ATTRIBUTES_MAX_BITS] = {
@@ -501,6 +503,24 @@  out:
     return r;
 }
 
+static bool tdx_guest_get_sept_ve_disable(Object *obj, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    return !!(tdx->attributes & TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE);
+}
+
+static void tdx_guest_set_sept_ve_disable(Object *obj, bool value, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    if (value) {
+        tdx->attributes |= TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE;
+    } else {
+        tdx->attributes &= ~TDX_TD_ATTRIBUTES_SEPT_VE_DISABLE;
+    }
+}
+
 /* tdx guest */
 OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
                                    tdx_guest,
@@ -516,6 +536,10 @@  static void tdx_guest_init(Object *obj)
     qemu_mutex_init(&tdx->lock);
 
     tdx->attributes = 0;
+
+    object_property_add_bool(obj, "sept-ve-disable",
+                             tdx_guest_get_sept_ve_disable,
+                             tdx_guest_set_sept_ve_disable);
 }
 
 static void tdx_guest_finalize(Object *obj)