* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 10:07 ` Laurent Vivier
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Vivier @ 2016-03-02 10:07 UTC (permalink / raw)
To: Thomas Huth, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 09:40, Thomas Huth wrote:
> Introduce a test for sPAPR hypercalls, starting with the
> three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> v3:
> - Use report_xfail() in H_RANDOM test to report whether the
> h-call is available or not
>
> v2:
> - Rebased to the final version of Andrew's initial ppc64
> support patches that got merged yesterday
> - Added a test for the H_RANDOM hypercall
>
> Please note that you need the latest QEMU development version
> since some of the hypercalls have only been added recently.
>
> In case somebody wants to review the description of the h-calls, you
> can find them in LoPAPR, chapters 14.5.4.3.1, 14.5.4.3.3 and 14.15.1.
> See: https://members.openpowerfoundation.org/document/dl/469
>
> lib/powerpc/asm/hcall.h | 3 +
> powerpc/Makefile.common | 5 +-
> powerpc/spapr_hcall.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 3 +
> 4 files changed, 177 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/spapr_hcall.c
>
> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> index 750c655..f6f9ea8 100644
> --- a/lib/powerpc/asm/hcall.h
> +++ b/lib/powerpc/asm/hcall.h
> @@ -15,8 +15,11 @@
> #define H_PRIVILEGE -3
> #define H_PARAMETER -4
>
> +#define H_SET_SPRG0 0x24
> #define H_SET_DABR 0x28
> +#define H_PAGE_INIT 0x2c
> #define H_PUT_TERM_CHAR 0x58
> +#define H_RANDOM 0x300
>
> #ifndef __ASSEMBLY__
> /*
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index b526668..2ce6494 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -5,7 +5,8 @@
> #
>
> tests-common = \
> - $(TEST_DIR)/selftest.elf
> + $(TEST_DIR)/selftest.elf \
> + $(TEST_DIR)/spapr_hcall.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> @@ -63,3 +64,5 @@ generated_files = $(asm-offsets)
> test_cases: $(generated_files) $(tests-common) $(tests)
>
> $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
> +
> +$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
> diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
> new file mode 100644
> index 0000000..46731e1
> --- /dev/null
> +++ b/powerpc/spapr_hcall.c
> @@ -0,0 +1,167 @@
> +/*
> + * Test sPAPR hypervisor calls (aka. h-calls)
> + *
> + * Copyright 2016 Thomas Huth, Red Hat Inc.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <util.h>
> +#include <alloc.h>
> +#include <asm/hcall.h>
> +
> +#define PAGE_SIZE 4096
> +
> +#define H_ZERO_PAGE (1UL << (63-48))
> +#define H_COPY_PAGE (1UL << (63-49))
> +
> +#define mfspr(nr) ({ \
> + uint64_t ret; \
> + asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
> + ret; \
> +})
> +
> +#define SPR_SPRG0 0x110
> +
> +/**
> + * Test the H_SET_SPRG0 h-call by setting some values and checking whether
> + * the SPRG0 register contains the correct values afterwards
> + */
> +static void test_h_set_sprg0(int argc, char **argv)
> +{
> + uint64_t sprg0, sprg0_orig;
> + int rc;
> +
> + if (argc > 1)
> + report_abort("Unsupported argument: '%s'", argv[1]);
> +
> + sprg0_orig = mfspr(SPR_SPRG0);
> +
> + rc = hcall(H_SET_SPRG0, 0xcafebabedeadbeefULL);
> + sprg0 = mfspr(SPR_SPRG0);
> + report("sprg0 = 0xcafebabedeadbeef",
> + rc == H_SUCCESS && sprg0 == 0xcafebabedeadbeefULL);
> +
> + rc = hcall(H_SET_SPRG0, 0xaaaaaaaa55555555ULL);
> + sprg0 = mfspr(SPR_SPRG0);
> + report("sprg0 = 0xaaaaaaaa55555555",
> + rc == H_SUCCESS && sprg0 == 0xaaaaaaaa55555555ULL);
> +
> + rc = hcall(H_SET_SPRG0, sprg0_orig);
> + sprg0 = mfspr(SPR_SPRG0);
> + report("sprg0 = 0x%llx",
> + rc == H_SUCCESS && sprg0 == sprg0_orig, sprg0_orig);
> +}
> +
> +/**
> + * Test the H_PAGE_INIT h-call by using it to clear and to copy a page, and
> + * by checking for the correct values in the destination page afterwards
> + */
> +static void test_h_page_init(int argc, char **argv)
> +{
> + u8 *dst, *src;
> + int rc;
> +
> + if (argc > 1)
> + report_abort("Unsupported argument: '%s'", argv[1]);
> +
> + dst = memalign(PAGE_SIZE, PAGE_SIZE);
> + src = memalign(PAGE_SIZE, PAGE_SIZE);
> + if (!dst || !src)
> + report_abort("Failed to alloc memory");
> +
> + memset(dst, 0xaa, PAGE_SIZE);
> + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst, src);
> + report("h_zero_page", rc == H_SUCCESS && *(uint64_t*)dst == 0);
> +
> + *(uint64_t*)src = 0xbeefc0dedeadcafeULL;
> + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src);
> + report("h_copy_page",
> + rc == H_SUCCESS && *(uint64_t*)dst == 0xbeefc0dedeadcafeULL);
> +
> + *(uint64_t*)src = 0x9abcdef012345678ULL;
> + rc = hcall(H_PAGE_INIT, H_COPY_PAGE|H_ZERO_PAGE, dst, src);
> + report("h_copy_page+h_zero_page",
> + rc == H_SUCCESS && *(uint64_t*)dst == 0x9abcdef012345678ULL);
> +
> + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst + 0x123, src);
> + report("h_zero_page unaligned dst", rc == H_PARAMETER);
> +
> + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src + 0x123);
> + report("h_copy_page unaligned src", rc == H_PARAMETER);
> +}
> +
> +static int h_random(uint64_t *val)
> +{
> + register uint64_t r3 asm("r3") = H_RANDOM;
> + register uint64_t r4 asm("r4");
> +
> + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
as you use "+r" with r3 in the output operands field, I think you don't
have to declare it in the input operands field.
> + *val = r4;
> +
> + return r3;
> +}
> +
> +/**
> + * Test H_RANDOM by calling it a couple of times to check whether all bit
> + * positions really toggle (there should be no "stuck" bits in the output)
> + */
> +static void test_h_random(int argc, char **argv)
> +{
> + uint64_t rval, val0, val1;
> + int rc, i;
> +
> + if (argc > 1)
> + report_abort("Unsupported argument: '%s'", argv[1]);
> +
> + /* H_RANDOM is optional - so check for sane return values first */
> + rc = h_random(&rval);
> + report_xfail("h-call available", rc == H_FUNCTION, rc == H_SUCCESS);
> + if (rc != H_SUCCESS)
> + return;
I think it can be disturbing to not always have the same number of
tests, so remove the initial checking and use the report_xfail on the
following report.
> + val0 = 0ULL;
> + val1 = ~0ULL;
> +
> + i = 100;
> + do {
> + rc = h_random(&rval);
> + if (rc != H_SUCCESS)
> + break;
> + val0 |= rval;
> + val1 &= rval;
> + } while (i-- > 0 && (val0 != ~0ULL || val1 != 0ULL));
> +
> + report("no stuck bits", rc == H_SUCCESS && val0 == ~0ULL && val1 == 0);
something like:
report_xfail("no stuck bits", rc == H_FUNCTION, \
rc == H_SUCCESS && val0 == ~0ULL && val1 == 0);
Is it possible to use "ibm,hypertasfunctions" of /rtas node to know if
it is supported (instead of H_FUNCTION)?
> +}
> +
> +struct {
> + const char *name;
> + void (*func)(int argc, char **argv);
> +} hctests[] = {
> + { "h_set_sprg0", test_h_set_sprg0 },
> + { "h_page_init", test_h_page_init },
> + { "h_random", test_h_random },
> + { NULL, NULL }
> +};
> +
> +int main(int argc, char **argv)
> +{
> + int all = 0;
> + int i;
> +
> + report_prefix_push("hypercall");
> +
> + if (!argc || (argc == 1 && !strcmp(argv[0], "all")))
> + all = 1;
> +
> + for (i = 0; hctests[i].name != NULL; i++) {
> + report_prefix_push(hctests[i].name);
> + if (all || strcmp(argv[0], hctests[i].name) == 0) {
> + hctests[i].func(argc, argv);
> + }
> + report_prefix_pop();
> + }
> +
> + return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 60f9be8..d858436 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -28,3 +28,6 @@ file = selftest.elf
> smp = 2
> extra_params = -m 256 -append 'setup smp=2 mem=256'
> groups = selftest
> +
> +[spapr_hcall]
> +file = spapr_hcall.elf
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 10:07 ` Laurent Vivier
@ 2016-03-02 10:22 ` Paolo Bonzini
-1 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2016-03-02 10:22 UTC (permalink / raw)
To: Laurent Vivier, Thomas Huth, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:07, Laurent Vivier wrote:
>> > + report_xfail("h-call available", rc = H_FUNCTION, rc = H_SUCCESS);
>> > + if (rc != H_SUCCESS)
>> > + return;
> I think it can be disturbing to not always have the same number of
> tests, so remove the initial checking and use the report_xfail on the
> following report.
>
>
It's actually fairly common, see for example x86/apic and x86/xsave tests.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 10:22 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2016-03-02 10:22 UTC (permalink / raw)
To: Laurent Vivier, Thomas Huth, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:07, Laurent Vivier wrote:
>> > + report_xfail("h-call available", rc == H_FUNCTION, rc == H_SUCCESS);
>> > + if (rc != H_SUCCESS)
>> > + return;
> I think it can be disturbing to not always have the same number of
> tests, so remove the initial checking and use the report_xfail on the
> following report.
>
>
It's actually fairly common, see for example x86/apic and x86/xsave tests.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 10:22 ` Paolo Bonzini
@ 2016-03-02 10:33 ` Laurent Vivier
-1 siblings, 0 replies; 16+ messages in thread
From: Laurent Vivier @ 2016-03-02 10:33 UTC (permalink / raw)
To: Paolo Bonzini, Thomas Huth, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:22, Paolo Bonzini wrote:
>
>
> On 02/03/2016 11:07, Laurent Vivier wrote:
>>>> + report_xfail("h-call available", rc = H_FUNCTION, rc = H_SUCCESS);
>>>> + if (rc != H_SUCCESS)
>>>> + return;
>> I think it can be disturbing to not always have the same number of
>> tests, so remove the initial checking and use the report_xfail on the
>> following report.
>>
>>
>
> It's actually fairly common, see for example x86/apic and x86/xsave tests.
OK, so you can add my
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
to this patch (the other comment about asm() doesn't worth a re-spin)
Laurent
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 10:33 ` Laurent Vivier
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Vivier @ 2016-03-02 10:33 UTC (permalink / raw)
To: Paolo Bonzini, Thomas Huth, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:22, Paolo Bonzini wrote:
>
>
> On 02/03/2016 11:07, Laurent Vivier wrote:
>>>> + report_xfail("h-call available", rc == H_FUNCTION, rc == H_SUCCESS);
>>>> + if (rc != H_SUCCESS)
>>>> + return;
>> I think it can be disturbing to not always have the same number of
>> tests, so remove the initial checking and use the report_xfail on the
>> following report.
>>
>>
>
> It's actually fairly common, see for example x86/apic and x86/xsave tests.
OK, so you can add my
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
to this patch (the other comment about asm() doesn't worth a re-spin)
Laurent
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 10:07 ` Laurent Vivier
@ 2016-03-02 10:56 ` Thomas Huth
-1 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2016-03-02 10:56 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02.03.2016 11:07, Laurent Vivier wrote:
>
>
> On 02/03/2016 09:40, Thomas Huth wrote:
>> Introduce a test for sPAPR hypercalls, starting with the
>> three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> v3:
>> - Use report_xfail() in H_RANDOM test to report whether the
>> h-call is available or not
>>
>> v2:
>> - Rebased to the final version of Andrew's initial ppc64
>> support patches that got merged yesterday
>> - Added a test for the H_RANDOM hypercall
>>
>> Please note that you need the latest QEMU development version
>> since some of the hypercalls have only been added recently.
>>
>> In case somebody wants to review the description of the h-calls, you
>> can find them in LoPAPR, chapters 14.5.4.3.1, 14.5.4.3.3 and 14.15.1.
>> See: https://members.openpowerfoundation.org/document/dl/469
>>
>> lib/powerpc/asm/hcall.h | 3 +
>> powerpc/Makefile.common | 5 +-
>> powerpc/spapr_hcall.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 3 +
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/spapr_hcall.c
>>
>> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
>> index 750c655..f6f9ea8 100644
>> --- a/lib/powerpc/asm/hcall.h
>> +++ b/lib/powerpc/asm/hcall.h
>> @@ -15,8 +15,11 @@
>> #define H_PRIVILEGE -3
>> #define H_PARAMETER -4
>>
>> +#define H_SET_SPRG0 0x24
>> #define H_SET_DABR 0x28
>> +#define H_PAGE_INIT 0x2c
>> #define H_PUT_TERM_CHAR 0x58
>> +#define H_RANDOM 0x300
>>
>> #ifndef __ASSEMBLY__
>> /*
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index b526668..2ce6494 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -5,7 +5,8 @@
>> #
>>
>> tests-common = \
>> - $(TEST_DIR)/selftest.elf
>> + $(TEST_DIR)/selftest.elf \
>> + $(TEST_DIR)/spapr_hcall.elf
>>
>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>
>> @@ -63,3 +64,5 @@ generated_files = $(asm-offsets)
>> test_cases: $(generated_files) $(tests-common) $(tests)
>>
>> $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>> +
>> +$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>> diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
>> new file mode 100644
>> index 0000000..46731e1
>> --- /dev/null
>> +++ b/powerpc/spapr_hcall.c
...
>> +static int h_random(uint64_t *val)
>> +{
>> + register uint64_t r3 asm("r3") = H_RANDOM;
>> + register uint64_t r4 asm("r4");
>> +
>> + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>
> as you use "+r" with r3 in the output operands field, I think you don't
> have to declare it in the input operands field.
Ah, ok, ... gcc inline assembly is always confusing...
Paolo, Andrew, shall I sent a new version for fixing this nit, or is it
ok to keep it in the current, slightly redundant shape?
>> + *val = r4;
>> +
>> + return r3;
>> +}
>> +
>> +/**
>> + * Test H_RANDOM by calling it a couple of times to check whether all bit
>> + * positions really toggle (there should be no "stuck" bits in the output)
>> + */
>> +static void test_h_random(int argc, char **argv)
>> +{
>> + uint64_t rval, val0, val1;
>> + int rc, i;
>> +
>> + if (argc > 1)
>> + report_abort("Unsupported argument: '%s'", argv[1]);
>> +
>> + /* H_RANDOM is optional - so check for sane return values first */
>> + rc = h_random(&rval);
>> + report_xfail("h-call available", rc = H_FUNCTION, rc = H_SUCCESS);
>> + if (rc != H_SUCCESS)
>> + return;
...
> Is it possible to use "ibm,hypertasfunctions" of /rtas node to know if
> it is supported (instead of H_FUNCTION)?
Theoretically yes, but QEMU still does not provide this in that property
yet, so testing for H_FUNCTION is currently the better solution.
Adding the missing entries to the hypertas property is on my TODO list,
there are also a couple of other entries missing in that property, but I
just did not have time for that yet...
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 10:56 ` Thomas Huth
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2016-03-02 10:56 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02.03.2016 11:07, Laurent Vivier wrote:
>
>
> On 02/03/2016 09:40, Thomas Huth wrote:
>> Introduce a test for sPAPR hypercalls, starting with the
>> three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> v3:
>> - Use report_xfail() in H_RANDOM test to report whether the
>> h-call is available or not
>>
>> v2:
>> - Rebased to the final version of Andrew's initial ppc64
>> support patches that got merged yesterday
>> - Added a test for the H_RANDOM hypercall
>>
>> Please note that you need the latest QEMU development version
>> since some of the hypercalls have only been added recently.
>>
>> In case somebody wants to review the description of the h-calls, you
>> can find them in LoPAPR, chapters 14.5.4.3.1, 14.5.4.3.3 and 14.15.1.
>> See: https://members.openpowerfoundation.org/document/dl/469
>>
>> lib/powerpc/asm/hcall.h | 3 +
>> powerpc/Makefile.common | 5 +-
>> powerpc/spapr_hcall.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
>> powerpc/unittests.cfg | 3 +
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> create mode 100644 powerpc/spapr_hcall.c
>>
>> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
>> index 750c655..f6f9ea8 100644
>> --- a/lib/powerpc/asm/hcall.h
>> +++ b/lib/powerpc/asm/hcall.h
>> @@ -15,8 +15,11 @@
>> #define H_PRIVILEGE -3
>> #define H_PARAMETER -4
>>
>> +#define H_SET_SPRG0 0x24
>> #define H_SET_DABR 0x28
>> +#define H_PAGE_INIT 0x2c
>> #define H_PUT_TERM_CHAR 0x58
>> +#define H_RANDOM 0x300
>>
>> #ifndef __ASSEMBLY__
>> /*
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index b526668..2ce6494 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -5,7 +5,8 @@
>> #
>>
>> tests-common = \
>> - $(TEST_DIR)/selftest.elf
>> + $(TEST_DIR)/selftest.elf \
>> + $(TEST_DIR)/spapr_hcall.elf
>>
>> all: $(TEST_DIR)/boot_rom.bin test_cases
>>
>> @@ -63,3 +64,5 @@ generated_files = $(asm-offsets)
>> test_cases: $(generated_files) $(tests-common) $(tests)
>>
>> $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
>> +
>> +$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
>> diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
>> new file mode 100644
>> index 0000000..46731e1
>> --- /dev/null
>> +++ b/powerpc/spapr_hcall.c
...
>> +static int h_random(uint64_t *val)
>> +{
>> + register uint64_t r3 asm("r3") = H_RANDOM;
>> + register uint64_t r4 asm("r4");
>> +
>> + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>
> as you use "+r" with r3 in the output operands field, I think you don't
> have to declare it in the input operands field.
Ah, ok, ... gcc inline assembly is always confusing...
Paolo, Andrew, shall I sent a new version for fixing this nit, or is it
ok to keep it in the current, slightly redundant shape?
>> + *val = r4;
>> +
>> + return r3;
>> +}
>> +
>> +/**
>> + * Test H_RANDOM by calling it a couple of times to check whether all bit
>> + * positions really toggle (there should be no "stuck" bits in the output)
>> + */
>> +static void test_h_random(int argc, char **argv)
>> +{
>> + uint64_t rval, val0, val1;
>> + int rc, i;
>> +
>> + if (argc > 1)
>> + report_abort("Unsupported argument: '%s'", argv[1]);
>> +
>> + /* H_RANDOM is optional - so check for sane return values first */
>> + rc = h_random(&rval);
>> + report_xfail("h-call available", rc == H_FUNCTION, rc == H_SUCCESS);
>> + if (rc != H_SUCCESS)
>> + return;
...
> Is it possible to use "ibm,hypertasfunctions" of /rtas node to know if
> it is supported (instead of H_FUNCTION)?
Theoretically yes, but QEMU still does not provide this in that property
yet, so testing for H_FUNCTION is currently the better solution.
Adding the missing entries to the hypertas property is on my TODO list,
there are also a couple of other entries missing in that property, but I
just did not have time for that yet...
Thomas
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 10:56 ` Thomas Huth
@ 2016-03-02 11:37 ` Paolo Bonzini
-1 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2016-03-02 11:37 UTC (permalink / raw)
To: Thomas Huth, Laurent Vivier, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:56, Thomas Huth wrote:
> > > +
> > > + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
> >
> > as you use "+r" with r3 in the output operands field, I think you don't
> > have to declare it in the input operands field.
>
> Ah, ok, ... gcc inline assembly is always confusing...
> Paolo, Andrew, shall I sent a new version for fixing this nit, or is it
> ok to keep it in the current, slightly redundant shape?
I guess the current one is fine because of the asm register in the
declarations.
The "really correct" one would use "=r" in the r3 output and "0" in the
r3 input, I think.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 11:37 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2016-03-02 11:37 UTC (permalink / raw)
To: Thomas Huth, Laurent Vivier, kvm, kvm-ppc, drjones; +Cc: dgibson
On 02/03/2016 11:56, Thomas Huth wrote:
> > > +
> > > + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
> >
> > as you use "+r" with r3 in the output operands field, I think you don't
> > have to declare it in the input operands field.
>
> Ah, ok, ... gcc inline assembly is always confusing...
> Paolo, Andrew, shall I sent a new version for fixing this nit, or is it
> ok to keep it in the current, slightly redundant shape?
I guess the current one is fine because of the asm register in the
declarations.
The "really correct" one would use "=r" in the r3 output and "0" in the
r3 input, I think.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 10:07 ` Laurent Vivier
@ 2016-03-02 23:23 ` David Gibson
-1 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2016-03-02 23:23 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Thomas Huth, kvm, kvm-ppc, drjones
[-- Attachment #1: Type: text/plain, Size: 6316 bytes --]
On Wed, 2 Mar 2016 11:07:55 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 02/03/2016 09:40, Thomas Huth wrote:
> > Introduce a test for sPAPR hypercalls, starting with the
> > three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > v3:
> > - Use report_xfail() in H_RANDOM test to report whether the
> > h-call is available or not
> >
> > v2:
> > - Rebased to the final version of Andrew's initial ppc64
> > support patches that got merged yesterday
> > - Added a test for the H_RANDOM hypercall
> >
> > Please note that you need the latest QEMU development version
> > since some of the hypercalls have only been added recently.
> >
> > In case somebody wants to review the description of the h-calls, you
> > can find them in LoPAPR, chapters 14.5.4.3.1, 14.5.4.3.3 and 14.15.1.
> > See: https://members.openpowerfoundation.org/document/dl/469
> >
> > lib/powerpc/asm/hcall.h | 3 +
> > powerpc/Makefile.common | 5 +-
> > powerpc/spapr_hcall.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
> > powerpc/unittests.cfg | 3 +
> > 4 files changed, 177 insertions(+), 1 deletion(-)
> > create mode 100644 powerpc/spapr_hcall.c
> >
> > diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> > index 750c655..f6f9ea8 100644
> > --- a/lib/powerpc/asm/hcall.h
> > +++ b/lib/powerpc/asm/hcall.h
> > @@ -15,8 +15,11 @@
> > #define H_PRIVILEGE -3
> > #define H_PARAMETER -4
> >
> > +#define H_SET_SPRG0 0x24
> > #define H_SET_DABR 0x28
> > +#define H_PAGE_INIT 0x2c
> > #define H_PUT_TERM_CHAR 0x58
> > +#define H_RANDOM 0x300
> >
> > #ifndef __ASSEMBLY__
> > /*
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index b526668..2ce6494 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -5,7 +5,8 @@
> > #
> >
> > tests-common = \
> > - $(TEST_DIR)/selftest.elf
> > + $(TEST_DIR)/selftest.elf \
> > + $(TEST_DIR)/spapr_hcall.elf
> >
> > all: $(TEST_DIR)/boot_rom.bin test_cases
> >
> > @@ -63,3 +64,5 @@ generated_files = $(asm-offsets)
> > test_cases: $(generated_files) $(tests-common) $(tests)
> >
> > $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
> > +
> > +$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
> > diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
> > new file mode 100644
> > index 0000000..46731e1
> > --- /dev/null
> > +++ b/powerpc/spapr_hcall.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Test sPAPR hypervisor calls (aka. h-calls)
> > + *
> > + * Copyright 2016 Thomas Huth, Red Hat Inc.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <util.h>
> > +#include <alloc.h>
> > +#include <asm/hcall.h>
> > +
> > +#define PAGE_SIZE 4096
> > +
> > +#define H_ZERO_PAGE (1UL << (63-48))
> > +#define H_COPY_PAGE (1UL << (63-49))
> > +
> > +#define mfspr(nr) ({ \
> > + uint64_t ret; \
> > + asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
> > + ret; \
> > +})
> > +
> > +#define SPR_SPRG0 0x110
> > +
> > +/**
> > + * Test the H_SET_SPRG0 h-call by setting some values and checking whether
> > + * the SPRG0 register contains the correct values afterwards
> > + */
> > +static void test_h_set_sprg0(int argc, char **argv)
> > +{
> > + uint64_t sprg0, sprg0_orig;
> > + int rc;
> > +
> > + if (argc > 1)
> > + report_abort("Unsupported argument: '%s'", argv[1]);
> > +
> > + sprg0_orig = mfspr(SPR_SPRG0);
> > +
> > + rc = hcall(H_SET_SPRG0, 0xcafebabedeadbeefULL);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0xcafebabedeadbeef",
> > + rc == H_SUCCESS && sprg0 == 0xcafebabedeadbeefULL);
> > +
> > + rc = hcall(H_SET_SPRG0, 0xaaaaaaaa55555555ULL);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0xaaaaaaaa55555555",
> > + rc == H_SUCCESS && sprg0 == 0xaaaaaaaa55555555ULL);
> > +
> > + rc = hcall(H_SET_SPRG0, sprg0_orig);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0x%llx",
> > + rc == H_SUCCESS && sprg0 == sprg0_orig, sprg0_orig);
> > +}
> > +
> > +/**
> > + * Test the H_PAGE_INIT h-call by using it to clear and to copy a page, and
> > + * by checking for the correct values in the destination page afterwards
> > + */
> > +static void test_h_page_init(int argc, char **argv)
> > +{
> > + u8 *dst, *src;
> > + int rc;
> > +
> > + if (argc > 1)
> > + report_abort("Unsupported argument: '%s'", argv[1]);
> > +
> > + dst = memalign(PAGE_SIZE, PAGE_SIZE);
> > + src = memalign(PAGE_SIZE, PAGE_SIZE);
> > + if (!dst || !src)
> > + report_abort("Failed to alloc memory");
> > +
> > + memset(dst, 0xaa, PAGE_SIZE);
> > + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst, src);
> > + report("h_zero_page", rc == H_SUCCESS && *(uint64_t*)dst == 0);
> > +
> > + *(uint64_t*)src = 0xbeefc0dedeadcafeULL;
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src);
> > + report("h_copy_page",
> > + rc == H_SUCCESS && *(uint64_t*)dst == 0xbeefc0dedeadcafeULL);
> > +
> > + *(uint64_t*)src = 0x9abcdef012345678ULL;
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE|H_ZERO_PAGE, dst, src);
> > + report("h_copy_page+h_zero_page",
> > + rc == H_SUCCESS && *(uint64_t*)dst == 0x9abcdef012345678ULL);
> > +
> > + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst + 0x123, src);
> > + report("h_zero_page unaligned dst", rc == H_PARAMETER);
> > +
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src + 0x123);
> > + report("h_copy_page unaligned src", rc == H_PARAMETER);
> > +}
> > +
> > +static int h_random(uint64_t *val)
> > +{
> > + register uint64_t r3 asm("r3") = H_RANDOM;
> > + register uint64_t r4 asm("r4");
> > +
> > + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>
> as you use "+r" with r3 in the output operands field, I think you don't
> have to declare it in the input operands field.
More importantly, I think a hypercall is allowed to clobber any of the
volatile registers (r0 & r3..r12), so they probably need to be declared
in the clobbers as well.
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-02 23:23 ` David Gibson
0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2016-03-02 23:23 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Thomas Huth, kvm, kvm-ppc, drjones
[-- Attachment #1: Type: text/plain, Size: 6316 bytes --]
On Wed, 2 Mar 2016 11:07:55 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 02/03/2016 09:40, Thomas Huth wrote:
> > Introduce a test for sPAPR hypercalls, starting with the
> > three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > v3:
> > - Use report_xfail() in H_RANDOM test to report whether the
> > h-call is available or not
> >
> > v2:
> > - Rebased to the final version of Andrew's initial ppc64
> > support patches that got merged yesterday
> > - Added a test for the H_RANDOM hypercall
> >
> > Please note that you need the latest QEMU development version
> > since some of the hypercalls have only been added recently.
> >
> > In case somebody wants to review the description of the h-calls, you
> > can find them in LoPAPR, chapters 14.5.4.3.1, 14.5.4.3.3 and 14.15.1.
> > See: https://members.openpowerfoundation.org/document/dl/469
> >
> > lib/powerpc/asm/hcall.h | 3 +
> > powerpc/Makefile.common | 5 +-
> > powerpc/spapr_hcall.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++
> > powerpc/unittests.cfg | 3 +
> > 4 files changed, 177 insertions(+), 1 deletion(-)
> > create mode 100644 powerpc/spapr_hcall.c
> >
> > diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> > index 750c655..f6f9ea8 100644
> > --- a/lib/powerpc/asm/hcall.h
> > +++ b/lib/powerpc/asm/hcall.h
> > @@ -15,8 +15,11 @@
> > #define H_PRIVILEGE -3
> > #define H_PARAMETER -4
> >
> > +#define H_SET_SPRG0 0x24
> > #define H_SET_DABR 0x28
> > +#define H_PAGE_INIT 0x2c
> > #define H_PUT_TERM_CHAR 0x58
> > +#define H_RANDOM 0x300
> >
> > #ifndef __ASSEMBLY__
> > /*
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index b526668..2ce6494 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -5,7 +5,8 @@
> > #
> >
> > tests-common = \
> > - $(TEST_DIR)/selftest.elf
> > + $(TEST_DIR)/selftest.elf \
> > + $(TEST_DIR)/spapr_hcall.elf
> >
> > all: $(TEST_DIR)/boot_rom.bin test_cases
> >
> > @@ -63,3 +64,5 @@ generated_files = $(asm-offsets)
> > test_cases: $(generated_files) $(tests-common) $(tests)
> >
> > $(TEST_DIR)/selftest.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/selftest.o
> > +
> > +$(TEST_DIR)/spapr_hcall.elf: $(cstart.o) $(reloc.o) $(TEST_DIR)/spapr_hcall.o
> > diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
> > new file mode 100644
> > index 0000000..46731e1
> > --- /dev/null
> > +++ b/powerpc/spapr_hcall.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Test sPAPR hypervisor calls (aka. h-calls)
> > + *
> > + * Copyright 2016 Thomas Huth, Red Hat Inc.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <util.h>
> > +#include <alloc.h>
> > +#include <asm/hcall.h>
> > +
> > +#define PAGE_SIZE 4096
> > +
> > +#define H_ZERO_PAGE (1UL << (63-48))
> > +#define H_COPY_PAGE (1UL << (63-49))
> > +
> > +#define mfspr(nr) ({ \
> > + uint64_t ret; \
> > + asm volatile("mfspr %0,%1" : "=r"(ret) : "i"(nr)); \
> > + ret; \
> > +})
> > +
> > +#define SPR_SPRG0 0x110
> > +
> > +/**
> > + * Test the H_SET_SPRG0 h-call by setting some values and checking whether
> > + * the SPRG0 register contains the correct values afterwards
> > + */
> > +static void test_h_set_sprg0(int argc, char **argv)
> > +{
> > + uint64_t sprg0, sprg0_orig;
> > + int rc;
> > +
> > + if (argc > 1)
> > + report_abort("Unsupported argument: '%s'", argv[1]);
> > +
> > + sprg0_orig = mfspr(SPR_SPRG0);
> > +
> > + rc = hcall(H_SET_SPRG0, 0xcafebabedeadbeefULL);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0xcafebabedeadbeef",
> > + rc == H_SUCCESS && sprg0 == 0xcafebabedeadbeefULL);
> > +
> > + rc = hcall(H_SET_SPRG0, 0xaaaaaaaa55555555ULL);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0xaaaaaaaa55555555",
> > + rc == H_SUCCESS && sprg0 == 0xaaaaaaaa55555555ULL);
> > +
> > + rc = hcall(H_SET_SPRG0, sprg0_orig);
> > + sprg0 = mfspr(SPR_SPRG0);
> > + report("sprg0 = 0x%llx",
> > + rc == H_SUCCESS && sprg0 == sprg0_orig, sprg0_orig);
> > +}
> > +
> > +/**
> > + * Test the H_PAGE_INIT h-call by using it to clear and to copy a page, and
> > + * by checking for the correct values in the destination page afterwards
> > + */
> > +static void test_h_page_init(int argc, char **argv)
> > +{
> > + u8 *dst, *src;
> > + int rc;
> > +
> > + if (argc > 1)
> > + report_abort("Unsupported argument: '%s'", argv[1]);
> > +
> > + dst = memalign(PAGE_SIZE, PAGE_SIZE);
> > + src = memalign(PAGE_SIZE, PAGE_SIZE);
> > + if (!dst || !src)
> > + report_abort("Failed to alloc memory");
> > +
> > + memset(dst, 0xaa, PAGE_SIZE);
> > + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst, src);
> > + report("h_zero_page", rc == H_SUCCESS && *(uint64_t*)dst == 0);
> > +
> > + *(uint64_t*)src = 0xbeefc0dedeadcafeULL;
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src);
> > + report("h_copy_page",
> > + rc == H_SUCCESS && *(uint64_t*)dst == 0xbeefc0dedeadcafeULL);
> > +
> > + *(uint64_t*)src = 0x9abcdef012345678ULL;
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE|H_ZERO_PAGE, dst, src);
> > + report("h_copy_page+h_zero_page",
> > + rc == H_SUCCESS && *(uint64_t*)dst == 0x9abcdef012345678ULL);
> > +
> > + rc = hcall(H_PAGE_INIT, H_ZERO_PAGE, dst + 0x123, src);
> > + report("h_zero_page unaligned dst", rc == H_PARAMETER);
> > +
> > + rc = hcall(H_PAGE_INIT, H_COPY_PAGE, dst, src + 0x123);
> > + report("h_copy_page unaligned src", rc == H_PARAMETER);
> > +}
> > +
> > +static int h_random(uint64_t *val)
> > +{
> > + register uint64_t r3 asm("r3") = H_RANDOM;
> > + register uint64_t r4 asm("r4");
> > +
> > + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>
> as you use "+r" with r3 in the output operands field, I think you don't
> have to declare it in the input operands field.
More importantly, I think a hypercall is allowed to clobber any of the
volatile registers (r0 & r3..r12), so they probably need to be declared
in the clobbers as well.
--
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
2016-03-02 23:23 ` David Gibson
@ 2016-03-03 8:58 ` Thomas Huth
-1 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2016-03-03 8:58 UTC (permalink / raw)
To: David Gibson, Laurent Vivier; +Cc: kvm, kvm-ppc, drjones
[-- Attachment #1: Type: text/plain, Size: 962 bytes --]
On 03.03.2016 00:23, David Gibson wrote:
> On Wed, 2 Mar 2016 11:07:55 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> On 02/03/2016 09:40, Thomas Huth wrote:
>>> Introduce a test for sPAPR hypercalls, starting with the
>>> three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
...
>>> +static int h_random(uint64_t *val)
>>> +{
>>> + register uint64_t r3 asm("r3") = H_RANDOM;
>>> + register uint64_t r4 asm("r4");
>>> +
>>> + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>>
>> as you use "+r" with r3 in the output operands field, I think you don't
>> have to declare it in the input operands field.
>
> More importantly, I think a hypercall is allowed to clobber any of the
> volatile registers (r0 & r3..r12), so they probably need to be declared
> in the clobbers as well.
Ok, that's a point ... I'll fix it up and send a v4.
Thomas
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3] powerpc: Add tests for sPAPR h-calls
@ 2016-03-03 8:58 ` Thomas Huth
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2016-03-03 8:58 UTC (permalink / raw)
To: David Gibson, Laurent Vivier; +Cc: kvm, kvm-ppc, drjones
[-- Attachment #1: Type: text/plain, Size: 962 bytes --]
On 03.03.2016 00:23, David Gibson wrote:
> On Wed, 2 Mar 2016 11:07:55 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> On 02/03/2016 09:40, Thomas Huth wrote:
>>> Introduce a test for sPAPR hypercalls, starting with the
>>> three hypercalls H_SET_SPRG0, H_PAGE_INIT and H_RANDOM.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
...
>>> +static int h_random(uint64_t *val)
>>> +{
>>> + register uint64_t r3 asm("r3") = H_RANDOM;
>>> + register uint64_t r4 asm("r4");
>>> +
>>> + asm volatile (" sc 1 " : "+r"(r3), "=r"(r4) : "r"(r3));
>>
>> as you use "+r" with r3 in the output operands field, I think you don't
>> have to declare it in the input operands field.
>
> More importantly, I think a hypercall is allowed to clobber any of the
> volatile registers (r0 & r3..r12), so they probably need to be declared
> in the clobbers as well.
Ok, that's a point ... I'll fix it up and send a v4.
Thomas
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread