diff mbox

[v2,3/4] tests: ibm, configure-connector RTAS call implementation

Message ID 20171031204330.14803-4-danielhb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Henrique Barboza Oct. 31, 2017, 8:43 p.m. UTC
'ibm,configure-connector' hypercall is used by the guest OS
to start the configuration of a given device, where the
machine configures the said device and all its sub-devices,
giving back FDTs to the caller for each sub-device.

This hypercall is supposed to be called multiple times by the
guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
that the device is now properly configured and ready
to be used, or a return value < 0 when an error occurs.

This patch implements the 'ibm,configure-connector' RTAS
hypercall in tests/libqos/rtas.c, with an extra test
case for it inside tests/rtas-tests.c.

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
 tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
 tests/libqos/rtas.h |  1 +
 tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)

Comments

Laurent Vivier Nov. 6, 2017, 5:46 p.m. UTC | #1
On 31/10/2017 21:43, Daniel Henrique Barboza wrote:
> 'ibm,configure-connector' hypercall is used by the guest OS
> to start the configuration of a given device, where the
> machine configures the said device and all its sub-devices,
> giving back FDTs to the caller for each sub-device.
> 
> This hypercall is supposed to be called multiple times by the
> guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
> that the device is now properly configured and ready
> to be used, or a return value < 0 when an error occurs.
> 
> This patch implements the 'ibm,configure-connector' RTAS
> hypercall in tests/libqos/rtas.c, with an extra test
> case for it inside tests/rtas-tests.c.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> ---
>  tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
>  tests/libqos/rtas.h |  1 +
>  tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+)
> 
> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
> index ade572a84f..1cb9e2b495 100644
> --- a/tests/libqos/rtas.c
> +++ b/tests/libqos/rtas.c
> @@ -184,3 +184,38 @@ int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
>  
>      return ret[0];
>  }
> +
> +/*
> + * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
> + *
> + * nargs = 2
> + * nrets = 1
> + *
> + * args[0] and args[1] compose the 64 bit Work Area address.
> + *
> + * This call will configure not only the device reported in the first
> + * offset of the Work Area but all of its siblingis as well, returning
> + * the FDT of each configured sub-device as well as a return code
> + * 'Next child', 'Next property' or 'Previous parent'. When the whole
> + * configuration is done, 'Configuration completed' (0) is returned.
> + *
> + * configure-connector will always reply with status code 'Next child'(2)
> + * on the first successful call. The DRC configuration will be
> + * completed when configure-connector returns status 0. Any return
> + * status < 0 indicates an error.
> + */
> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
> +{
> +    uint32_t args[2], ret[1];
> +    int res;
> +
> +    args[0] = (uint32_t)(wa_addr);
> +    args[1] = (uint32_t)(wa_addr >> 32);

This part looks strange to me. I agree it's what qemu does, but
according to spec args[0] is "Address of work area" and args[1] "0 or
address of additional page":

Linux on Power Architecture Platform Reference
Version 1.1
24 March 2016

13.5.3.5 ibm,configure-connector RTAS Call
The “need more memory” status code, is similar in semantics to the “call
again” status. However, on the next ibm,configure-connector call, the OS
will supply, via the Memory extent parameter, the address of another
page of memory for RTAS to add to the work area in order for
configuration to continue. On all other calls to ibm,configure-connector
the contents of the Memory extent parameter should be 0.

> +    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
> +    if (res != 0) {
> +        return -1;
> +    }
> +
> +    return ret[0];
> +}
> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
> index 9dfa18f32b..35c44fb967 100644
> --- a/tests/libqos/rtas.h
> +++ b/tests/libqos/rtas.h
> @@ -16,4 +16,5 @@ 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);
> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
>  #endif /* LIBQOS_RTAS_H */
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> index 2c34b6e83c..b3538bf878 100644
> --- a/tests/rtas-test.c
> +++ b/tests/rtas-test.c
> @@ -15,6 +15,8 @@
>  #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
>  #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
>  
> +#define CC_WA_LEN 4096
> +
>  static void test_rtas_get_time_of_day(void)
>  {
>      QOSState *qs;
> @@ -169,6 +171,70 @@ static void test_rtas_set_indicator(void)
>      qtest_shutdown(qs);
>  }
>  
> +static void test_rtas_ibm_configure_connector(void)
> +{
> +    QOSState *qs;
> +    uint64_t ret;
> +    uintptr_t guest_buf_addr, guest_drc_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);
> +
> +    /*
> +     * Same 108 bytes offset magic used and explained in
> +     * test_rtas_set_indicator.
> +     */
> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));
> +    g_free(buf);
> +
> +    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);
> +
> +    /*
> +     * Call ibm,configure-connector to finish the hotplugged device
> +     * configuration, putting its DRC into 'ready' state.
> +     *
> +     * We're not interested in the generated FDTs during the config
> +     * process, thus we simply keep calling configure-connector
> +     * until it returns SUCCESS(0) or an error.
> +     *
> +     * The full explanation logic behind this process can be found
> +     * at PAPR 2.7+, 13.5.3.5.
> +     */
> +    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));

