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

[Thomas Huth <thuth@redhat.com>]

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?

> +    }
>   }
>   
>   static bool machine_get_nvdimm(Object *obj, Error **errp)

Reviewed-by: Thomas Huth <thuth@redhat.com>