Re: [PATCH 1/5] confidential guest support: Introduce a 'check' class handler

["Cédric Le Goater" <clg@kaod.org>]

On 1/5/23 09:46, Thomas Huth wrote:
> On 04/01/2023 12.51, Cédric Le Goater wrote:
>> From: Cédric Le Goater <clg@redhat.com>
>>
>> Some machines have specific requirements to activate confidential
>> guest support. Add a class handler to the confidential guest support
>> interface to let the arch implementation perform extra checks.
>>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   include/exec/confidential-guest-support.h |  4 +++-
>>   hw/core/machine.c                         | 11 ++++++-----
>>   2 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/exec/confidential-guest-support.h b/include/exec/confidential-guest-support.h
>> index ba2dd4b5df..9e6d362b26 100644
>> --- a/include/exec/confidential-guest-support.h
>> +++ b/include/exec/confidential-guest-support.h
>> @@ -23,7 +23,8 @@
>>   #include "qom/object.h"
>>   #define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
>> -OBJECT_DECLARE_SIMPLE_TYPE(ConfidentialGuestSupport, CONFIDENTIAL_GUEST_SUPPORT)
>> +OBJECT_DECLARE_TYPE(ConfidentialGuestSupport, ConfidentialGuestSupportClass,
>> +                    CONFIDENTIAL_GUEST_SUPPORT)
>>   struct ConfidentialGuestSupport {
>>       Object parent;
>> @@ -55,6 +56,7 @@ struct ConfidentialGuestSupport {
>>   typedef struct ConfidentialGuestSupportClass {
>>       ObjectClass parent;
>> +    bool (*check)(const Object *obj, Error **errp);
>>   } ConfidentialGuestSupportClass;
>>   #endif /* !CONFIG_USER_ONLY */
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index f589b92909..bab43cd675 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -502,11 +502,12 @@ static void machine_check_confidential_guest_support(const Object *obj,
>>                                                        Object *new_target,
>>                                                        Error **errp)
>>   {
>> -    /*
>> -     * So far the only constraint is that the target has the
>> -     * TYPE_CONFIDENTIAL_GUEST_SUPPORT interface, and that's checked
>> -     * by the QOM core
>> -     */
>> +    ConfidentialGuestSupportClass *cgsc =
>> +        CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(new_target);
>> +
>> +    if (cgsc->check) {
>> +        cgsc->check(obj, errp);
> 
> I assume the caller is checking *errp, so it's ok to ignore the return value of the check function here?

yes. routine machine_check_confidential_guest_support() is a direct parameter
of object_class_property_add_link() call, which checks *errp. See routine
object_set_link_property().

Thanks,

C.

> 
>> +    }
>>   }
>>   static bool machine_get_nvdimm(Object *obj, Error **errp)
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>