CC_WA_LEN is already 4096, why "* sizeof(uint32_t"?
It must be aligned to a 4096 bytes boundaries.

> +    writel(guest_drc_addr, drc_index);
> +    writel(guest_drc_addr + sizeof(uint32_t), 0)> +
> +    do {
> +        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
> +    } while (ret > 0);

I think it would be interesting to check the result of the call (return
value, node name, property name,...)

And you should also exit with error in the case of ret == 5 (need more
memory).

> +    guest_free(qs->alloc, guest_drc_addr);
> +
> +    g_assert_cmpint(ret, ==, 0);
> +
> +    qtest_shutdown(qs);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      const char *arch = qtest_get_arch();
> @@ -185,6 +251,8 @@ int main(int argc, char *argv[])
>      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);
> +    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
> +                   test_rtas_ibm_configure_connector);
>  
>      return g_test_run();
>  }
> 

Thanks,
Laurent
Daniel Henrique Barboza Nov. 9, 2017, 12:35 p.m. UTC | #2
On 11/06/2017 03:46 PM, Laurent Vivier wrote:
> On 31/10/2017 21:43, Daniel Henrique Barboza wrote:
>> 'ibm,configure-connector' hypercall is used by the guest OS
>> to start the configuration of a given device, where the
>> machine configures the said device and all its sub-devices,
>> giving back FDTs to the caller for each sub-device.
>>
>> This hypercall is supposed to be called multiple times by the
>> guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
>> that the device is now properly configured and ready
>> to be used, or a return value < 0 when an error occurs.
>>
>> This patch implements the 'ibm,configure-connector' RTAS
>> hypercall in tests/libqos/rtas.c, with an extra test
>> case for it inside tests/rtas-tests.c.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>> ---
>>   tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
>>   tests/libqos/rtas.h |  1 +
>>   tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 104 insertions(+)
>>
>> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
>> index ade572a84f..1cb9e2b495 100644
>> --- a/tests/libqos/rtas.c
>> +++ b/tests/libqos/rtas.c
>> @@ -184,3 +184,38 @@ int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
>>   
>>       return ret[0];
>>   }
>> +
>> +/*
>> + * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
>> + *
>> + * nargs = 2
>> + * nrets = 1
>> + *
>> + * args[0] and args[1] compose the 64 bit Work Area address.
>> + *
>> + * This call will configure not only the device reported in the first
>> + * offset of the Work Area but all of its siblingis as well, returning
>> + * the FDT of each configured sub-device as well as a return code
>> + * 'Next child', 'Next property' or 'Previous parent'. When the whole
>> + * configuration is done, 'Configuration completed' (0) is returned.
>> + *
>> + * configure-connector will always reply with status code 'Next child'(2)
>> + * on the first successful call. The DRC configuration will be
>> + * completed when configure-connector returns status 0. Any return
>> + * status < 0 indicates an error.
>> + */
>> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
>> +{
>> +    uint32_t args[2], ret[1];
>> +    int res;
>> +
>> +    args[0] = (uint32_t)(wa_addr);
>> +    args[1] = (uint32_t)(wa_addr >> 32);
> This part looks strange to me. I agree it's what qemu does, but
> according to spec args[0] is "Address of work area" and args[1] "0 or
> address of additional page":
>
> Linux on Power Architecture Platform Reference
> Version 1.1
> 24 March 2016
>
> 13.5.3.5 ibm,configure-connector RTAS Call
> The “need more memory” status code, is similar in semantics to the “call
> again” status. However, on the next ibm,configure-connector call, the OS
> will supply, via the Memory extent parameter, the address of another
> page of memory for RTAS to add to the work area in order for
> configuration to continue. On all other calls to ibm,configure-connector
> the contents of the Memory extent parameter should be 0.

Hmmmmm thinking about it, the reason why it works in QEMU is because
wa_addr >> 32 will always be zero, perhaps?

At any rate, In this test, makes sense to leave args[1] to be set by the 
caller,
setting it to 0 in the non "need more memory" scenario.


>
>> +    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
>> +    if (res != 0) {
>> +        return -1;
>> +    }
>> +
>> +    return ret[0];
>> +}
>> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
>> index 9dfa18f32b..35c44fb967 100644
>> --- a/tests/libqos/rtas.h
>> +++ b/tests/libqos/rtas.h
>> @@ -16,4 +16,5 @@ 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);
>> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
>>   #endif /* LIBQOS_RTAS_H */
>> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
>> index 2c34b6e83c..b3538bf878 100644
>> --- a/tests/rtas-test.c
>> +++ b/tests/rtas-test.c
>> @@ -15,6 +15,8 @@
>>   #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
>>   #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
>>   
>> +#define CC_WA_LEN 4096
>> +
>>   static void test_rtas_get_time_of_day(void)
>>   {
>>       QOSState *qs;
>> @@ -169,6 +171,70 @@ static void test_rtas_set_indicator(void)
>>       qtest_shutdown(qs);
>>   }
>>   
>> +static void test_rtas_ibm_configure_connector(void)
>> +{
>> +    QOSState *qs;
>> +    uint64_t ret;
>> +    uintptr_t guest_buf_addr, guest_drc_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);
>> +
>> +    /*
>> +     * Same 108 bytes offset magic used and explained in
>> +     * test_rtas_set_indicator.
>> +     */
>> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));
>> +    g_free(buf);
>> +
>> +    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);
>> +
>> +    /*
>> +     * Call ibm,configure-connector to finish the hotplugged device
>> +     * configuration, putting its DRC into 'ready' state.
>> +     *
>> +     * We're not interested in the generated FDTs during the config
>> +     * process, thus we simply keep calling configure-connector
>> +     * until it returns SUCCESS(0) or an error.
>> +     *
>> +     * The full explanation logic behind this process can be found
>> +     * at PAPR 2.7+, 13.5.3.5.
>> +     */
>> +    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));
> CC_WA_LEN is already 4096, why "* sizeof(uint32_t"?
> It must be aligned to a 4096 bytes boundaries.

