[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_its: downgrade error_report t
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_its: downgrade error_report to warn_report in kvm_arm_its_reset |
Date: |
Fri, 20 Jul 2018 10:01:23 +0100 |
On 20 July 2018 at 02:22, Jia He <address@hidden> wrote:
> Hi Peter。 Thanks for the comments
>
> On 7/19/2018 8:41 PM, Peter Maydell Wrote:
>> On 19 July 2018 at 04:11, Jia He <address@hidden> wrote:
>>> In scripts/arch-run.bash of kvm-unit-tests, it will check the qemu
>>> output log with:
>>> if [ -z "$(echo "$errors" | grep -vi warning)" ]; then
>>>
>>> Thus without the warning prefix, all of the test fail.
>>>
>>> Since it is not unrecoverable error in kvm_arm_its_reset for
>>> current implementation, downgrading the report from error to
>>> warn makes sense.
>>
>> I think the counterargument would be that this should report
>> an error, because you just asked the device to do something
>> that it doesn't support (ie to do a clean reset). Since the
>> device isn't going to behave correctly, the tests should fail,
>> and the way to make them pass is to upgrade to a kernel that
>> implements the device correctly (by implementing the necessary
>> feature).
>>
>> But we could maybe move to warn_report() here -- I'm not
>> sure what our rules are for what counts as an error and
>> what counts as a warning.
> I reviewed the other error_report in qemu. Some of them are followed
> by exit(), but the rest are not. So it seems there is no definite
> rules for error_report.
>
> IMO, the best resolution is to change the output grep search criteria.
> But it is difficult for kvm-unit-tests to identify whether it is an
> error without exit() or a warning. The fastest resolution is moving
> error_report to warn_report.
Well, as I say, this implementation of the ITS *should* fail
tests -- it doesn't behave to spec. The fastest resolution
is for you to upgrade your host kernel to one that has the
bugfix :-)
thanks
-- PMM