Hi Andrew, On 2/3/20 6:59 PM, Andrew Jones wrote: > On Fri, Jan 31, 2020 at 04:37:18PM +0000, Alexandru Elisei wrote: >> These are the patches that were left unmerged from the previous version of >> the series, plus a few new patches. Patch #1 "Makefile: Use >> no-stack-protector compiler options" is straightforward and came about >> because of a compile error I experienced on RockPro64. >> >> Patches #3 and #5 are the result of Andre's comments on the previous >> version. When adding ISBs after register writes I noticed in the ARM ARM >> that a read of the timer counter value can be reordered, and patch #4 >> tries to avoid that. >> >> Patch #7 is also the result of a review comment. For the GIC tests, we wait >> up to 5 seconds for the interrupt to be asserted. However, the GIC tests >> can use more than one CPU, which is not the case with the timer test. And >> waiting for the GIC to assert the interrupt can happen up to 6 times (8 >> times after patch #9), so I figured that a timeout of 10 seconds for the >> test is acceptable. >> >> Patch #8 tries to improve the way we test how the timer generates the >> interrupt. If the GIC asserts the timer interrupt, but the device itself is >> not generating it, that's a pretty big problem. >> >> Ran the same tests as before: >> >> - with kvmtool, on an arm64 host kernel: 64 and 32 bit tests, with GICv3 >> (on an Ampere eMAG) and GICv2 (on a AMD Seattle box). >> >> - with qemu, on an arm64 host kernel: >> a. with accel=kvm, 64 and 32 bit tests, with GICv3 (Ampere eMAG) and >> GICv2 (Seattle). >> b. with accel=tcg, 64 and 32 bit tests, on the Ampere eMAG machine. >> >> Changes: >> * Patches #1, #3, #4, #5, #7, #8 are new. >> * For patch #2, as per Drew's suggestion, I changed the entry point to halt >> because the test is supposed to test if CPU_ON is successful. >> * Removed the ISB from patch #6 because that was fixed by #3. >> * Moved the architecture dependent function init_dcache_line_size to >> cpu_init in lib/arm/setup.c as per Drew's suggestion. >> >> Alexandru Elisei (10): >> Makefile: Use no-stack-protector compiler options >> arm/arm64: psci: Don't run C code without stack or vectors >> arm64: timer: Add ISB after register writes >> arm64: timer: Add ISB before reading the counter value >> arm64: timer: Make irq_received volatile >> arm64: timer: EOIR the interrupt after masking the timer >> arm64: timer: Wait for the GIC to sample timer interrupt state >> arm64: timer: Check the timer interrupt state >> arm64: timer: Test behavior when timer disabled or masked >> arm/arm64: Perform dcache clean + invalidate after turning MMU off >> >> Makefile | 4 +- >> lib/arm/asm/processor.h | 13 +++++++ >> lib/arm64/asm/processor.h | 12 ++++++ >> lib/arm/setup.c | 8 ++++ >> arm/cstart.S | 22 +++++++++++ >> arm/cstart64.S | 23 ++++++++++++ >> arm/psci.c | 15 ++++++-- >> arm/timer.c | 79 ++++++++++++++++++++++++++++++++------- >> arm/unittests.cfg | 2 +- >> 9 files changed, 158 insertions(+), 20 deletions(-) >> >> -- >> 2.20.1 >> > The series looks good to me. The first patch probably could have been > posted separately, but I'll try to test the whole series tomorrow. If Noted, next time I will try to do a better job separating the patches. I found the bug while testing the arm64 fixes, and I was getting ready to send the patches, so I just figured I'll send it as part of the series. > all looks well, I'll prepare a pull request for Paolo. Thank you very much for taking the time to review the patches! Much appreciated. Thanks, Alex > > Thanks, > drew >
