Re: [xen-unstable bisection] complete test-armhf-armhf-xl-multivcpu

[Julien Grall <julien@xen.org>]

Hi Jan,

On 12/04/2021 09:53, Jan Beulich wrote:
> On 11.04.2021 04:12, osstest service owner wrote:
>> branch xen-unstable
>> xenbranch xen-unstable
>> job test-armhf-armhf-xl-multivcpu
>> testid guest-start/debian.repeat
>>
>> Tree: linux git://xenbits.xen.org/linux-pvops.git
>> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
>> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
>> Tree: xen git://xenbits.xen.org/xen.git
>>
>> *** Found and reproduced problem changeset ***
>>
>>    Bug is in tree:  xen git://xenbits.xen.org/xen.git
>>    Bug introduced:  9617d5f9c19d1d157629e1e436791509526e0ce5
>>    Bug not present: 5c3c410bd2ea8d2cc520e8e8f83ad143c9c5cff7
>>    Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/160931/
>>
>>
>>    commit 9617d5f9c19d1d157629e1e436791509526e0ce5
>>    Author: Julien Grall <jgrall@amazon.com>
>>    Date:   Sat Feb 20 17:54:13 2021 +0000
>>    
>>        xen/arm: mm: flush_page_to_ram() only need to clean to PoC
>>        
>>        At the moment, flush_page_to_ram() is both cleaning and invalidate to
>>        PoC the page.
>>        
>>        The goal of flush_page_to_ram() is to prevent corruption when the guest
>>        has disabled the cache (the cache line may be dirty) and the guest to
>>        read previous content.
>>        
>>        Per this definition, the invalidating the line is not necessary. So
>>        invalidating the cache is unnecessary. In fact, it may be counter-
>>        productive as the line may be (speculatively) accessed a bit after.
>>        So this will incurr an expensive access to the memory.
>>        
>>        More generally, we should avoid interferring too much with cache.
>>        Therefore, flush_page_to_ram() is updated to only clean to PoC the page.
>>        
>>        The performance impact of this change will depend on your
>>        workload/processor.
>>        
>>        Signed-off-by: Julien Grall <jgrall@amazon.com>
>>        Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>        Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Is it possible that other code (guest one in particular considering the
> failure pattern, but possibly not limited to that) has developed a
> dependency on the prior behavior?

This is not a problem in the guest. I overlooked that 
flush_page_to_ram() is also used when emulating the instruction to 
invalidate by set/way the data cache.

We would need to tell flush_page_to_ram() which type of operation we 
want to do. I will not have time to work on it right now, so I have 
reverted the patch to unblock OssTest.

Sorry for the breakage.

Cheers,

-- 
Julien Grall