[kvm-unit-tests PULL 0/2] Ppc next patches

[kvm-unit-tests PULL 0/2] Ppc next patches

[Laurent Vivier]

The following changes since commit 14bc602f3479d3f5c5e11034daa1070c61bd5640:

  Merge tag 'pull-request-2019-04-19' of https://gitlab.com/huth/kvm-unit-tests (2019-05-03 10:53:30 -0600)

are available in the Git repository at:

  https://github.com/vivier/kvm-unit-tests.git tags/ppc-next-pull-request

for you to fetch changes up to aa3a3a9e6654fce23a78141c8cfadcd6ba871af1:

  powerpc: Make h_cede_tm test run by default (2019-05-17 13:50:47 +0200)

----------------------------------------------------------------
Fix h_cede_tm timeout

----------------------------------------------------------------

Suraj Jitindar Singh (2):
  powerpc: Allow for a custom decr value to be specified to load on decr
    excp
  powerpc: Make h_cede_tm test run by default

 lib/powerpc/handlers.c | 7 ++++---
 powerpc/sprs.c         | 3 ++-
 powerpc/tm.c           | 4 +++-
 powerpc/unittests.cfg  | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.20.1

[kvm-unit-tests PULL 1/2] powerpc: Allow for a custom decr value to be specified to load on decr excp

[Laurent Vivier]

From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Currently the handler for a decrementer exception will simply reload the
maximum value (0x7FFFFFFF), which will take ~4 seconds to expire again.
This means that if a vcpu cedes, it will be ~4 seconds between wakeups.

The h_cede_tm test is testing a known breakage when a guest cedes while
suspended. To be sure we cede 500 times to check for the bug. However
since it takes ~4 seconds to be woken up once we've ceded, we only get
through ~20 iterations before we reach the 90 seconds timeout and the
test appears to fail.

Add an option when registering the decrementer handler to specify the
value which should be reloaded by the handler, allowing the timeout to be
chosen.

Modify the spr test to use the max timeout to preserve existing
behaviour.
Modify the h_cede_tm test to use a 10ms timeout to ensure we can perform
500 iterations before hitting the 90 second time limit for a test.

This means the h_cede_tm test now succeeds rather than timing out.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
[lv: reset initial value to 0x3FFFFFFF]
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 lib/powerpc/handlers.c | 7 ++++---
 powerpc/sprs.c         | 3 ++-
 powerpc/tm.c           | 4 +++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
index be8226a2e573..c8721e0a11ae 100644
--- a/lib/powerpc/handlers.c
+++ b/lib/powerpc/handlers.c
@@ -12,11 +12,12 @@
 
 /*
  * Generic handler for decrementer exceptions (0x900)
- * Just reset the decrementer back to its maximum value (0x7FFFFFFF)
+ * Just reset the decrementer back to the value specified when registering the
+ * handler
  */
-void dec_except_handler(struct pt_regs *regs __unused, void *data __unused)
+void dec_except_handler(struct pt_regs *regs __unused, void *data)
 {
-	uint32_t dec = 0x7FFFFFFF;
+	uint64_t dec = *((uint64_t *) data);
 
 	asm volatile ("mtdec %0" : : "r" (dec));
 }
diff --git a/powerpc/sprs.c b/powerpc/sprs.c
index 6744bd8d8049..3c2d98c9ca99 100644
--- a/powerpc/sprs.c
+++ b/powerpc/sprs.c
@@ -253,6 +253,7 @@ int main(int argc, char **argv)
 		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
 		-1ULL,
 	};
