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: address@hidden
> Signed-off-by: Michael Roth <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  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, &error_abort);
> +    spapr_add_lmbs(dev, addr, size, node,
> +                   spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
> +                   &error_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