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

[Julien Grall <julien@xxxxxxx>]

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@xxxxxxxxxx>
> >    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@xxxxxxxxxx>
> >        Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> >        Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 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