From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [PATCH v8 4/7] Qemu-Xen-vTPM: Register Xen stubdom
 vTPM frontend driver
Date: Mon, 22 Jun 2015 17:55:09 +0100
Message-ID: <alpine.DEB.2.02.1506221744280.4824__14865.1897012771$1434992283$gmane$org@kaball.uk.xensource.com>
References: <1431840030-2797-1-git-send-email-quan.xu@intel.com>
	<1431840030-2797-6-git-send-email-quan.xu@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1431840030-2797-6-git-send-email-quan.xu@intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Quan Xu <quan.xu@intel.com>
Cc: wei.liu2@citrix.com, stefanb@linux.vnet.ibm.com, stefano.stabellini@eu.citrix.com, qemu-devel@nongnu.org, xen-devel@lists.xen.org, dgdegra@tycho.nsa.gov, eblake@redhat.com
List-Id: xen-devel@lists.xenproject.org

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 <quan.xu@intel.com>
> Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

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?