Re: [Qemu-devel] [PATCH 09/10] spapr: use count+index for memory hotplug

[David Gibson]

On Mon, Oct 24, 2016 at 11:47:35PM -0500, Michael Roth wrote:
> Commit 0a417869:
> 
> spapr: Move memory hotplug to RTAS_LOG_V6_HP_ID_DRC_COUNT type
> 
> dropped per-DRC/per-LMB hotplugs event in favor of a bulk add via a
> single LMB count value. This was to avoid overrunning the guest EPOW
> event queue with hotplug events. This works fine, but relies on the
> guest exhaustively scanning for pluggable LMBs to satisfy the
> requested count by issuing rtas-get-sensor(DR_ENTITY_SENSE, ...) calls
> until all the LMBs associated with the DIMM are identified.
> 
> With newer support for dedicated hotplug event source, this queue
> exhaustion is no longer as much of an issue due to implementation
> details on the guest side, but we still try to avoid excessive hotplug
> events by now supporting both a count and a starting index to avoid
> unecessary work. This patch makes use of that approach when the
> capability is available.
> 
> Cc: bhar...@linux.vnet.ibm.com
> Signed-off-by: Michael Roth 

Reviewed-by: David Gibson 

> ---
>  hw/ppc/spapr.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index dc4224b..0b3aa2f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2202,14 +2202,16 @@ static void spapr_nmi(NMIState *n, int cpu_index, 
> Error **errp)
>  }
>  }
>  
> -static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size,
> -   uint32_t node, Error **errp)
> +static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t 
> size,
> +   uint32_t node, bool dedicated_hp_event_source,
> +   Error **errp)
>  {
>  sPAPRDRConnector *drc;
>  sPAPRDRConnectorClass *drck;
>  uint32_t nr_lmbs = size/SPAPR_MEMORY_BLOCK_SIZE;
>  int i, fdt_offset, fdt_size;
>  void *fdt;
> +uint64_t addr = addr_start;
>  
>  for (i = 0; i < nr_lmbs; i++) {
>  drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> @@ -2228,7 +2230,17 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
> addr, uint64_t size,
>   * guest only in case of hotplugged memory
>   */
>  if (dev->hotplugged) {
> -   spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs);
> +if (dedicated_hp_event_source) {
> +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> +addr_start / SPAPR_MEMORY_BLOCK_SIZE);
> +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +
> spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
> +   nr_lmbs,
> +   drck->get_index(drc));
> +} else {
> +spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB,
> +   nr_lmbs);
> +}
>  }
>  }
>  
> @@ -2261,7 +2273,9 @@ static void spapr_memory_plug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  goto out;
>  }
>  
> -spapr_add_lmbs(dev, addr, size, node, _abort);
> +spapr_add_lmbs(dev, addr, size, node,
> +   spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
> +   _abort);
>  
>  out:
>  error_propagate(errp, local_err);

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

[Qemu-devel] [PATCH 09/10] spapr: use count+index for memory hotplug

[Michael Roth]

Commit 0a417869:

spapr: Move memory hotplug to RTAS_LOG_V6_HP_ID_DRC_COUNT type

dropped per-DRC/per-LMB hotplugs event in favor of a bulk add via a
single LMB count value. This was to avoid overrunning the guest EPOW
event queue with hotplug events. This works fine, but relies on the
guest exhaustively scanning for pluggable LMBs to satisfy the
requested count by issuing rtas-get-sensor(DR_ENTITY_SENSE, ...) calls
until all the LMBs associated with the DIMM are identified.

With newer support for dedicated hotplug event source, this queue
exhaustion is no longer as much of an issue due to implementation
details on the guest side, but we still try to avoid excessive hotplug
events by now supporting both a count and a starting index to avoid
unecessary work. This patch makes use of that approach when the
capability is available.

Cc: bhar...@linux.vnet.ibm.com
Signed-off-by: Michael Roth 
---
 hw/ppc/spapr.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index dc4224b..0b3aa2f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2202,14 +2202,16 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error 
**errp)
 }
 }
 
-static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size,
-   uint32_t node, Error **errp)
+static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t 
size,
+   uint32_t node, bool dedicated_hp_event_source,
+   Error **errp)
 {
 sPAPRDRConnector *drc;
 sPAPRDRConnectorClass *drck;
 uint32_t nr_lmbs = size/SPAPR_MEMORY_BLOCK_SIZE;
 int i, fdt_offset, fdt_size;
 void *fdt;
+uint64_t addr = addr_start;
 
 for (i = 0; i < nr_lmbs; i++) {
 drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
@@ -2228,7 +2230,17 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
addr, uint64_t size,
  * guest only in case of hotplugged memory
  */
 if (dev->hotplugged) {
-   spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs);
+if (dedicated_hp_event_source) {
+drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
+addr_start / SPAPR_MEMORY_BLOCK_SIZE);
+drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+spapr_hotplug_req_add_by_count_indexed(SPAPR_DR_CONNECTOR_TYPE_LMB,
+   nr_lmbs,
+   drck->get_index(drc));
+} else {
+spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB,
+   nr_lmbs);
+}
 }
 }
 
@@ -2261,7 +2273,9 @@ static void spapr_memory_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 goto out;
 }
 
-spapr_add_lmbs(dev, addr, size, node, _abort);
+spapr_add_lmbs(dev, addr, size, node,
+   spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
+   _abort);
 
 out:
 error_propagate(errp, local_err);
-- 
1.9.1