Re: [PATCH v4] ppc/spapr: Implement H_RANDOM hypercall in QEMU

[Greg Kurz <gkurz@linux.vnet.ibm.com>]

On Thu, 17 Sep 2015 10:49:41 +0200
Thomas Huth <thuth@redhat.com> wrote:

> The PAPR interface defines a hypercall to pass high-quality
> hardware generated random numbers to guests. Recent kernels can
> already provide this hypercall to the guest if the right hardware
> random number generator is available. But in case the user wants
> to use another source like EGD, or QEMU is running with an older
> kernel, we should also have this call in QEMU, so that guests that
> do not support virtio-rng yet can get good random numbers, too.
> 
> This patch now adds a new pseudo-device to QEMU that either
> directly provides this hypercall to the guest or is able to
> enable the in-kernel hypercall if available. The in-kernel
> hypercall can be enabled with the use-kvm property, e.g.:
> 
>  qemu-system-ppc64 -device spapr-rng,use-kvm=true
> 
> For handling the hypercall in QEMU instead, a "RngBackend" is
> required since the hypercall should provide "good" random data
> instead of pseudo-random (like from a "simple" library function
> like rand() or g_random_int()). Since there are multiple RngBackends
> available, the user must select an appropriate back-end via the
> "rng" property of the device, e.g.:
> 
>  qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=gid0 \
>                    -device spapr-rng,rng=gid0 ...
> 
> See http://wiki.qemu-project.org/Features-Done/VirtIORNG for
> other example of specifying RngBackends.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

It is a good thing that the user can choose between in-kernel and backend,
and this patch does the work.

This being said, I am not sure about the use case where a user has a hwrng
capable platform and wants to run guests without any hwrng support at all is
an appropriate default behavior... I guess we will find more users that want
in-kernel being the default if it is available.

The patch below modifies yours to do just this: the pseudo-device is only
created if hwrng is present and not already created.

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 240fab72e7af..4b92efed2ef3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -939,6 +939,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr)
     return rc;
 }
 
+static void spapr_rng_create(void)
+{
+    Object *rng = object_new(TYPE_SPAPR_RNG);
+
+    object_property_set_bool(rng, true, "use-kvm", &error_abort);
+    object_property_set_bool(rng, true, "realized", &error_abort);
+}
+
 static void ppc_spapr_reset(void)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -962,6 +970,14 @@ static void ppc_spapr_reset(void)
     spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE;
     spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE;
 
+    /* Create a rng device if the user did not provide it already and
+     * KVM has hwrng support.
+     */
+    if (kvmppc_hwrng_present() &&
+        !object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
+        spapr_rng_create();
+    }
+
     /* Load the fdt */
     spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
                        spapr->rtas_size);
diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
index ed43d5e04221..ee5af302bd4d 100644
--- a/hw/ppc/spapr_rng.c
+++ b/hw/ppc/spapr_rng.c
@@ -114,7 +114,7 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
     sPAPRRngState *rngstate = SPAPR_RNG(dev);
 
     if (rngstate->use_kvm) {
-        if (kvmppc_enable_hwrng() = 0) {
+        if (kvmppc_hwrng_present() && kvmppc_enable_hwrng() = 0) {
             return;
         }
         /*
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 42f66fea23e9..008f8a26ab17 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2485,11 +2485,12 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
     return data & 0xffff;
 }
 
-int kvmppc_enable_hwrng(void)
+bool kvmppc_hwrng_present(void)
 {
-    if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
-        return -1;
-    }
+    return kvm_enabled() && kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG);
+}
 
+int kvmppc_enable_hwrng(void)
+{
     return kvmppc_enable_hcall(kvm_state, H_RANDOM);
 }
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 68836b401105..4b78bfe5224a 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -55,6 +55,7 @@ void kvmppc_hash64_free_pteg(uint64_t token);
 void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
                              target_ulong pte0, target_ulong pte1);
 bool kvmppc_has_cap_fixup_hcalls(void);
+bool kvmppc_hwrng_present(void);
 int kvmppc_enable_hwrng(void);
 
 #else
@@ -249,6 +250,11 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void)
     abort();
 }
 
+static inline bool kvmppc_hwrng_present(void)
+{
+    return false;
+}
+
 static inline int kvmppc_enable_hwrng(void)
 {
     return -1;

>  hw/ppc/Makefile.objs   |   2 +-
>  hw/ppc/spapr.c         |   8 +++
>  hw/ppc/spapr_rng.c     | 186 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |   4 ++
>  target-ppc/kvm.c       |   9 +++
>  target-ppc/kvm_ppc.h   |   5 ++
>  6 files changed, 213 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ppc/spapr_rng.c
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index c8ab06e..c1ffc77 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> -obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o
>  endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bf0c64f..34e7d24 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -768,6 +768,14 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>          exit(1);
>      }
> 
> +    if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
> +        ret = spapr_rng_populate_dt(fdt);
> +        if (ret < 0) {
> +            fprintf(stderr, "could not set up rng device in the fdt\n");
> +            exit(1);
> +        }
> +    }
> +
>      QLIST_FOREACH(phb, &spapr->phbs, list) {
>          ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
>      }
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> new file mode 100644
> index 0000000..ed43d5e
> --- /dev/null
> +++ b/hw/ppc/spapr_rng.c
> @@ -0,0 +1,186 @@
> +/*
> + * QEMU sPAPR random number generator "device" for H_RANDOM hypercall
> + *
> + * Copyright 2015 Thomas Huth, Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License,
> + * or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/error-report.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/device_tree.h"
> +#include "sysemu/rng.h"
> +#include "hw/ppc/spapr.h"
> +#include "kvm_ppc.h"
> +
> +#define SPAPR_RNG(obj) \
> +    OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
> +
> +struct sPAPRRngState {
> +    /*< private >*/
> +    DeviceState ds;
> +    RngBackend *backend;
> +    bool use_kvm;
> +};
> +typedef struct sPAPRRngState sPAPRRngState;
> +
> +struct HRandomData {
> +    QemuSemaphore sem;
> +    union {
> +        uint64_t v64;
> +        uint8_t v8[8];
> +    } val;
> +    int received;
> +};
> +typedef struct HRandomData HRandomData;
> +
> +/* Callback function for the RngBackend */
> +static void random_recv(void *dest, const void *src, size_t size)
> +{
> +    HRandomData *hrdp = dest;
> +
> +    if (src && size > 0) {
> +        assert(size + hrdp->received <= sizeof(hrdp->val.v8));
> +        memcpy(&hrdp->val.v8[hrdp->received], src, size);
> +        hrdp->received += size;
> +    }
> +
> +    qemu_sem_post(&hrdp->sem);
> +}
> +
> +/* Handler for the H_RANDOM hypercall */
> +static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> +                             target_ulong opcode, target_ulong *args)
> +{
> +    sPAPRRngState *rngstate;
> +    HRandomData hrdata;
> +
> +    rngstate = SPAPR_RNG(object_resolve_path_type("", TYPE_SPAPR_RNG, NULL));
> +
> +    if (!rngstate || !rngstate->backend) {
> +        return H_HARDWARE;
> +    }
> +
> +    qemu_sem_init(&hrdata.sem, 0);
> +    hrdata.val.v64 = 0;
> +    hrdata.received = 0;
> +
> +    qemu_mutex_unlock_iothread();
> +    while (hrdata.received < 8) {
> +        rng_backend_request_entropy(rngstate->backend, 8 - hrdata.received,
> +                                    random_recv, &hrdata);
> +        qemu_sem_wait(&hrdata.sem);
> +    }
> +    qemu_mutex_lock_iothread();
> +
> +    qemu_sem_destroy(&hrdata.sem);
> +    args[0] = hrdata.val.v64;
> +
> +    return H_SUCCESS;
> +}
> +
> +static void spapr_rng_instance_init(Object *obj)
> +{
> +    sPAPRRngState *rngstate = SPAPR_RNG(obj);
> +
> +    if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL) != NULL) {
> +        error_report("spapr-rng can not be instantiated twice!");
> +        return;
> +    }
> +
> +    object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
> +                             (Object **)&rngstate->backend,
> +                             object_property_allow_set_link,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
> +    object_property_set_description(obj, "rng",
> +                                    "ID of the random number generator backend",
> +                                    NULL);
> +}
> +
> +static void spapr_rng_realize(DeviceState *dev, Error **errp)
> +{
> +
> +    sPAPRRngState *rngstate = SPAPR_RNG(dev);
> +
> +    if (rngstate->use_kvm) {
> +        if (kvmppc_enable_hwrng() = 0) {
> +            return;
> +        }
> +        /*
> +         * If user specified both, use-kvm and a backend, we fall back to
> +         * the backend now. If not, provide an appropriate error message.
> +         */
> +        if (!rngstate->backend) {
> +            error_setg(errp, "Could not initialize in-kernel H_RANDOM call!");
> +            return;
> +        }
> +    }
> +
> +    if (rngstate->backend) {
> +        spapr_register_hypercall(H_RANDOM, h_random);
> +    } else {
> +        error_setg(errp, "spapr-rng needs an RNG backend!");
> +    }
> +}
> +
> +int spapr_rng_populate_dt(void *fdt)
> +{
> +    int node;
> +    int ret;
> +
> +    node = qemu_fdt_add_subnode(fdt, "/ibm,platform-facilities");
> +    if (node <= 0) {
> +        return -1;
> +    }
> +    ret = fdt_setprop_string(fdt, node, "device_type",
> +                             "ibm,platform-facilities");
> +    ret |= fdt_setprop_cell(fdt, node, "#address-cells", 0x1);
> +    ret |= fdt_setprop_cell(fdt, node, "#size-cells", 0x0);
> +
> +    node = fdt_add_subnode(fdt, node, "ibm,random-v1");
> +    if (node <= 0) {
> +        return -1;
> +    }
> +    ret |= fdt_setprop_string(fdt, node, "compatible", "ibm,random");
> +
> +    return ret ? -1 : 0;
> +}
> +
> +static Property spapr_rng_properties[] = {
> +    DEFINE_PROP_BOOL("use-kvm", sPAPRRngState, use_kvm, false),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void spapr_rng_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = spapr_rng_realize;
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    dc->props = spapr_rng_properties;
> +}
> +
> +static const TypeInfo spapr_rng_info = {
> +    .name          = TYPE_SPAPR_RNG,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(sPAPRRngState),
> +    .instance_init = spapr_rng_instance_init,
> +    .class_init    = spapr_rng_class_init,
> +};
> +
> +static void spapr_rng_register_type(void)
> +{
> +    type_register_static(&spapr_rng_info);
> +}
> +type_init(spapr_rng_register_type)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 91a61ab..4e8aa2d 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -331,6 +331,7 @@ struct sPAPRMachineState {
>  #define H_SET_MPP               0x2D0
>  #define H_GET_MPP               0x2D4
>  #define H_XIRR_X                0x2FC
> +#define H_RANDOM                0x300
>  #define H_SET_MODE              0x31C
>  #define MAX_HCALL_OPCODE        H_SET_MODE
> 
> @@ -603,10 +604,13 @@ struct sPAPRConfigureConnectorState {
>  void spapr_ccs_reset_hook(void *opaque);
> 
>  #define TYPE_SPAPR_RTC "spapr-rtc"
> +#define TYPE_SPAPR_RNG "spapr-rng"
> 
>  void spapr_rtc_read(DeviceState *dev, struct tm *tm, uint32_t *ns);
>  int spapr_rtc_import_offset(DeviceState *dev, int64_t legacy_offset);
> 
> +int spapr_rng_populate_dt(void *fdt);
> +
>  #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
> 
>  #endif /* !defined (__HW_SPAPR_H__) */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 110436d..42f66fe 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2484,3 +2484,12 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>  {
>      return data & 0xffff;
>  }
> +
> +int kvmppc_enable_hwrng(void)
> +{
> +    if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) {
> +        return -1;
> +    }
> +
> +    return kvmppc_enable_hcall(kvm_state, H_RANDOM);
> +}
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 4d30e27..68836b4 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -55,6 +55,7 @@ void kvmppc_hash64_free_pteg(uint64_t token);
>  void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
>                               target_ulong pte0, target_ulong pte1);
>  bool kvmppc_has_cap_fixup_hcalls(void);
> +int kvmppc_enable_hwrng(void);
> 
>  #else
> 
> @@ -248,6 +249,10 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void)
>      abort();
>  }
> 
> +static inline int kvmppc_enable_hwrng(void)
> +{
> +    return -1;
> +}
>  #endif
> 
>  #ifndef CONFIG_KVM