Re: [Qemu-devel] [RFC PATCH v2 1/2] spapr: Add DRC count indexed hotplug identifier type

2016-03-19 Thread Michael Roth 
Quoting Bharata B Rao (2016-03-14 23:38:55)
> Add support for DRC count indexed hotplug ID type which is primarily
> needed for memory hot unplug. This type allows for specifying the
> number of DRs that should be plugged/unplugged starting from a given
> DRC index.
> 
> NOTE: This new hotplug identifier type is not yet part of PAPR.
> 
> Signed-off-by: Bharata B Rao 
> ---
>  hw/ppc/spapr_events.c  | 57 
> +-
>  include/hw/ppc/spapr.h |  2 ++
>  2 files changed, 45 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 39f4682..5d1d13d 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -171,6 +171,16 @@ struct epow_log_full {
>  struct rtas_event_log_v6_epow epow;
>  } QEMU_PACKED;
> 
> +union drc_id {
> +uint32_t index;
> +uint32_t count;
> +struct count_index {
> +uint32_t index;
> +uint32_t count;

The current version of the spec proposal is actually count followed by
index. I kind of wish it was in the opposite order, and it's probably
not too late to change this if there's pressing reason, but that's how
things stand atm.

> +} count_index;

> +char name[1];
> +} QEMU_PACKED;
> +
>  struct rtas_event_log_v6_hp {
>  #define RTAS_LOG_V6_SECTION_ID_HOTPLUG  0x4850 /* HP */
>  struct rtas_event_log_v6_section_header hdr;
> @@ -187,12 +197,9 @@ struct rtas_event_log_v6_hp {
>  #define RTAS_LOG_V6_HP_ID_DRC_NAME   1
>  #define RTAS_LOG_V6_HP_ID_DRC_INDEX  2
>  #define RTAS_LOG_V6_HP_ID_DRC_COUNT  3
> +#define RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED  4
>  uint8_t reserved;
> -union {
> -uint32_t index;
> -uint32_t count;
> -char name[1];
> -} drc;
> +union drc_id drc_id;
>  } QEMU_PACKED;
> 
>  struct hp_log_full {
> @@ -389,7 +396,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
> 
>  static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
>  sPAPRDRConnectorType drc_type,
> -uint32_t drc)
> +union drc_id *drc_id)
>  {
>  sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  struct hp_log_full *new_hp;
> @@ -446,9 +453,12 @@ static void spapr_hotplug_req_event(uint8_t hp_id, 
> uint8_t hp_action,
>  }
> 
>  if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT) {
> -hp->drc.count = cpu_to_be32(drc);
> +hp->drc_id.count = cpu_to_be32(drc_id->count);
>  } else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_INDEX) {
> -hp->drc.index = cpu_to_be32(drc);
> +hp->drc_id.index = cpu_to_be32(drc_id->index);
> +} else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED) {
> +hp->drc_id.count_index.count = 
> cpu_to_be32(drc_id->count_index.count);
> +hp->drc_id.count_index.index = 
> cpu_to_be32(drc_id->count_index.index);
>  }
> 
>  rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
> @@ -460,34 +470,53 @@ void spapr_hotplug_req_add_by_index(sPAPRDRConnector 
> *drc)
>  {
>  sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>  sPAPRDRConnectorType drc_type = drck->get_type(drc);
> -uint32_t index = drck->get_index(drc);
> +union drc_id drc_id;

I'd rather we used 'union drc_id id' or something. Having the typename
and variable names be identical is a little confusing.

> +drc_id.index = drck->get_index(drc);
> 
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
> -RTAS_LOG_V6_HP_ACTION_ADD, drc_type, index);
> +RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
>  }
> 
>  void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc)
>  {
>  sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>  sPAPRDRConnectorType drc_type = drck->get_type(drc);
> -uint32_t index = drck->get_index(drc);
> +union drc_id drc_id;
> +drc_id.index = drck->get_index(drc);
> 
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
> -RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, index);
> +RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, _id);
>  }
> 
>  void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType drc_type,
> uint32_t count)
>  {
> +union drc_id drc_id;
> +drc_id.count = count;
> +
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
> -RTAS_LOG_V6_HP_ACTION_ADD, drc_type, count);
> +RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
>  }
> 
>  void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
>uint32_t count)
>  {
> +union drc_id drc_id;
> +drc_id.count = count;
> +
>

Re: [Qemu-devel] [RFC PATCH v2 1/2] spapr: Add DRC count indexed hotplug identifier type

2016-03-15 Thread David Gibson 
On Tue, Mar 15, 2016 at 10:08:55AM +0530, Bharata B Rao wrote:
> Add support for DRC count indexed hotplug ID type which is primarily
> needed for memory hot unplug. This type allows for specifying the
> number of DRs that should be plugged/unplugged starting from a given
> DRC index.
> 
> NOTE: This new hotplug identifier type is not yet part of PAPR.
> 
> Signed-off-by: Bharata B Rao 

Reviewed-by: David Gibson 

Looks correct, but obviously I won't apply until the change reaches
PAPR.


> ---
>  hw/ppc/spapr_events.c  | 57 
> +-
>  include/hw/ppc/spapr.h |  2 ++
>  2 files changed, 45 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 39f4682..5d1d13d 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -171,6 +171,16 @@ struct epow_log_full {
>  struct rtas_event_log_v6_epow epow;
>  } QEMU_PACKED;
>  
> +union drc_id {
> +uint32_t index;
> +uint32_t count;
> +struct count_index {
> +uint32_t index;
> +uint32_t count;
> +} count_index;
> +char name[1];
> +} QEMU_PACKED;
> +
>  struct rtas_event_log_v6_hp {
>  #define RTAS_LOG_V6_SECTION_ID_HOTPLUG  0x4850 /* HP */
>  struct rtas_event_log_v6_section_header hdr;
> @@ -187,12 +197,9 @@ struct rtas_event_log_v6_hp {
>  #define RTAS_LOG_V6_HP_ID_DRC_NAME   1
>  #define RTAS_LOG_V6_HP_ID_DRC_INDEX  2
>  #define RTAS_LOG_V6_HP_ID_DRC_COUNT  3
> +#define RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED  4
>  uint8_t reserved;
> -union {
> -uint32_t index;
> -uint32_t count;
> -char name[1];
> -} drc;
> +union drc_id drc_id;
>  } QEMU_PACKED;
>  
>  struct hp_log_full {
> @@ -389,7 +396,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
>  
>  static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
>  sPAPRDRConnectorType drc_type,
> -uint32_t drc)
> +union drc_id *drc_id)
>  {
>  sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  struct hp_log_full *new_hp;
> @@ -446,9 +453,12 @@ static void spapr_hotplug_req_event(uint8_t hp_id, 
> uint8_t hp_action,
>  }
>  
>  if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT) {
> -hp->drc.count = cpu_to_be32(drc);
> +hp->drc_id.count = cpu_to_be32(drc_id->count);
>  } else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_INDEX) {
> -hp->drc.index = cpu_to_be32(drc);
> +hp->drc_id.index = cpu_to_be32(drc_id->index);
> +} else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED) {
> +hp->drc_id.count_index.count = 
> cpu_to_be32(drc_id->count_index.count);
> +hp->drc_id.count_index.index = 
> cpu_to_be32(drc_id->count_index.index);
>  }
>  
>  rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
> @@ -460,34 +470,53 @@ void spapr_hotplug_req_add_by_index(sPAPRDRConnector 
> *drc)
>  {
>  sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>  sPAPRDRConnectorType drc_type = drck->get_type(drc);
> -uint32_t index = drck->get_index(drc);
> +union drc_id drc_id;
> +drc_id.index = drck->get_index(drc);
>  
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
> -RTAS_LOG_V6_HP_ACTION_ADD, drc_type, index);
> +RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
>  }
>  
>  void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc)
>  {
>  sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>  sPAPRDRConnectorType drc_type = drck->get_type(drc);
> -uint32_t index = drck->get_index(drc);
> +union drc_id drc_id;
> +drc_id.index = drck->get_index(drc);
>  
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
> -RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, index);
> +RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, _id);
>  }
>  
>  void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType drc_type,
> uint32_t count)
>  {
> +union drc_id drc_id;
> +drc_id.count = count;
> +
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
> -RTAS_LOG_V6_HP_ACTION_ADD, drc_type, count);
> +RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
>  }
>  
>  void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
>uint32_t count)
>  {
> +union drc_id drc_id;
> +drc_id.count = count;
> +
>  spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
> -RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, count);
> +RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type,

