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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16/05/2019 01.27, Suraj Jitindar Singh wrote:
> On Wed, 2019-05-15 at 18:22 +0200, Laurent Vivier wrote:
>> On 15/05/2019 02:28, Suraj Jitindar Singh wrote:
>>> 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@xxxxxxxxx>
>>>
>>> ---
>>>
>>> V1 -> V2:
>>> - Make decr variables static
>>> - Load intial decr value in tm test to ensure known value present
>>> ---
>>>  lib/powerpc/handlers.c | 7 ++++---
>>>  powerpc/sprs.c         | 5 +++--
>>>  powerpc/tm.c           | 4 +++-
>>>  3 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
>>> index be8226a..c8721e0 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 6744bd8..0e2e1c9 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,8 +289,8 @@ int main(int argc, char **argv)
>>>  		(void) getchar();
>>>  	} else {
>>>  		puts("Sleeping...\n");
>>> -		handle_exception(0x900, &dec_except_handler,
>>> NULL);
>>> -		asm volatile ("mtdec %0" : : "r" (0x3FFFFFFF));
>>> +		handle_exception(0x900, &dec_except_handler,
>>> &decr);
>>> +		asm volatile ("mtdec %0" : : "r" (decr));
>>
>> why do you replace the 0x3FFFFFFF by decr which is 0x7FFFFFFF?
> 
> Oh yeah, my mistake. I mis-read and thought they were the same. Is
> there any reason it was 0x3FFFFFFF? Can this be fixed up when applying
> or should I resend?

Should be fine to fix this when the patch gets picked up. With the value
fixed:

Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux