All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
@ 2018-10-08 12:39 Thomas Huth
  2018-10-09 11:26 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thomas Huth @ 2018-10-08 12:39 UTC (permalink / raw)
  To: David Gibson, qemu-ppc; +Cc: qemu-devel

The spapr-rng device is suboptimal when compared to virtio-rng, so
users might want to disable it in their builds. Thus let's introduce
a proper CONFIG switch to allow us to compile QEMU without this device.
The function spapr_rng_populate_dt is required for linking, so move it
to a different location.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Put spapr_rng_populate_dt() into spapr.c instead of a header

 default-configs/ppc64-softmmu.mak |  1 +
 hw/ppc/Makefile.objs              |  3 ++-
 hw/ppc/spapr.c                    | 23 +++++++++++++++++++++++
 hw/ppc/spapr_rng.c                | 23 -----------------------
 include/hw/ppc/spapr.h            |  2 --
 5 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index b94af6c..24d4717 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -17,3 +17,4 @@ CONFIG_XICS=$(CONFIG_PSERIES)
 CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM))
 CONFIG_MEM_HOTPLUG=y
+CONFIG_SPAPR_RNG=y
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 4ab5564..4e0c1c0 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -3,8 +3,9 @@ obj-y += ppc.o ppc_booke.o fdt.o
 # IBM pSeries (sPAPR)
 obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.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 spapr_rng.o
+obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
 obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
+obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
 # IBM PowerNV
 obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 98868d8..e666872 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -610,6 +610,29 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
     g_free(rev);
 }
 
+static 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 uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
 {
     MemoryDeviceInfoList *info;
diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
index d2acd61..644bac9 100644
--- a/hw/ppc/spapr_rng.c
+++ b/hw/ppc/spapr_rng.c
@@ -132,29 +132,6 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
     }
 }
 
-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_LINK("rng", sPAPRRngState, backend, TYPE_RNG_BACKEND,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ad4d7cf..0805ad8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -745,8 +745,6 @@ int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
 
 #define TYPE_SPAPR_RNG "spapr-rng"
 
-int spapr_rng_populate_dt(void *fdt);
-
 #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
 
 /*
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
  2018-10-08 12:39 [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c Thomas Huth
@ 2018-10-09 11:26 ` Greg Kurz
  2018-10-09 11:45 ` [Qemu-devel] " Philippe Mathieu-Daudé
  2018-10-10  1:06 ` David Gibson
  2 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2018-10-09 11:26 UTC (permalink / raw)
  To: Thomas Huth; +Cc: David Gibson, qemu-ppc, qemu-devel

On Mon,  8 Oct 2018 14:39:42 +0200
Thomas Huth <thuth@redhat.com> wrote:

> The spapr-rng device is suboptimal when compared to virtio-rng, so
> users might want to disable it in their builds. Thus let's introduce
> a proper CONFIG switch to allow us to compile QEMU without this device.
> The function spapr_rng_populate_dt is required for linking, so move it
> to a different location.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  v2: Put spapr_rng_populate_dt() into spapr.c instead of a header
> 
>  default-configs/ppc64-softmmu.mak |  1 +
>  hw/ppc/Makefile.objs              |  3 ++-
>  hw/ppc/spapr.c                    | 23 +++++++++++++++++++++++
>  hw/ppc/spapr_rng.c                | 23 -----------------------
>  include/hw/ppc/spapr.h            |  2 --
>  5 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index b94af6c..24d4717 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -17,3 +17,4 @@ CONFIG_XICS=$(CONFIG_PSERIES)
>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>  CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM))
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_SPAPR_RNG=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 4ab5564..4e0c1c0 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -3,8 +3,9 @@ obj-y += ppc.o ppc_booke.o fdt.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.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 spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> +obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d8..e666872 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -610,6 +610,29 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
>      g_free(rev);
>  }
>  
> +static 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 uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>  {
>      MemoryDeviceInfoList *info;
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> index d2acd61..644bac9 100644
> --- a/hw/ppc/spapr_rng.c
> +++ b/hw/ppc/spapr_rng.c
> @@ -132,29 +132,6 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> -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_LINK("rng", sPAPRRngState, backend, TYPE_RNG_BACKEND,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ad4d7cf..0805ad8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -745,8 +745,6 @@ int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
>  
>  #define TYPE_SPAPR_RNG "spapr-rng"
>  
> -int spapr_rng_populate_dt(void *fdt);
> -
>  #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
>  
>  /*

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
  2018-10-08 12:39 [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c Thomas Huth
  2018-10-09 11:26 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2018-10-09 11:45 ` Philippe Mathieu-Daudé
  2018-10-09 13:27   ` Thomas Huth
  2018-10-10  1:06 ` David Gibson
  2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-09 11:45 UTC (permalink / raw)
  To: Thomas Huth, David Gibson, qemu-ppc; +Cc: qemu-devel

Hi Thomas,

On 08/10/2018 14:39, Thomas Huth wrote:
> The spapr-rng device is suboptimal when compared to virtio-rng, so
> users might want to disable it in their builds. Thus let's introduce
> a proper CONFIG switch to allow us to compile QEMU without this device.
> The function spapr_rng_populate_dt is required for linking, so move it
> to a different location.

Without CONFIG_SPAPR_RNG this function is not reachable, can you use a
stub instead?

Thanks,

Phil.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Put spapr_rng_populate_dt() into spapr.c instead of a header
> 
>  default-configs/ppc64-softmmu.mak |  1 +
>  hw/ppc/Makefile.objs              |  3 ++-
>  hw/ppc/spapr.c                    | 23 +++++++++++++++++++++++
>  hw/ppc/spapr_rng.c                | 23 -----------------------
>  include/hw/ppc/spapr.h            |  2 --
>  5 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index b94af6c..24d4717 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -17,3 +17,4 @@ CONFIG_XICS=$(CONFIG_PSERIES)
>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>  CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM))
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_SPAPR_RNG=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 4ab5564..4e0c1c0 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -3,8 +3,9 @@ obj-y += ppc.o ppc_booke.o fdt.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.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 spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> +obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d8..e666872 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -610,6 +610,29 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
>      g_free(rev);
>  }
>  
> +static 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 uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>  {
>      MemoryDeviceInfoList *info;
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> index d2acd61..644bac9 100644
> --- a/hw/ppc/spapr_rng.c
> +++ b/hw/ppc/spapr_rng.c
> @@ -132,29 +132,6 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> -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_LINK("rng", sPAPRRngState, backend, TYPE_RNG_BACKEND,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ad4d7cf..0805ad8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -745,8 +745,6 @@ int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
>  
>  #define TYPE_SPAPR_RNG "spapr-rng"
>  
> -int spapr_rng_populate_dt(void *fdt);
> -
>  #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
>  
>  /*
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
  2018-10-09 11:45 ` [Qemu-devel] " Philippe Mathieu-Daudé
@ 2018-10-09 13:27   ` Thomas Huth
  2018-10-09 13:58     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2018-10-09 13:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, David Gibson, qemu-ppc; +Cc: qemu-devel

On 2018-10-09 13:45, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 08/10/2018 14:39, Thomas Huth wrote:
>> The spapr-rng device is suboptimal when compared to virtio-rng, so
>> users might want to disable it in their builds. Thus let's introduce
>> a proper CONFIG switch to allow us to compile QEMU without this device.
>> The function spapr_rng_populate_dt is required for linking, so move it
>> to a different location.
> 
> Without CONFIG_SPAPR_RNG this function is not reachable, can you use a
> stub instead?

I'd like to avoid stubs if possible. stubs are fine if there is no easy
other solution, but they are a little bit confusing ("which of these
functions is now linked into the executable?"), so in case there is no
urgent need, it's IMHO way nicer to get along without them. In this
case, there is no real urgent need - the function is just small, and
does not have any other dependencies into the disabled code, so why
should we complicate things here and introduce a stub?

 Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
  2018-10-09 13:27   ` Thomas Huth
@ 2018-10-09 13:58     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-09 13:58 UTC (permalink / raw)
  To: Thomas Huth, David Gibson, qemu-ppc; +Cc: qemu-devel

On 09/10/2018 15:27, Thomas Huth wrote:
> On 2018-10-09 13:45, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 08/10/2018 14:39, Thomas Huth wrote:
>>> The spapr-rng device is suboptimal when compared to virtio-rng, so
>>> users might want to disable it in their builds. Thus let's introduce
>>> a proper CONFIG switch to allow us to compile QEMU without this device.
>>> The function spapr_rng_populate_dt is required for linking, so move it
>>> to a different location.
>>
>> Without CONFIG_SPAPR_RNG this function is not reachable, can you use a
>> stub instead?
> 
> I'd like to avoid stubs if possible. stubs are fine if there is no easy
> other solution, but they are a little bit confusing ("which of these
> functions is now linked into the executable?"), so in case there is no
> urgent need, it's IMHO way nicer to get along without them. In this
> case, there is no real urgent need - the function is just small, and
> does not have any other dependencies into the disabled code, so why
> should we complicate things here and introduce a stub?

In this case this is fair enough because this function uses the same API
than the surrounding functions (no extra headers/libs required).

[I just read v1 comments on this series]

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c
  2018-10-08 12:39 [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c Thomas Huth
  2018-10-09 11:26 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
  2018-10-09 11:45 ` [Qemu-devel] " Philippe Mathieu-Daudé
@ 2018-10-10  1:06 ` David Gibson
  2 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2018-10-10  1:06 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 4974 bytes --]

On Mon, Oct 08, 2018 at 02:39:42PM +0200, Thomas Huth wrote:
> The spapr-rng device is suboptimal when compared to virtio-rng, so
> users might want to disable it in their builds. Thus let's introduce
> a proper CONFIG switch to allow us to compile QEMU without this device.
> The function spapr_rng_populate_dt is required for linking, so move it
> to a different location.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Applied to ppc-for-3.1, thanks.

> ---
>  v2: Put spapr_rng_populate_dt() into spapr.c instead of a header
> 
>  default-configs/ppc64-softmmu.mak |  1 +
>  hw/ppc/Makefile.objs              |  3 ++-
>  hw/ppc/spapr.c                    | 23 +++++++++++++++++++++++
>  hw/ppc/spapr_rng.c                | 23 -----------------------
>  include/hw/ppc/spapr.h            |  2 --
>  5 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> index b94af6c..24d4717 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -17,3 +17,4 @@ CONFIG_XICS=$(CONFIG_PSERIES)
>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>  CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM))
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_SPAPR_RNG=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 4ab5564..4e0c1c0 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -3,8 +3,9 @@ obj-y += ppc.o ppc_booke.o fdt.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.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 spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> +obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d8..e666872 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -610,6 +610,29 @@ static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
>      g_free(rev);
>  }
>  
> +static 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 uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>  {
>      MemoryDeviceInfoList *info;
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> index d2acd61..644bac9 100644
> --- a/hw/ppc/spapr_rng.c
> +++ b/hw/ppc/spapr_rng.c
> @@ -132,29 +132,6 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp)
>      }
>  }
>  
> -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_LINK("rng", sPAPRRngState, backend, TYPE_RNG_BACKEND,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ad4d7cf..0805ad8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -745,8 +745,6 @@ int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);
>  
>  #define TYPE_SPAPR_RNG "spapr-rng"
>  
> -int spapr_rng_populate_dt(void *fdt);
> -
>  #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
>  
>  /*

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-10-10  1:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-08 12:39 [Qemu-devel] [PATCH v2] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c Thomas Huth
2018-10-09 11:26 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2018-10-09 11:45 ` [Qemu-devel] " Philippe Mathieu-Daudé
2018-10-09 13:27   ` Thomas Huth
2018-10-09 13:58     ` Philippe Mathieu-Daudé
2018-10-10  1:06 ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.