[Qemu-devel] [RFC PATCH v2 1/2] spapr: Add DRC count indexed hotplug identifier type

2016-03-14 Thread Bharata B Rao 
Add support for DRC count indexed hotplug ID type which is primarily
needed for memory hot unplug. This type allows for specifying the
number of DRs that should be plugged/unplugged starting from a given
DRC index.

NOTE: This new hotplug identifier type is not yet part of PAPR.

Signed-off-by: Bharata B Rao 
---
 hw/ppc/spapr_events.c  | 57 +-
 include/hw/ppc/spapr.h |  2 ++
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 39f4682..5d1d13d 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -171,6 +171,16 @@ struct epow_log_full {
 struct rtas_event_log_v6_epow epow;
 } QEMU_PACKED;
 
+union drc_id {
+uint32_t index;
+uint32_t count;
+struct count_index {
+uint32_t index;
+uint32_t count;
+} count_index;
+char name[1];
+} QEMU_PACKED;
+
 struct rtas_event_log_v6_hp {
 #define RTAS_LOG_V6_SECTION_ID_HOTPLUG  0x4850 /* HP */
 struct rtas_event_log_v6_section_header hdr;
@@ -187,12 +197,9 @@ struct rtas_event_log_v6_hp {
 #define RTAS_LOG_V6_HP_ID_DRC_NAME   1
 #define RTAS_LOG_V6_HP_ID_DRC_INDEX  2
 #define RTAS_LOG_V6_HP_ID_DRC_COUNT  3
+#define RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED  4
 uint8_t reserved;
-union {
-uint32_t index;
-uint32_t count;
-char name[1];
-} drc;
+union drc_id drc_id;
 } QEMU_PACKED;
 
 struct hp_log_full {
@@ -389,7 +396,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
 
 static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
 sPAPRDRConnectorType drc_type,
-uint32_t drc)
+union drc_id *drc_id)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 struct hp_log_full *new_hp;
@@ -446,9 +453,12 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t 
hp_action,
 }
 
 if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT) {
-hp->drc.count = cpu_to_be32(drc);
+hp->drc_id.count = cpu_to_be32(drc_id->count);
 } else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_INDEX) {
-hp->drc.index = cpu_to_be32(drc);
+hp->drc_id.index = cpu_to_be32(drc_id->index);
+} else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED) {
+hp->drc_id.count_index.count = cpu_to_be32(drc_id->count_index.count);
+hp->drc_id.count_index.index = cpu_to_be32(drc_id->count_index.index);
 }
 
 rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
@@ -460,34 +470,53 @@ void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc)
 {
 sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 sPAPRDRConnectorType drc_type = drck->get_type(drc);
-uint32_t index = drck->get_index(drc);
+union drc_id drc_id;
+drc_id.index = drck->get_index(drc);
 
 spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
-RTAS_LOG_V6_HP_ACTION_ADD, drc_type, index);
+RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
 }
 
 void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc)
 {
 sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 sPAPRDRConnectorType drc_type = drck->get_type(drc);
-uint32_t index = drck->get_index(drc);
+union drc_id drc_id;
+drc_id.index = drck->get_index(drc);
 
 spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
-RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, index);
+RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, _id);
 }
 
 void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType drc_type,
uint32_t count)
 {
+union drc_id drc_id;
+drc_id.count = count;
+
 spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
-RTAS_LOG_V6_HP_ACTION_ADD, drc_type, count);
+RTAS_LOG_V6_HP_ACTION_ADD, drc_type, _id);
 }
 
 void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
   uint32_t count)
 {
+union drc_id drc_id;
+drc_id.count = count;
+
 spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
-RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, count);
+RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, _id);
+}
+
+void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
+   uint32_t count, uint32_t index)
+{
+union drc_id drc_id;
+drc_id.count_index.count = count;
+drc_id.count_index.index = index;
+
+spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED,
+RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, _id);
 }
 
 static void