On 1/25/19 4:14 PM, Andre Przywara wrote:
> On Fri, 25 Jan 2019 17:05:45 +0100
> Andrew Jones <drjones@xxxxxxxxxx> wrote:
>
> Hi,
>
>> On Fri, Jan 25, 2019 at 04:31:37PM +0100, Andrew Jones wrote:
>>> On Fri, Jan 25, 2019 at 02:56:30PM +0000, Alexandru Elisei wrote:
>>>> On 1/24/19 1:35 PM, Andrew Jones wrote:
>>>>> On Thu, Jan 24, 2019 at 02:00:20PM +0100, Andrew Jones wrote:
>>>>>> [..]
>>>>>> chr_testdev_init() ensures vcon is NULL if it fails to
>>>>>> initialize. chr_testdev_exit() immediately returns if vcon is
>>>>>> NULL. This was done by design to allow fallback exits to be
>>>>>> placed below the chr_testdev_exit call, e.g. halt().
>>>>>>
>>>>>> We should be able to drop patch 3/7 and change exit() to this
>>>>>>
>>>>>> void exit(int code)
>>>>>> {
>>>>>> chr_testdev_exit(code);
>>>>>> psci_system_off();
>>>>>> halt(code);
>>>>>> __builtin_unreachable();
>>>>>> }
>>>>>>
>>>>> There's also a framework for exits that can't return status
>>>>> codes. powerpc uses it. Before exiting with psci_system_off we
>>>>> need to make this print statement
>>>>>
>>>>> printf("\nEXIT: STATUS=%d\n", ((code) << 1) | 1);
>>>>>
>>>>> And run_qemu in arm/run needs to be changed to run_qemu_status.
>>>>> It's hacky, but maybe we can live with it until kvmtool offers
>>>>> some sort of debug exit.
>>>>>
>>>>> Thanks,
>>>>> drew
>>>> I can make the change, but if I understand the
>>>> scripts/arch-run.bash code correctly, run_qemu() will check if
>>>> QEMU was terminated because of a signal and adjust the return
>>>> code to take that into account. Using run_qemu_status() means
>>>> that check won't be made when the tests are run under QEMU, is
>>>> that acceptable?
>>> run_qemu_status() first calls run_qemu(). If the return value from
>>> run_qemu() is 1, then it'll change the value to whatever the EXIT:
>>> status line says it should be. If run_qemu() detected that a signal
>>> caused the exit, then the return value won't be 1. In that case
>>> run_qemu_status() will simply propagate the value to the caller.
>>>
>>> It might be worth doing a few tests to ensure that all works out as
>>> designed, but I'm pretty sure it should.
>>>
>> It just occurred to me that you must not be using the run scripts
>> anyway, since they would require further patches to work. In that
>> case, there's no need to change arm/run unless you also provide those
>> additional patches.
>>
>> BTW, I wouldn't be opposed to a second run script, rather than trying
>> to make one script work for both qemu and kvmtool. Ideally both
>> scripts would be driven by the same higher level scripts using the
>> same unittests.cfg file though. The unittests.cfg extra_params field
>> will make that a bit challenging, but otherwise I think adding a few
>> new helper functions to scripts/arch-run.bash may be all that's
>> necessary.
> Yeah, I had some patches along those lines: split test parameters
> from QEMU parameters, abstract common stuff like number of cores and
> amount of memory, mark tests as QEMU only and so on. And I had a
> separate run script for kvmtool, IIRC.
>
> If there is interest I can try to post them, but I would consider
> this an additional effort on top of this series.
>
> Cheers,
> Andre.
I don't use the kvm-unit-tests run script, that is correct. I would prefer that
I don't change arm/run and keep the function exit() like you originally suggested:
void exit(int code)
{
chr_testdev_exit(code);
psci_system_off();
halt(code);
__builtin_unreachable();
}
If anyone is interested, I or Andre can post patches for the run scripts as a separate set. Is that alright with you, Andrew?
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
