[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v11 1/2] hw/ppc/spapr_events.c: removing 'exceptio
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH v11 1/2] hw/ppc/spapr_events.c: removing 'exception' from sPAPREventLogEntry |
Date: |
Fri, 19 May 2017 12:26:43 +1000 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Thu, May 18, 2017 at 05:24:01PM -0300, Daniel Henrique Barboza wrote:
> Currenty we do not have any RTAS event that is reported by the
> event-scan interface. The existing events, RTAS_LOG_TYPE_EPOW and
> RTAS_LOG_TYPE_HOTPLUG, are being reported by the check-exception
> interface and, as such, marked as 'exception=true'.
>
> Commit 79853e18d9, 'spapr_events: event-scan RTAS interface', added
> the event_scan interface because the guest kernel requires it to
> initialize other required interfaces. It is acting since then as
> a stub because no events that would be reported by it were added
> since then. However, the existence of the 'exception' boolean adds
> an unnecessary load in the future migration of the pending_events,
> sPAPREventLogEntry QTAILQ that hosts the pending RTAS events.
>
> To make the code cleaner and ease the future migration changes, this
> patch makes the following changes:
>
> - remove the 'exception' boolean that filter these events. There is
> nothing to filter since all events are reported by check-exception;
>
> - functions rtas_event_log_queue, rtas_event_log_dequeue and
> rtas_event_log_contains don't receive the 'exception' boolean
> as parameter;
>
> - event_scan function was simplified. It was calling
> 'rtas_event_log_dequeue(mask, false)' that was always returning
> 'NULL' because we have no events that are created with
> exception=false, thus in the end it would execute a jump to
> 'out_no_events' all the time. The function now assumes that
> this will always be the case and all the remaining logic were
> deleted.
>
> In the future, when or if we add new RTAS events that should
> be reported with the event_scan interface, we can refer to
> the changes made in this patch to add the event_scan logic
> back.
>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
Applied to ppc-for-2.10.
> ---
> hw/ppc/spapr_events.c | 52
> +++++++-------------------------------------------
> include/hw/ppc/spapr.h | 1 -
> 2 files changed, 7 insertions(+), 46 deletions(-)
>
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index f0b28d8..73e2a18 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -342,20 +342,18 @@ static int rtas_event_log_to_irq(sPAPRMachineState
> *spapr, int log_type)
> return source->irq;
> }
>
> -static void rtas_event_log_queue(int log_type, void *data, bool exception)
> +static void rtas_event_log_queue(int log_type, void *data)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> sPAPREventLogEntry *entry = g_new(sPAPREventLogEntry, 1);
>
> g_assert(data);
> entry->log_type = log_type;
> - entry->exception = exception;
> entry->data = data;
> QTAILQ_INSERT_TAIL(&spapr->pending_events, entry, next);
> }
>
> -static sPAPREventLogEntry *rtas_event_log_dequeue(uint32_t event_mask,
> - bool exception)
> +static sPAPREventLogEntry *rtas_event_log_dequeue(uint32_t event_mask)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> sPAPREventLogEntry *entry = NULL;
> @@ -364,10 +362,6 @@ static sPAPREventLogEntry
> *rtas_event_log_dequeue(uint32_t event_mask,
> const sPAPREventSource *source =
> rtas_event_log_to_source(spapr, entry->log_type);
>
> - if (entry->exception != exception) {
> - continue;
> - }
> -
> if (source->mask & event_mask) {
> break;
> }
> @@ -380,7 +374,7 @@ static sPAPREventLogEntry
> *rtas_event_log_dequeue(uint32_t event_mask,
> return entry;
> }
>
> -static bool rtas_event_log_contains(uint32_t event_mask, bool exception)
> +static bool rtas_event_log_contains(uint32_t event_mask)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> sPAPREventLogEntry *entry = NULL;
> @@ -389,10 +383,6 @@ static bool rtas_event_log_contains(uint32_t event_mask,
> bool exception)
> const sPAPREventSource *source =
> rtas_event_log_to_source(spapr, entry->log_type);
>
> - if (entry->exception != exception) {
> - continue;
> - }
> -
> if (source->mask & event_mask) {
> return true;
> }
> @@ -479,7 +469,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
> epow->event_modifier = RTAS_LOG_V6_EPOW_MODIFIER_NORMAL;
> epow->extended_modifier = RTAS_LOG_V6_EPOW_XMODIFIER_PARTITION_SPECIFIC;
>
> - rtas_event_log_queue(RTAS_LOG_TYPE_EPOW, new_epow, true);
> + rtas_event_log_queue(RTAS_LOG_TYPE_EPOW, new_epow);
>
> qemu_irq_pulse(xics_get_qirq(XICS_FABRIC(spapr),
> rtas_event_log_to_irq(spapr,
> @@ -572,7 +562,7 @@ static void spapr_hotplug_req_event(uint8_t hp_id,
> uint8_t hp_action,
> cpu_to_be32(drc_id->count_indexed.index);
> }
>
> - rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
> + rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp);
>
> qemu_irq_pulse(xics_get_qirq(XICS_FABRIC(spapr),
> rtas_event_log_to_irq(spapr,
> @@ -667,7 +657,7 @@ static void check_exception(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> xinfo |= (uint64_t)rtas_ld(args, 6) << 32;
> }
>
> - event = rtas_event_log_dequeue(mask, true);
> + event = rtas_event_log_dequeue(mask);
> if (!event) {
> goto out_no_events;
> }
> @@ -690,7 +680,7 @@ static void check_exception(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> * interrupts.
> */
> for (i = 0; i < EVENT_CLASS_MAX; i++) {
> - if (rtas_event_log_contains(EVENT_CLASS_MASK(i), true)) {
> + if (rtas_event_log_contains(EVENT_CLASS_MASK(i))) {
> const sPAPREventSource *source =
> spapr_event_sources_get_source(spapr->event_sources, i);
>
> @@ -710,38 +700,10 @@ static void event_scan(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> - uint32_t mask, buf, len, event_len;
> - sPAPREventLogEntry *event;
> - struct rtas_error_log *hdr;
> -
> if (nargs != 4 || nret != 1) {
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> return;
> }
> -
> - mask = rtas_ld(args, 0);
> - buf = rtas_ld(args, 2);
> - len = rtas_ld(args, 3);
> -
> - event = rtas_event_log_dequeue(mask, false);
> - if (!event) {
> - goto out_no_events;
> - }
> -
> - hdr = event->data;
> - event_len = be32_to_cpu(hdr->extended_length) + sizeof(*hdr);
> -
> - if (event_len < len) {
> - len = event_len;
> - }
> -
> - cpu_physical_memory_write(buf, event->data, len);
> - rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> - g_free(event->data);
> - g_free(event);
> - return;
> -
> -out_no_events:
> rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND);
> }
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5802f88..02239a5 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -598,7 +598,6 @@ sPAPRTCETable *spapr_tce_find_by_liobn(target_ulong
> liobn);
>
> struct sPAPREventLogEntry {
> int log_type;
> - bool exception;
> void *data;
> QTAILQ_ENTRY(sPAPREventLogEntry) next;
> };
--
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