+	static uint64_t decr = 0x7FFFFFFF; /* Max value */
 
 	for (i = 1; i < argc; i++) {
 		if (!strcmp(argv[i], "-w")) {
@@ -288,7 +289,7 @@ int main(int argc, char **argv)
 		(void) getchar();
 	} else {
 		puts("Sleeping...\n");
-		handle_exception(0x900, &dec_except_handler, NULL);
+		handle_exception(0x900, &dec_except_handler, &decr);
 		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
 		hcall(H_CEDE);
 	}
diff --git a/powerpc/tm.c b/powerpc/tm.c
index bd56baa5b3d8..c588985352f4 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -95,11 +95,13 @@ static bool enable_tm(void)
 static void test_h_cede_tm(int argc, char **argv)
 {
 	int i;
+	static uint64_t decr = 0x3FFFFF; /* ~10ms */
 
 	if (argc > 2)
 		report_abort("Unsupported argument: '%s'", argv[2]);
 
-	handle_exception(0x900, &dec_except_handler, NULL);
+	handle_exception(0x900, &dec_except_handler, &decr);
+	asm volatile ("mtdec %0" : : "r" (decr));
 
 	if (!start_all_cpus(halt, 0))
 		report_abort("Failed to start secondary cpus");
-- 
2.20.1

Re: [kvm-unit-tests PULL 1/2] powerpc: Allow for a custom decr value to be specified to load on decr excp

[Thomas Huth]

On 17/05/2019 15.03, Laurent Vivier wrote:
> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> 
> Currently the handler for a decrementer exception will simply reload the
> maximum value (0x7FFFFFFF), which will take ~4 seconds to expire again.
> This means that if a vcpu cedes, it will be ~4 seconds between wakeups.
> 
> The h_cede_tm test is testing a known breakage when a guest cedes while
> suspended. To be sure we cede 500 times to check for the bug. However
> since it takes ~4 seconds to be woken up once we've ceded, we only get
> through ~20 iterations before we reach the 90 seconds timeout and the
> test appears to fail.
> 
> Add an option when registering the decrementer handler to specify the
> value which should be reloaded by the handler, allowing the timeout to be
> chosen.
> 
> Modify the spr test to use the max timeout to preserve existing
> behaviour.
> Modify the h_cede_tm test to use a 10ms timeout to ensure we can perform
> 500 iterations before hitting the 90 second time limit for a test.
> 
> This means the h_cede_tm test now succeeds rather than timing out.
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> [lv: reset initial value to 0x3FFFFFFF]

Looks like something went wrong here? There is still the 0x7FFFFFFF in
the hunk below...

> diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> index 6744bd8d8049..3c2d98c9ca99 100644
> --- a/powerpc/sprs.c
> +++ b/powerpc/sprs.c
> @@ -253,6 +253,7 @@ int main(int argc, char **argv)
>  		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
>  		-1ULL,
>  	};
> +	static uint64_t decr = 0x7FFFFFFF; /* Max value */
>  
>  	for (i = 1; i < argc; i++) {
>  		if (!strcmp(argv[i], "-w")) {
> @@ -288,7 +289,7 @@ int main(int argc, char **argv)
>  		(void) getchar();
>  	} else {
>  		puts("Sleeping...\n");
> -		handle_exception(0x900, &dec_except_handler, NULL);
> +		handle_exception(0x900, &dec_except_handler, &decr);
>  		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
>  		hcall(H_CEDE);
>  	}

 Thomas

Re: [kvm-unit-tests PULL 1/2] powerpc: Allow for a custom decr value to be specified to load on decr excp

[Suraj Jitindar Singh]

On Fri, 2019-05-17 at 15:10 +0200, Thomas Huth wrote:
> On 17/05/2019 15.03, Laurent Vivier wrote:
> > From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > 
> > Currently the handler for a decrementer exception will simply
> > reload the
> > maximum value (0x7FFFFFFF), which will take ~4 seconds to expire
> > again.
> > This means that if a vcpu cedes, it will be ~4 seconds between
> > wakeups.
> > 
> > The h_cede_tm test is testing a known breakage when a guest cedes
> > while
> > suspended. To be sure we cede 500 times to check for the bug.
> > However
> > since it takes ~4 seconds to be woken up once we've ceded, we only
> > get
> > through ~20 iterations before we reach the 90 seconds timeout and
> > the
> > test appears to fail.
> > 
> > Add an option when registering the decrementer handler to specify
> > the
> > value which should be reloaded by the handler, allowing the timeout
> > to be
> > chosen.
> > 
> > Modify the spr test to use the max timeout to preserve existing
> > behaviour.
> > Modify the h_cede_tm test to use a 10ms timeout to ensure we can
> > perform
> > 500 iterations before hitting the 90 second time limit for a test.
> > 
> > This means the h_cede_tm test now succeeds rather than timing out.
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> > [lv: reset initial value to 0x3FFFFFFF]
> 
> Looks like something went wrong here? There is still the 0x7FFFFFFF
> in
> the hunk below...

No, I think this is correct.
Max value is ox7FFFFFFF, but the initial value we load via mtdec is the
original 0x3FFFFFFF.

> 
> > diff --git a/powerpc/sprs.c b/powerpc/sprs.c
> > index 6744bd8d8049..3c2d98c9ca99 100644
> > --- a/powerpc/sprs.c
> > +++ b/powerpc/sprs.c
> > @@ -253,6 +253,7 @@ int main(int argc, char **argv)
> >  		0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL,
> >  		-1ULL,
> >  	};
> > +	static uint64_t decr = 0x7FFFFFFF; /* Max value */
> >  
> >  	for (i = 1; i < argc; i++) {
> >  		if (!strcmp(argv[i], "-w")) {
> > @@ -288,7 +289,7 @@ int main(int argc, char **argv)
> >  		(void) getchar();
> >  	} else {
> >  		puts("Sleeping...\n");
> > -		handle_exception(0x900, &dec_except_handler,
> > NULL);
> > +		handle_exception(0x900, &dec_except_handler,
> > &decr);
> >  		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
> >  		hcall(H_CEDE);
> >  	}
> 
>  Thomas

Re: [kvm-unit-tests PULL 1/2] powerpc: Allow for a custom decr value to be specified to load on decr excp

[Laurent Vivier]

On 20/05/2019 06:47, Suraj Jitindar Singh wrote:
> On Fri, 2019-05-17 at 15:10 +0200, Thomas Huth wrote:
>> On 17/05/2019 15.03, Laurent Vivier wrote:
>>> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>>
>>> Currently the handler for a decrementer exception will simply
>>> reload the
>>> maximum value (0x7FFFFFFF), which will take ~4 seconds to expire
>>> again.
>>> This means that if a vcpu cedes, it will be ~4 seconds between
>>> wakeups.
>>>
>>> The h_cede_tm test is testing a known breakage when a guest cedes
>>> while
>>> suspended. To be sure we cede 500 times to check for the bug.
>>> However
>>> since it takes ~4 seconds to be woken up once we've ceded, we only
>>> get
>>> through ~20 iterations before we reach the 90 seconds timeout and
>>> the
>>> test appears to fail.
>>>
>>> Add an option when registering the decrementer handler to specify
>>> the
>>> value which should be reloaded by the handler, allowing the timeout
>>> to be
>>> chosen.
>>>
>>> Modify the spr test to use the max timeout to preserve existing
>>> behaviour.
>>> Modify the h_cede_tm test to use a 10ms timeout to ensure we can
>>> perform
>>> 500 iterations before hitting the 90 second time limit for a test.
>>>
>>> This means the h_cede_tm test now succeeds rather than timing out.
>>>
>>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>>> [lv: reset initial value to 0x3FFFFFFF]
>>
>> Looks like something went wrong here? There is still the 0x7FFFFFFF
>> in
>> the hunk below...
> 
> No, I think this is correct.
> Max value is ox7FFFFFFF, but the initial value we load via mtdec is the
> original 0x3FFFFFFF.

Yes, that's it.

Thanks,
Laurent

Re: [kvm-unit-tests PULL 1/2] powerpc: Allow for a custom decr value to be specified to load on decr excp

[Thomas Huth]

On 20/05/2019 08.12, Laurent Vivier wrote:
> On 20/05/2019 06:47, Suraj Jitindar Singh wrote:
>> On Fri, 2019-05-17 at 15:10 +0200, Thomas Huth wrote:
>>> On 17/05/2019 15.03, Laurent Vivier wrote:
>>>> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>>>
>>>> Currently the handler for a decrementer exception will simply
>>>> reload the
>>>> maximum value (0x7FFFFFFF), which will take ~4 seconds to expire
>>>> again.
>>>> This means that if a vcpu cedes, it will be ~4 seconds between
>>>> wakeups.
>>>>
>>>> The h_cede_tm test is testing a known breakage when a guest cedes
>>>> while
>>>> suspended. To be sure we cede 500 times to check for the bug.
>>>> However
>>>> since it takes ~4 seconds to be woken up once we've ceded, we only
>>>> get
>>>> through ~20 iterations before we reach the 90 seconds timeout and
>>>> the
>>>> test appears to fail.
>>>>
>>>> Add an option when registering the decrementer handler to specify
>>>> the
>>>> value which should be reloaded by the handler, allowing the timeout
>>>> to be
>>>> chosen.
>>>>
>>>> Modify the spr test to use the max timeout to preserve existing
>>>> behaviour.
>>>> Modify the h_cede_tm test to use a 10ms timeout to ensure we can
>>>> perform
>>>> 500 iterations before hitting the 90 second time limit for a test.
>>>>
>>>> This means the h_cede_tm test now succeeds rather than timing out.
>>>>
>>>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>>>> [lv: reset initial value to 0x3FFFFFFF]
>>>
>>> Looks like something went wrong here? There is still the 0x7FFFFFFF
>>> in
>>> the hunk below...
>>
>> No, I think this is correct.
>> Max value is ox7FFFFFFF, but the initial value we load via mtdec is the
>> original 0x3FFFFFFF.
> 
> Yes, that's it.

Ok, since we're only calling H_CEDE once here, we should be fine,
indeed. Sorry for the confusion.

 Thomas

[kvm-unit-tests PULL 2/2] powerpc: Make h_cede_tm test run by default

[Laurent Vivier]

From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

This test was initially designed to test for a known bug where
performing a sequence of H_CEDE hcalls while suspended would cause a
vcpu to lockup in the host. The fix has been available for some time
now, so to increase coverage of this test remove the no-default flag.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 powerpc/unittests.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index af535b7f8982..1e7494801273 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -64,7 +64,7 @@ file = emulator.elf
 file = tm.elf
 smp = 2,threads=2
 extra_params = -machine cap-htm=on -append "h_cede_tm"
-groups = nodefault,h_cede_tm
+groups = h_cede_tm
 
 [sprs]
 file = sprs.elf
-- 
2.20.1

Re: [kvm-unit-tests PULL 0/2] Ppc next patches

[Laurent Vivier]

Radim, Paolo,

is there something that prevents this PR to be merged?

Thanks,
Laurent

On 17/05/2019 15:03, Laurent Vivier wrote:
> The following changes since commit 14bc602f3479d3f5c5e11034daa1070c61bd5640:
> 
>   Merge tag 'pull-request-2019-04-19' of https://gitlab.com/huth/kvm-unit-tests (2019-05-03 10:53:30 -0600)
> 
> are available in the Git repository at:
> 
>   https://github.com/vivier/kvm-unit-tests.git tags/ppc-next-pull-request
> 
> for you to fetch changes up to aa3a3a9e6654fce23a78141c8cfadcd6ba871af1:
> 
>   powerpc: Make h_cede_tm test run by default (2019-05-17 13:50:47 +0200)
> 
> ----------------------------------------------------------------
> Fix h_cede_tm timeout
> 
> ----------------------------------------------------------------
> 
> Suraj Jitindar Singh (2):
>   powerpc: Allow for a custom decr value to be specified to load on decr
>     excp
>   powerpc: Make h_cede_tm test run by default
> 
>  lib/powerpc/handlers.c | 7 ++++---
>  powerpc/sprs.c         | 3 ++-
>  powerpc/tm.c           | 4 +++-
>  powerpc/unittests.cfg  | 2 +-
>  4 files changed, 10 insertions(+), 6 deletions(-)
> 

Re: [kvm-unit-tests PULL 0/2] Ppc next patches

[Paolo Bonzini]

On 03/06/19 10:24, Laurent Vivier wrote:
> Radim, Paolo,
> 
> is there something that prevents this PR to be merged?

FWIW I was just waiting for the merge window waters to settle down, so
that I could go through all the pending patches for both KVM and
kvm-unit-tests.

Paolo

> Thanks,
> Laurent
> 
> On 17/05/2019 15:03, Laurent Vivier wrote:
>> The following changes since commit 14bc602f3479d3f5c5e11034daa1070c61bd5640:
>>
>>   Merge tag 'pull-request-2019-04-19' of https://gitlab.com/huth/kvm-unit-tests (2019-05-03 10:53:30 -0600)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/vivier/kvm-unit-tests.git tags/ppc-next-pull-request
>>
>> for you to fetch changes up to aa3a3a9e6654fce23a78141c8cfadcd6ba871af1:
>>
>>   powerpc: Make h_cede_tm test run by default (2019-05-17 13:50:47 +0200)
>>
>> ----------------------------------------------------------------
>> Fix h_cede_tm timeout
>>
>> ----------------------------------------------------------------
>>
>> Suraj Jitindar Singh (2):
>>   powerpc: Allow for a custom decr value to be specified to load on decr
>>     excp
>>   powerpc: Make h_cede_tm test run by default
>>
>>  lib/powerpc/handlers.c | 7 ++++---
>>  powerpc/sprs.c         | 3 ++-
>>  powerpc/tm.c           | 4 +++-
>>  powerpc/unittests.cfg  | 2 +-
>>  4 files changed, 10 insertions(+), 6 deletions(-)
>>
> 

Re: [kvm-unit-tests PULL 0/2] Ppc next patches

[Paolo Bonzini]

On 17/05/19 15:03, Laurent Vivier wrote:
>   https://github.com/vivier/kvm-unit-tests.git tags/ppc-next-pull-request

Pulled, thanks.

Paolo