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

[Michael Roth]

Quoting David Gibson (2019-07-16 21:01:15)
> 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).

Yes, that's probably a good idea. SVM hcalls use a reserved range
0xEF00-0xEF80. I'll send a v2 that uses a separate table for these.

> 
> >  
> > -#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