Good catch. I'll fix it.

>
>> +    writel(guest_drc_addr, drc_index);
>> +    writel(guest_drc_addr + sizeof(uint32_t), 0)> +
>> +    do {
>> +        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
>> +    } while (ret > 0);
> I think it would be interesting to check the result of the call (return
> value, node name, property name,...)
>
> And you should also exit with error in the case of ret == 5 (need more
> memory).
>
>> +    guest_free(qs->alloc, guest_drc_addr);
>> +
>> +    g_assert_cmpint(ret, ==, 0);
>> +
>> +    qtest_shutdown(qs);
>> +}
>> +
>>   int main(int argc, char *argv[])
>>   {
>>       const char *arch = qtest_get_arch();
>> @@ -185,6 +251,8 @@ int main(int argc, char *argv[])
>>       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);
>> +    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
>> +                   test_rtas_ibm_configure_connector);
>>   
>>       return g_test_run();
>>   }
>>
> Thanks,
> Laurent
>
Michael Roth Nov. 29, 2017, 12:02 a.m. UTC | #3
Quoting Daniel Henrique Barboza (2017-11-09 06:35:30)
> 
> 
> On 11/06/2017 03:46 PM, Laurent Vivier wrote:
> > On 31/10/2017 21:43, Daniel Henrique Barboza wrote:
> >> 'ibm,configure-connector' hypercall is used by the guest OS
> >> to start the configuration of a given device, where the
> >> machine configures the said device and all its sub-devices,
> >> giving back FDTs to the caller for each sub-device.
> >>
> >> This hypercall is supposed to be called multiple times by the
> >> guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
> >> that the device is now properly configured and ready
> >> to be used, or a return value < 0 when an error occurs.
> >>
> >> This patch implements the 'ibm,configure-connector' RTAS
> >> hypercall in tests/libqos/rtas.c, with an extra test
> >> case for it inside tests/rtas-tests.c.
> >>
> >> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> >> ---
> >>   tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
> >>   tests/libqos/rtas.h |  1 +
> >>   tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 104 insertions(+)
> >>
> >> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
> >> index ade572a84f..1cb9e2b495 100644
> >> --- a/tests/libqos/rtas.c
> >> +++ b/tests/libqos/rtas.c
> >> @@ -184,3 +184,38 @@ int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
> >>   
> >>       return ret[0];
> >>   }
> >> +
> >> +/*
> >> + * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
> >> + *
> >> + * nargs = 2
> >> + * nrets = 1
> >> + *
> >> + * args[0] and args[1] compose the 64 bit Work Area address.
> >> + *
> >> + * This call will configure not only the device reported in the first
> >> + * offset of the Work Area but all of its siblingis as well, returning
> >> + * the FDT of each configured sub-device as well as a return code
> >> + * 'Next child', 'Next property' or 'Previous parent'. When the whole
> >> + * configuration is done, 'Configuration completed' (0) is returned.
> >> + *
> >> + * configure-connector will always reply with status code 'Next child'(2)
> >> + * on the first successful call. The DRC configuration will be
> >> + * completed when configure-connector returns status 0. Any return
> >> + * status < 0 indicates an error.
> >> + */
> >> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
> >> +{
> >> +    uint32_t args[2], ret[1];
> >> +    int res;
> >> +
> >> +    args[0] = (uint32_t)(wa_addr);
> >> +    args[1] = (uint32_t)(wa_addr >> 32);
> > This part looks strange to me. I agree it's what qemu does, but
> > according to spec args[0] is "Address of work area" and args[1] "0 or
> > address of additional page":
> >
> > Linux on Power Architecture Platform Reference
> > Version 1.1
> > 24 March 2016
> >
> > 13.5.3.5 ibm,configure-connector RTAS Call
> > The “need more memory” status code, is similar in semantics to the “call
> > again” status. However, on the next ibm,configure-connector call, the OS
> > will supply, via the Memory extent parameter, the address of another
> > page of memory for RTAS to add to the work area in order for
> > configuration to continue. On all other calls to ibm,configure-connector
> > the contents of the Memory extent parameter should be 0.
> 
> Hmmmmm thinking about it, the reason why it works in QEMU is because
> wa_addr >> 32 will always be zero, perhaps?

It looks that way. At least with linux guests the "need more memory"
handling remains unimplemented, so we only ever end up dealing with:

  rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);

which effectively sets args[1] to 0. Still worth fixing though of
course.

> 
> At any rate, In this test, makes sense to leave args[1] to be set by the 
> caller,
> setting it to 0 in the non "need more memory" scenario.
> 
> 
> >
> >> +    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
> >> +    if (res != 0) {
> >> +        return -1;
> >> +    }
> >> +
> >> +    return ret[0];
> >> +}
> >> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
> >> index 9dfa18f32b..35c44fb967 100644
> >> --- a/tests/libqos/rtas.h
> >> +++ b/tests/libqos/rtas.h
> >> @@ -16,4 +16,5 @@ 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);
> >> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
> >>   #endif /* LIBQOS_RTAS_H */
> >> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> >> index 2c34b6e83c..b3538bf878 100644
> >> --- a/tests/rtas-test.c
> >> +++ b/tests/rtas-test.c
> >> @@ -15,6 +15,8 @@
> >>   #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
> >>   #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
> >>   
> >> +#define CC_WA_LEN 4096
> >> +
> >>   static void test_rtas_get_time_of_day(void)
> >>   {
> >>       QOSState *qs;
> >> @@ -169,6 +171,70 @@ static void test_rtas_set_indicator(void)
> >>       qtest_shutdown(qs);
> >>   }
> >>   
> >> +static void test_rtas_ibm_configure_connector(void)
> >> +{
> >> +    QOSState *qs;
> >> +    uint64_t ret;
> >> +    uintptr_t guest_buf_addr, guest_drc_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);
> >> +
> >> +    /*
> >> +     * Same 108 bytes offset magic used and explained in
> >> +     * test_rtas_set_indicator.
> >> +     */
> >> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));
> >> +    g_free(buf);
> >> +
> >> +    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);
> >> +
> >> +    /*
> >> +     * Call ibm,configure-connector to finish the hotplugged device
> >> +     * configuration, putting its DRC into 'ready' state.
> >> +     *
> >> +     * We're not interested in the generated FDTs during the config
> >> +     * process, thus we simply keep calling configure-connector
> >> +     * until it returns SUCCESS(0) or an error.
> >> +     *
> >> +     * The full explanation logic behind this process can be found
> >> +     * at PAPR 2.7+, 13.5.3.5.
> >> +     */
> >> +    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));
> > CC_WA_LEN is already 4096, why "* sizeof(uint32_t"?
> > It must be aligned to a 4096 bytes boundaries.
> 
> Good catch. I'll fix it.
> 
> >
> >> +    writel(guest_drc_addr, drc_index);
> >> +    writel(guest_drc_addr + sizeof(uint32_t), 0)> +
> >> +    do {
> >> +        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
> >> +    } while (ret > 0);
> > I think it would be interesting to check the result of the call (return
> > value, node name, property name,...)
> >
> > And you should also exit with error in the case of ret == 5 (need more
> > memory).
> >
> >> +    guest_free(qs->alloc, guest_drc_addr);
> >> +
> >> +    g_assert_cmpint(ret, ==, 0);
> >> +
> >> +    qtest_shutdown(qs);
> >> +}
> >> +
> >>   int main(int argc, char *argv[])
> >>   {
> >>       const char *arch = qtest_get_arch();
> >> @@ -185,6 +251,8 @@ int main(int argc, char *argv[])
> >>       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);
> >> +    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
> >> +                   test_rtas_ibm_configure_connector);
> >>   
> >>       return g_test_run();
> >>   }
> >>
> > Thanks,
> > Laurent
> >
>
Daniel Henrique Barboza Nov. 29, 2017, 8:57 a.m. UTC | #4
On 11/28/2017 10:02 PM, Michael Roth wrote:
> Quoting Daniel Henrique Barboza (2017-11-09 06:35:30)
>>
>> On 11/06/2017 03:46 PM, Laurent Vivier wrote:
>>> On 31/10/2017 21:43, Daniel Henrique Barboza wrote:
>>>> 'ibm,configure-connector' hypercall is used by the guest OS
>>>> to start the configuration of a given device, where the
>>>> machine configures the said device and all its sub-devices,
>>>> giving back FDTs to the caller for each sub-device.
>>>>
>>>> This hypercall is supposed to be called multiple times by the
>>>> guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
>>>> that the device is now properly configured and ready
>>>> to be used, or a return value < 0 when an error occurs.
>>>>
>>>> This patch implements the 'ibm,configure-connector' RTAS
>>>> hypercall in tests/libqos/rtas.c, with an extra test
>>>> case for it inside tests/rtas-tests.c.
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>>>> ---
>>>>    tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
>>>>    tests/libqos/rtas.h |  1 +
>>>>    tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 104 insertions(+)
>>>>
>>>> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
>>>> index ade572a84f..1cb9e2b495 100644
>>>> --- a/tests/libqos/rtas.c
>>>> +++ b/tests/libqos/rtas.c
>>>> @@ -184,3 +184,38 @@ int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
>>>>    
>>>>        return ret[0];
>>>>    }
>>>> +
>>>> +/*
>>>> + * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
>>>> + *
>>>> + * nargs = 2
>>>> + * nrets = 1
>>>> + *
>>>> + * args[0] and args[1] compose the 64 bit Work Area address.
>>>> + *
>>>> + * This call will configure not only the device reported in the first
>>>> + * offset of the Work Area but all of its siblingis as well, returning
>>>> + * the FDT of each configured sub-device as well as a return code
>>>> + * 'Next child', 'Next property' or 'Previous parent'. When the whole
>>>> + * configuration is done, 'Configuration completed' (0) is returned.
>>>> + *
>>>> + * configure-connector will always reply with status code 'Next child'(2)
>>>> + * on the first successful call. The DRC configuration will be
>>>> + * completed when configure-connector returns status 0. Any return
>>>> + * status < 0 indicates an error.
>>>> + */
>>>> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
>>>> +{
>>>> +    uint32_t args[2], ret[1];
>>>> +    int res;
>>>> +
>>>> +    args[0] = (uint32_t)(wa_addr);
>>>> +    args[1] = (uint32_t)(wa_addr >> 32);
>>> This part looks strange to me. I agree it's what qemu does, but
>>> according to spec args[0] is "Address of work area" and args[1] "0 or
>>> address of additional page":
>>>
>>> Linux on Power Architecture Platform Reference
>>> Version 1.1
>>> 24 March 2016
>>>
>>> 13.5.3.5 ibm,configure-connector RTAS Call
>>> The “need more memory” status code, is similar in semantics to the “call
>>> again” status. However, on the next ibm,configure-connector call, the OS
>>> will supply, via the Memory extent parameter, the address of another
>>> page of memory for RTAS to add to the work area in order for
>>> configuration to continue. On all other calls to ibm,configure-connector
>>> the contents of the Memory extent parameter should be 0.
>> Hmmmmm thinking about it, the reason why it works in QEMU is because
>> wa_addr >> 32 will always be zero, perhaps?
> It looks that way. At least with linux guests the "need more memory"
> handling remains unimplemented, so we only ever end up dealing with:
>
>    rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
>
> which effectively sets args[1] to 0. Still worth fixing though of
> course.

Thanks for the info Mike. Yeah, this is a good TODO.


Daniel
>
>> At any rate, In this test, makes sense to leave args[1] to be set by the
>> caller,
>> setting it to 0 in the non "need more memory" scenario.
>>
>>
>>>> +    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
>>>> +    if (res != 0) {
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    return ret[0];
>>>> +}
>>>> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
>>>> index 9dfa18f32b..35c44fb967 100644
>>>> --- a/tests/libqos/rtas.h
>>>> +++ b/tests/libqos/rtas.h
>>>> @@ -16,4 +16,5 @@ 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);
>>>> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
>>>>    #endif /* LIBQOS_RTAS_H */
>>>> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
>>>> index 2c34b6e83c..b3538bf878 100644
>>>> --- a/tests/rtas-test.c
>>>> +++ b/tests/rtas-test.c
>>>> @@ -15,6 +15,8 @@
>>>>    #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
>>>>    #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
>>>>    
>>>> +#define CC_WA_LEN 4096
>>>> +
>>>>    static void test_rtas_get_time_of_day(void)
>>>>    {
>>>>        QOSState *qs;
>>>> @@ -169,6 +171,70 @@ static void test_rtas_set_indicator(void)
>>>>        qtest_shutdown(qs);
>>>>    }
>>>>    
>>>> +static void test_rtas_ibm_configure_connector(void)
>>>> +{
>>>> +    QOSState *qs;
>>>> +    uint64_t ret;
>>>> +    uintptr_t guest_buf_addr, guest_drc_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);
>>>> +
>>>> +    /*
>>>> +     * Same 108 bytes offset magic used and explained in
>>>> +     * test_rtas_set_indicator.
>>>> +     */
>>>> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));
>>>> +    g_free(buf);
>>>> +
>>>> +    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);
>>>> +
>>>> +    /*
>>>> +     * Call ibm,configure-connector to finish the hotplugged device
>>>> +     * configuration, putting its DRC into 'ready' state.
>>>> +     *
>>>> +     * We're not interested in the generated FDTs during the config
>>>> +     * process, thus we simply keep calling configure-connector
>>>> +     * until it returns SUCCESS(0) or an error.
>>>> +     *
>>>> +     * The full explanation logic behind this process can be found
>>>> +     * at PAPR 2.7+, 13.5.3.5.
>>>> +     */
>>>> +    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));
>>> CC_WA_LEN is already 4096, why "* sizeof(uint32_t"?
>>> It must be aligned to a 4096 bytes boundaries.
>> Good catch. I'll fix it.
>>
>>>> +    writel(guest_drc_addr, drc_index);
>>>> +    writel(guest_drc_addr + sizeof(uint32_t), 0)> +
>>>> +    do {
>>>> +        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
>>>> +    } while (ret > 0);
>>> I think it would be interesting to check the result of the call (return
>>> value, node name, property name,...)
>>>
>>> And you should also exit with error in the case of ret == 5 (need more
>>> memory).
>>>
>>>> +    guest_free(qs->alloc, guest_drc_addr);
>>>> +
>>>> +    g_assert_cmpint(ret, ==, 0);
>>>> +
>>>> +    qtest_shutdown(qs);
>>>> +}
>>>> +
>>>>    int main(int argc, char *argv[])
>>>>    {
>>>>        const char *arch = qtest_get_arch();
>>>> @@ -185,6 +251,8 @@ int main(int argc, char *argv[])
>>>>        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);
>>>> +    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
>>>> +                   test_rtas_ibm_configure_connector);
>>>>    
>>>>        return g_test_run();
>>>>    }
>>>>
>>> Thanks,
>>> Laurent
>>>
>
Michael Roth Nov. 29, 2017, 12:21 p.m. UTC | #5
Quoting Daniel Henrique Barboza (2017-10-31 15:43:29)
> 'ibm,configure-connector' hypercall is used by the guest OS
> to start the configuration of a given device, where the
> machine configures the said device and all its sub-devices,
> giving back FDTs to the caller for each sub-device.
> 
> This hypercall is supposed to be called multiple times by the
> guest OS until it returns RTAS_OUT_SUCESS (code 0), indicating
> that the device is now properly configured and ready
> to be used, or a return value < 0 when an error occurs.
> 
> This patch implements the 'ibm,configure-connector' RTAS
> hypercall in tests/libqos/rtas.c, with an extra test
> case for it inside tests/rtas-tests.c.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> ---
>  tests/libqos/rtas.c | 35 +++++++++++++++++++++++++++
>  tests/libqos/rtas.h |  1 +
>  tests/rtas-test.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+)
> 
> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
> index ade572a84f..1cb9e2b495 100644
> --- a/tests/libqos/rtas.c
> +++ b/tests/libqos/rtas.c
> @@ -184,3 +184,38 @@ int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
> 
>      return ret[0];
>  }
> +
> +/*
> + * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
> + *
> + * nargs = 2
> + * nrets = 1
> + *
> + * args[0] and args[1] compose the 64 bit Work Area address.
> + *
> + * This call will configure not only the device reported in the first
> + * offset of the Work Area but all of its siblingis as well, returning
> + * the FDT of each configured sub-device as well as a return code
> + * 'Next child', 'Next property' or 'Previous parent'. When the whole
> + * configuration is done, 'Configuration completed' (0) is returned.
> + *
> + * configure-connector will always reply with status code 'Next child'(2)
> + * on the first successful call. The DRC configuration will be
> + * completed when configure-connector returns status 0. Any return
> + * status < 0 indicates an error.
> + */
> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
> +{
> +    uint32_t args[2], ret[1];
> +    int res;
> +
> +    args[0] = (uint32_t)(wa_addr);
> +    args[1] = (uint32_t)(wa_addr >> 32);
> +
> +    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
> +    if (res != 0) {
> +        return -1;
> +    }
> +
> +    return ret[0];
> +}
> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
> index 9dfa18f32b..35c44fb967 100644
> --- a/tests/libqos/rtas.h
> +++ b/tests/libqos/rtas.h
> @@ -16,4 +16,5 @@ 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);
> +int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
>  #endif /* LIBQOS_RTAS_H */
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> index 2c34b6e83c..b3538bf878 100644
> --- a/tests/rtas-test.c
> +++ b/tests/rtas-test.c
> @@ -15,6 +15,8 @@
>  #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
>  #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
> 
> +#define CC_WA_LEN 4096
> +
>  static void test_rtas_get_time_of_day(void)
>  {
>      QOSState *qs;
> @@ -169,6 +171,70 @@ static void test_rtas_set_indicator(void)
>      qtest_shutdown(qs);
>  }
> 
> +static void test_rtas_ibm_configure_connector(void)
> +{
> +    QOSState *qs;
> +    uint64_t ret;
> +    uintptr_t guest_buf_addr, guest_drc_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);
> +
> +    /*
> +     * Same 108 bytes offset magic used and explained in
> +     * test_rtas_set_indicator.
> +     */
> +    drc_index = be32toh(*((uint32_t *)(buf + 108)));
> +    g_free(buf);
> +
> +    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);
> +
> +    /*
> +     * Call ibm,configure-connector to finish the hotplugged device
> +     * configuration, putting its DRC into 'ready' state.
> +     *
> +     * We're not interested in the generated FDTs during the config
> +     * process, thus we simply keep calling configure-connector
> +     * until it returns SUCCESS(0) or an error.
> +     *
> +     * The full explanation logic behind this process can be found
> +     * at PAPR 2.7+, 13.5.3.5.
> +     */
> +    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));
> +    writel(guest_drc_addr, drc_index);
> +    writel(guest_drc_addr + sizeof(uint32_t), 0);
> +
> +    do {
> +        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
> +    } while (ret > 0);

This should result in a transition to a 'configured' state, but it's
hard to probe that sort of stuff using the rtas interfaces since it's
sort of an internal state that the guest doesn't set/read directly. I
think it makes sense focusing on making sure everything is coherant
from a guest perspective here, but maybe it would be good to eventually
add a set of drc/* tests to validate things like the drc->state
transitions themselves under various normal/edge cases.

> +
> +    guest_free(qs->alloc, guest_drc_addr);
> +
> +    g_assert_cmpint(ret, ==, 0);
> +
> +    qtest_shutdown(qs);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      const char *arch = qtest_get_arch();
> @@ -185,6 +251,8 @@ int main(int argc, char *argv[])
>      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);
> +    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
> +                   test_rtas_ibm_configure_connector);
> 
>      return g_test_run();
>  }
> -- 
> 2.13.6
>
diff mbox

Patch

diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
index ade572a84f..1cb9e2b495 100644
--- a/tests/libqos/rtas.c
+++ b/tests/libqos/rtas.c
@@ -184,3 +184,38 @@  int qrtas_set_indicator(QGuestAllocator *alloc, uint32_t type, uint32_t idx,
 
     return ret[0];
 }
+
+/*
+ * ibm,configure-connector as defined by PAPR 2.7+, 13.5.3.5
+ *
+ * nargs = 2
+ * nrets = 1
+ *
+ * args[0] and args[1] compose the 64 bit Work Area address.
+ *
+ * This call will configure not only the device reported in the first
+ * offset of the Work Area but all of its siblingis as well, returning
+ * the FDT of each configured sub-device as well as a return code
+ * 'Next child', 'Next property' or 'Previous parent'. When the whole
+ * configuration is done, 'Configuration completed' (0) is returned.
+ *
+ * configure-connector will always reply with status code 'Next child'(2)
+ * on the first successful call. The DRC configuration will be
+ * completed when configure-connector returns status 0. Any return
+ * status < 0 indicates an error.
+ */
+int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr)
+{
+    uint32_t args[2], ret[1];
+    int res;
+
+    args[0] = (uint32_t)(wa_addr);
+    args[1] = (uint32_t)(wa_addr >> 32);
+
+    res = qrtas_call(alloc, "ibm,configure-connector", 2, args, 1, ret);
+    if (res != 0) {
+        return -1;
+    }
+
+    return ret[0];
+}
diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
index 9dfa18f32b..35c44fb967 100644
--- a/tests/libqos/rtas.h
+++ b/tests/libqos/rtas.h
@@ -16,4 +16,5 @@  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);
+int qrtas_ibm_configure_connector(QGuestAllocator *alloc, uintptr_t wa_addr);
 #endif /* LIBQOS_RTAS_H */
diff --git a/tests/rtas-test.c b/tests/rtas-test.c
index 2c34b6e83c..b3538bf878 100644
--- a/tests/rtas-test.c
+++ b/tests/rtas-test.c
@@ -15,6 +15,8 @@ 
 #define SPAPR_DR_ALLOCATION_STATE_USABLE  1
 #define SPAPR_DR_ISOLATION_STATE_UNISOLATED 1
 
+#define CC_WA_LEN 4096
+
 static void test_rtas_get_time_of_day(void)
 {
     QOSState *qs;
@@ -169,6 +171,70 @@  static void test_rtas_set_indicator(void)
     qtest_shutdown(qs);
 }
 
+static void test_rtas_ibm_configure_connector(void)
+{
+    QOSState *qs;
+    uint64_t ret;
+    uintptr_t guest_buf_addr, guest_drc_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);
+
+    /*
+     * Same 108 bytes offset magic used and explained in
+     * test_rtas_set_indicator.
+     */
+    drc_index = be32toh(*((uint32_t *)(buf + 108)));
+    g_free(buf);
+
+    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);
+
+    /*
+     * Call ibm,configure-connector to finish the hotplugged device
+     * configuration, putting its DRC into 'ready' state.
+     *
+     * We're not interested in the generated FDTs during the config
+     * process, thus we simply keep calling configure-connector
+     * until it returns SUCCESS(0) or an error.
+     *
+     * The full explanation logic behind this process can be found
+     * at PAPR 2.7+, 13.5.3.5.
+     */
+    guest_drc_addr = guest_alloc(qs->alloc, CC_WA_LEN * sizeof(uint32_t));
+    writel(guest_drc_addr, drc_index);
+    writel(guest_drc_addr + sizeof(uint32_t), 0);
+
+    do {
+        ret = qrtas_ibm_configure_connector(qs->alloc, guest_drc_addr);
+    } while (ret > 0);
+
+    guest_free(qs->alloc, guest_drc_addr);
+
+    g_assert_cmpint(ret, ==, 0);
+
+    qtest_shutdown(qs);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -185,6 +251,8 @@  int main(int argc, char *argv[])
     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);
+    qtest_add_func("rtas/test_rtas_ibm_configure_connector",
+                   test_rtas_ibm_configure_connector);
 
     return g_test_run();
 }