Re: [Qemu-devel] [PATCH v8 4/7] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver

[Stefano Stabellini]

On Sun, 17 May 2015, Quan Xu wrote:
> This drvier transfers any request/repond between TPM xenstubdoms
> driver and Xen vTPM stubdom, and facilitates communications between
> Xen vTPM stubdom domain and vTPM xenstubdoms driver. It is a glue for
> the TPM xenstubdoms driver and Xen stubdom vTPM domain that provides
> the actual TPM functionality.
> 
> (Xen) Xen backend driver should run before running this frontend, and
> initialize XenStore as the following for communication.
> 
> [XenStore]
> 
> for example:
> 
> Domain 0: runs QEMU for guest A
> Domain 1: vtpmmgr
> Domain 2: vTPM for guest A
> Domain 3: HVM guest A
> 
> [...]
>  local = ""
>    domain = ""
>     0 = ""
>      frontend = ""
>       vtpm = ""
>        2 = ""
>         0 = ""
>          backend = "/local/domain/2/backend/vtpm/0/0"
>          backend-id = "2"
>          state = "*"
>          handle = "0"
>          domain = "Domain3's name"
>          ring-ref = "*"
>          event-channel = "*"
>          feature-protocol-v2 = "1"
>      backend = ""
>       qdisk = ""
>        [...]
>       console = ""
>       vif = ""
>        [...]
>     2 = ""
>      [...]
>      backend = ""
>       vtpm = ""
>        0 = ""
>         0 = ""
>          frontend = "/local/domain/0/frontend/vtpm/2/0"
>          frontend-id = "0" ('0', frontend is running in Domain-0)
>          [...]
>     3 = ""
>      [...]
>      device = "" (frontend device, the backend is running in QEMU/.etc)
>       vkbd = ""
>        [...]
>       vif = ""
>        [...]
> 
>  ..
> 
> (QEMU) xen_vtpmdev_ops is initialized with the following process:
>   xen_hvm_init()
>     [...]
>     -->xen_fe_register("vtpm", ...)
>       -->xenstore_fe_scan()
>         -->xen_fe_try_init(ops)
>           --> XenDevOps.init()
>         -->xen_fe_get_xendev()
>           --> XenDevOps.alloc()
>         -->xen_fe_check()
>           -->xen_fe_try_initialise()
>             --> XenDevOps.initialise()
>           -->xen_fe_try_connected()
>             --> XenDevOps.connected()
>         -->xs_watch()
>     [...]
> 
> Signed-off-by: Quan Xu <address@hidden>
> Reviewed-by: Stefan Berger <address@hidden>

Much better than the last version I read. Only one comment:


> +static void vtpm_backend_changed(struct XenDevice *xendev, const char *node)
> +{
> +    int be_state;
> +
> +    if (strcmp(node, "state") == 0) {
> +        xenstore_read_be_int(xendev, node, &be_state);
> +        switch (be_state) {
> +        case XenbusStateConnected:
> +            /* TODO */
> +            break;
> +        case XenbusStateClosing:
> +        case XenbusStateClosed:
> +            xenbus_switch_state(xendev, XenbusStateClosing);
> +            break;
> +        default:
> +            break;
> +        }
> +    }
> +}

I thought you agreed that moving this to xen_frontend.c and make it
generic was a better idea?