Re: [Qemu-devel] [PATCH v2 2/4] tests: adding 'set_indicator' RTAS call

[Michael Roth]

Quoting Daniel Henrique Barboza (2017-10-31 15:43:28)
> 'set_indicator' is a RTAS hypercall that is used to change
> the state of both physical and logical devices DRCs by setting
> allocation-state, isolation-state and dr-indicator.
> 
> This patch implements the set_indicator RTAS call in
> tests/libqos/rtas.c, adding its test in tests/rtas-test.c.
> 
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
> ---
>  tests/libqos/rtas.c | 33 ++++++++++++++++++++++++
>  tests/libqos/rtas.h |  2 ++
>  tests/rtas-test.c   | 73 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
> index fdeab448f7..ade572a84f 100644
> --- a/tests/libqos/rtas.c
> +++ b/tests/libqos/rtas.c
> @@ -151,3 +151,36 @@ int qrtas_check_exception(QGuestAllocator *alloc, 
> uint32_t mask,
> 
>      return ret[0];
>  }
> +
> +/*
> + * set_indicator as defined by PAPR 2.7+, 7.3.5.4
> + *
> + * nargs = 3
> + * nrets = 1
> + *
> + * arg[0] = the type of the indicator
> + * arg[1] = index of the specific indicator
> + * arg[2] = desired new state
> + *
> + * Depending on the input, set_indicator will call set_isolation_state,
> + * set_allocation_state or set_dr_indicator in hw/ppc/spapr_drc.c.
> + * These functions allows the guest to control the state of hotplugged
> + * and hot unplugged devices.
> + */
> +int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
> +                        uint32_t new_state)
> +{
> +    uint32_t args[3], ret[1];
> +    int res;
> +
> +    args[0] = type;
> +    args[1] = idx;
> +    args[2] = new_state;
> +
> +    res = qrtas_call(alloc, "set-indicator", 3, args, 1, ret);
> +    if (res != 0) {
> +        return -1;
> +    }
> +
> +    return ret[0];
> +}
> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
> index 330ecfd397..9dfa18f32b 100644
> --- a/tests/libqos/rtas.h
> +++ b/tests/libqos/rtas.h
> @@ -14,4 +14,6 @@ int qrtas_ibm_write_pci_config(QGuestAllocator *alloc, 
> uint64_t buid,
>                                 uint32_t addr, uint32_t size, uint32_t val);
>  int qrtas_check_exception(QGuestAllocator *alloc, uint32_t mask,
>                            uint32_t buf_addr, uint32_t buf_len);
> +int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
> +                        uint32_t new_state);
>  #endif /* LIBQOS_RTAS_H */
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> index c5a6080043..2c34b6e83c 100644
> --- a/tests/rtas-test.c
> +++ b/tests/rtas-test.c
> @@ -8,6 +8,13 @@
>  #define EVENT_MASK_EPOW (1 << 30)
>  #define EVENT_LOG_LEN 2048
> 
> +#define RTAS_SENSOR_TYPE_ISOLATION_STATE 9001
> +#define RTAS_SENSOR_TYPE_ALLOCATION_STATE 9003
> +#define SPAPR_DR_ISOLATION_STATE_ISOLATED 0
> +#define SPAPR_DR_ALLOCATION_STATE_UNUSABLE 0
> +#define SPAPR_DR_ALLOCATION_STATE_USABLE  1
> +#define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
> +
>  static void test_rtas_get_time_of_day(void)
>  {
>      QOSState *qs;
> @@ -97,6 +104,71 @@ static void test_rtas_check_exception_hotplug_event(void)
>      qtest_shutdown(qs);
>  }
> 
> +/*
> + * To test the 'set-indicator' RTAS call we will hotplug a device
> + * (in this case a CPU) and then make its DRC state go from
> + * the starting state UNUSABLE(1) to UNISOLATE(3). These DRC
> + * states transitions are described in further detail in
> + * PAPR 2.7+ 13.4.
> + */
> +static void test_rtas_set_indicator(void)
> +{
> +    QOSState *qs;
> +    uint64_t ret;
> +    uintptr_t guest_buf_addr;
> +    uint32_t drc_index;
> +    uint8_t *buf = g_malloc0(EVENT_LOG_LEN);
> +
> +    qs = qtest_spapr_boot("-machine pseries -cpu POWER8_v2.0 "
> +                          "-smp 1,sockets=4,cores=1,threads=1,maxcpus=4");
> +
> +    guest_buf_addr = guest_alloc(qs->alloc, EVENT_LOG_LEN * sizeof(uint8_t));
> +    qtest_qmp_device_add("power8_v2.0-spapr-cpu-core", "id-1",
> +                         "'core-id':'1'");
> +
> +    ret = qrtas_check_exception(qs->alloc, EVENT_MASK_EPOW,
> +                                guest_buf_addr, EVENT_LOG_LEN);
> +
> +    memread(guest_buf_addr, buf, EVENT_LOG_LEN);
> +    guest_free(qs->alloc, guest_buf_addr);
> +
> +    g_assert_cmpint(ret, ==, 0);
> +
> +    /*
> +     * This time we can't ignore the event log written in the
> +     * check_exception call - we need the DRC index of the
> +     * recently added CPU to make the state changes using set_indicator.
> +     *
> +     * A bit of magic to go straight to the DRC index by checking the
> +     * error log format in hw/ppc/spapr_events.c:
> +     *
> +     * - rtas_error_log size = 8 bytes
> +     * - all other structures until the hotplug event log = 88 bytes
> +     * - inside the hotplug event log, skip 8 + 4 bytes to get to
> +     *   the drc_id union.
> +     *
> +     * This gives us a 108 bytes skip to get the drc info, which is
> +     * written in be32.
> +     */
> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));

Definitely agree some helpers to parse/validate the event log structure would
be helpful here. I think you can pretty much cast to a struct epow_extended_log
if you break out some of the definitions in spapr_events.c.

> +    g_free(buf);
> +
> +    /*
> +     * According to the DRC state diagram, the guest first sets a device
> +     * to USABLE (2), then UNISOLATED (3). Both should return
> +     * RTAS_OUT_SUCCESS(0).
> +     */
> +    ret = qrtas_set_indicator(qs->alloc, RTAS_SENSOR_TYPE_ALLOCATION_STATE,
> +                              drc_index, SPAPR_DR_ALLOCATION_STATE_USABLE);
> +    g_assert_cmpint(ret, ==, 0);
> +
> +    ret = qrtas_set_indicator(qs->alloc, RTAS_SENSOR_TYPE_ISOLATION_STATE,
> +                              drc_index, 
> SPAPR_DR_ISOLATION_STATE_UNISOLATED);
> +    g_assert_cmpint(ret, ==, 0);

Would be good to also validate handling for some of the invalid transitions
too, like unisolating a device that's in an unusable state, eventually
at least.

> +
> +    qtest_shutdown(qs);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      const char *arch = qtest_get_arch();
> @@ -112,6 +184,7 @@ int main(int argc, char *argv[])
>                     test_rtas_check_exception_no_events);
>      qtest_add_func("rtas/rtas-check-exception-hotplug-event",
>                     test_rtas_check_exception_hotplug_event);
> +    qtest_add_func("rtas/test_rtas_set_indicator", test_rtas_set_indicator);
> 
>      return g_test_run();
>  }
> -- 
> 2.13.6
>