Re: [Qemu-devel] [PATCH for-4.2 2/2] spapr: initial implementation for H_TPM_COMM/spapr-tpm-proxy

[David Gibson]

On Tue, Jul 16, 2019 at 06:53:13PM -0500, Michael Roth wrote:
> This implements the H_TPM_COMM hypercall, which is used by an
> Ultravisor to pass TPM commands directly to the host's TPM device, or
> a TPM Resource Manager associated with the device.
> 
> This also introduces a new virtual device, spapr-tpm-proxy, which
> is used to configure the host TPM path to be used to service
> requests sent by H_TPM_COMM hcalls, for example:
> 
>   -device spapr-tpm-proxy,id=tpmp0,host-path=/dev/tpmrm0
> 
> By default, no spapr-tpm-proxy will be created, and hcalls will return
> H_FUNCTION.
> 
> The full specification for this hypercall can be found in
> docs/specs/ppc-spapr-uv-hcalls.txt

Mostly LGTM, but..

[...]
>  #define H_SUCCESS         0
> @@ -490,8 +492,9 @@ struct SpaprMachineState {
>  #define H_INT_ESB               0x3C8
>  #define H_INT_SYNC              0x3CC
>  #define H_INT_RESET             0x3D0
> +#define H_TPM_COMM              0xEF10

This is vastly increasing the size of the hcall dispatch table, which
isn't great.  Is the 0xE... range reserved for PEF related hypercalls?
I'm wondering if we want to make a third table here (we already have a
separate one for the qemu-specific hypercalls).

>  
> -#define MAX_HCALL_OPCODE        H_INT_RESET
> +#define MAX_HCALL_OPCODE        H_TPM_COMM
>  
>  /* The hcalls above are standardized in PAPR and implemented by pHyp
>   * as well.
> diff --git a/include/hw/ppc/spapr_tpm_proxy.h 
> b/include/hw/ppc/spapr_tpm_proxy.h
> new file mode 100644
> index 0000000000..4843cdaf58
> --- /dev/null
> +++ b/include/hw/ppc/spapr_tpm_proxy.h
> @@ -0,0 +1,31 @@
> +/*
> + * SPAPR TPM Proxy/Hypercall
> + *
> + * Copyright IBM Corp. 2019
> + *
> + * Authors:
> + *  Michael Roth      <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef HW_SPAPR_TPM_PROXY_H
> +#define HW_SPAPR_TPM_PROXY_H
> +
> +#include "qom/object.h"
> +#include "hw/qdev.h"
> +
> +#define TYPE_SPAPR_TPM_PROXY "spapr-tpm-proxy"
> +#define SPAPR_TPM_PROXY(obj) OBJECT_CHECK(SpaprTpmProxy, (obj), \
> +                                          TYPE_SPAPR_TPM_PROXY)
> +
> +typedef struct SpaprTpmProxy {
> +    /*< private >*/
> +    DeviceState parent;
> +
> +    char *host_path;
> +    int host_fd;
> +} SpaprTpmProxy;
> +
> +#endif /* HW_SPAPR_TPM_PROXY_H */

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature