Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property
Hi Guys, I looked at issue with P5600 machine under gdb of kernel. arch_check_elf from arch/mips/kernel/elf.c rejects our sysroot binaries with -ENOEXEC code, since our binaries do not have EF_MIPS_NAN2008 ELF header flag set and this CPU model does not have cpu_has_nan_legacy, i.e mips_use_nan_legacy=false. So at least we would need to have to change our user-land ABI compilation flags to cleanly match EF_MIPS_NAN2008 requirements. I am not sure whether it is an option, and how it would impact older CPUs. For experiment sake I added ieee754=relaxed kernel option to override mips_use_nan_legacy setting and system gets some sings of life after that but then it hangs further down the road. I briefly tried to look at this, but it is not clear what is going on. On first look it seems that system is trashing on nested do_page_fault calls. It might be that something missing in our kernel config, or we hitting some kernel bug, or bug in P5600 qemu model. It is hard to tell right now. Is it fair to say that we put enough effort exploring P5600 route and it seems does not work for us without additional substantial work. Is possible to come back to 34Kf route, doing very small localized very well defined change of bumping TLBs number for model that we know works well for us? Since we figured out that 34Kf spec allows 16, 32, or 64 TLBs my first personal preference would be to use Phil's patch series with addressing review comments. And additionally it would be great to set number of 34KF TLB to 64 by default. If anyone out there (IMO unlikely) depends that before model had only 16 TLBs, he/she can use cpu parameters to put it back to 16. My second alternative choice is to accept 34Kf-64tlb model, after I rephrase commit message. Thanks, Victor From: Khem Raj Sent: Wednesday, October 14, 2020 1:53 PM To: Victor Kamensky (kamensky) Cc: Philippe Mathieu-Daudé; Richard Purdie; qemu-devel@nongnu.org; Aleksandar Rikalo; Aleksandar Markovic; Aurelien Jarno; Richard Henderson Subject: Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property On Wed, Oct 14, 2020 at 1:20 PM Victor Kamensky (kamensky) wrote: > > In order just to keep on the same thread, here is piece of information > I found: > > I looked at "MIPS32® 34Kf™ Processor Core Datasheet" [1] > > Page 8 in "Joint TLB (JTLB)" section says: > > "The JTLB is a fully associative TLB cache containing 16, 32, > or 64-dual-entries mapping up to 128 virtual pages to their > corresponding physical addresses." > > So "34Kf-64tlb" cpu model I proposed turns out not to be "fictitious" > after all. Having 64 TLBs is all within this CPU spec. It is not clear > why original 34Kf model choose worst case scenario wrt > TLB numbers. Commit log where 34Kf was introduced does not > have much details. thanks for digging this information from CPU specs. It seems using 64 TLB as default might be a good option for 34Kf then > > So IMO on 34Kf route we have the following choices: > > 1) I can rephrase commit message and resubmit commit for > "34Kf-64tlb" cpu model, if it could be merged > > 2) We can bump up number of TLBs to 64 in existing 34Kf model > since it is within the spec. this looks a good option since it is with in specs and is backward compatible. > > 3) Use Phil's series and tlb-entries cpu parameter would cover all I agree. > 3 variants of 16,32,64 TLBs allowed by 34Kf data sheet spec. > > Please see inline wrt asked '-cpu P5600' testing. Look for 'victor2>' > > [1] > https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00419-2B-34Kf-DTS-01.20.pdf > > ________ > From: Philippe Mathieu-Daudé on behalf of > Philippe Mathieu-Daudé > Sent: Wednesday, October 14, 2020 7:53 AM > To: Richard Purdie; Victor Kamensky (kamensky); qemu-devel@nongnu.org > Cc: Aleksandar Rikalo; Khem Raj; Aleksandar Markovic; Aurelien Jarno; Richard > Henderson > Subject: Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a > CPU property > > On 10/14/20 9:14 AM, Richard Purdie wrote: > > On Wed, 2020-10-14 at 01:36 +, Victor Kamensky (kamensky) wrote: > >> Thank you very much for looking at this. I gave a spin to > >> your 3 patch series in original setup, and as expected with > >> '-cpu 34Kf,tlb-entries=64' option it works great. > >> > >> If nobody objects, and your patches could be merged, we > >> would greatly appreciate it. > > > > Speaking as one of the Yocto Project maintainers, this is really > > helpful for us, thanks! > > > > qemumips is one of our slowest platforms for automated testing so this > > performance improvement helps a lot. > > Could you try Ri
Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property
In order just to keep on the same thread, here is piece of information I found: I looked at "MIPS32® 34Kf™ Processor Core Datasheet" [1] Page 8 in "Joint TLB (JTLB)" section says: "The JTLB is a fully associative TLB cache containing 16, 32, or 64-dual-entries mapping up to 128 virtual pages to their corresponding physical addresses." So "34Kf-64tlb" cpu model I proposed turns out not to be "fictitious" after all. Having 64 TLBs is all within this CPU spec. It is not clear why original 34Kf model choose worst case scenario wrt TLB numbers. Commit log where 34Kf was introduced does not have much details. So IMO on 34Kf route we have the following choices: 1) I can rephrase commit message and resubmit commit for "34Kf-64tlb" cpu model, if it could be merged 2) We can bump up number of TLBs to 64 in existing 34Kf model since it is within the spec. 3) Use Phil's series and tlb-entries cpu parameter would cover all 3 variants of 16,32,64 TLBs allowed by 34Kf data sheet spec. Please see inline wrt asked '-cpu P5600' testing. Look for 'victor2>' [1] https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00419-2B-34Kf-DTS-01.20.pdf From: Philippe Mathieu-Daudé on behalf of Philippe Mathieu-Daudé Sent: Wednesday, October 14, 2020 7:53 AM To: Richard Purdie; Victor Kamensky (kamensky); qemu-devel@nongnu.org Cc: Aleksandar Rikalo; Khem Raj; Aleksandar Markovic; Aurelien Jarno; Richard Henderson Subject: Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property On 10/14/20 9:14 AM, Richard Purdie wrote: > On Wed, 2020-10-14 at 01:36 +, Victor Kamensky (kamensky) wrote: >> Thank you very much for looking at this. I gave a spin to >> your 3 patch series in original setup, and as expected with >> '-cpu 34Kf,tlb-entries=64' option it works great. >> >> If nobody objects, and your patches could be merged, we >> would greatly appreciate it. > > Speaking as one of the Yocto Project maintainers, this is really > helpful for us, thanks! > > qemumips is one of our slowest platforms for automated testing so this > performance improvement helps a lot. Could you try Richard's suggestion? Using '-cpu P5600' instead? It is available in Linux since v5.8. victor2> I've tried exact image that works on 34Kf and 34Kf-64tlb models victor2> image with '-cpu P5600'. it does not boot: it dies in init (systemd). victor2> I can look under gdb with qemu -s -S options, what is going on there victor2> but it will take time. victor2> If someone have some clues what might cause it please let victor2> me know. Here is high level information about setup: victor2>- qemu version is 5.1.0 victor2>- kernel base version is 5.8.9 victor2>- systemd version is 1_246.6 victor2>- user land CPU related build options "-meb -meb -mabi=32 -mhard-float -march=mips32r2 -mllsc -mips32r2" Thanks, Victor > > Cheers, > > Richard > >
Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property
Hi Richard, Please forgive my cumbersome mailing agent at work. Please look inline for 'victor>' From: Richard Henderson Sent: Tuesday, October 13, 2020 7:22 PM To: Philippe Mathieu-Daudé; qemu-devel@nongnu.org; Victor Kamensky (kamensky) Cc: Aleksandar Rikalo; Khem Raj; Aleksandar Markovic; Richard Purdie; Aurelien Jarno; Richard Henderson Subject: Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property On 10/13/20 4:11 PM, Richard Henderson wrote: > On 10/13/20 6:25 AM, Philippe Mathieu-Daudé wrote: >> Yocto developers have expressed interest in running MIPS32 >> CPU with custom number of TLB: >> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg03428.html >> >> Help them by making the number of TLB entries a CPU property, >> keeping our set of CPU definitions in sync with real hardware. > > You mean keeping the 34kf model within qemu in sync, rather than creating a > nonsense model that doesn't exist. victor> Question: do current MIPS "generic" qemu cpu models exist for real victor> out there? I agree my choice was not ideal, but it is not that outlandish victor> and IMO somewhat inline with existence of MIPS generic cpu models. > Question: is this cpu parameter useful for anything else? victor> If you are interested here are my testing numbers of how #TLBs impact victor> user land execution time: victor> https://lists.openembedded.org/g/openembedded-core/message/143115 > Because the ideal solution for a CI loop is to use one of the mips cpu models > that has the hw page table walker (CP0C3_PW). Having qemu being able to > refill > the tlb itself is massively faster. > > We do not currently implement a mips cpu that has the PW. When I downloaded Bah, "mips32 cpu". We do have the P5600 that does has it, though the code is wrapped up in TARGET_MIPS64. I'll also note that the code could be better placed [*] > (1) anyone know if the PW incompatible with mips32? I've since found a copy of the mips32-pra in the wayback machine and have answered this as "no" -- PW is documented for mips32. > (2) if not, was there any mips32 hw built with PW > that we could model? But I still don't know about this. A further question for the Yocto folks: could you make use of a 64-bit kernel in testing a 32-bit userspace? victor> Such test does exist and it is part of CI already, it is dubbed as MIPS multi-lib victor> tests where on top mips64 kernel tests are run for n64, n32, and o32 MIPS ABIs victor> user-land. victor> Note mips32 CI in question does cover kernel functionality for example victor> KLM build and check, SystemTap test, ltp and other kernel operations are tested. victor> I.e it does test 32 bit MIPS kernel as well, but user-land is involved in such victor> tests, and as it was described, it is slow compared to other qemu cases. Thanks, Victor And I guess maybe we should update our recommendations in the docs. Thoughts on this, Phil? r~ [*] Where it is now, it can't be used for gdb (mips_cpu_get_phys_page_debug). When used there, we should not modify cpu state, i.e. actually insert the PTE into the MIPS TLB, but we could still make use of the information available.
Re: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property
Hi Philippe, Thank you very much for looking at this. I gave a spin to your 3 patch series in original setup, and as expected with '-cpu 34Kf,tlb-entries=64' option it works great. If nobody objects, and your patches could be merged, we would greatly appreciate it. Thanks, Victor From: Philippe Mathieu-Daudé on behalf of Philippe Mathieu-Daudé Sent: Tuesday, October 13, 2020 6:25 AM To: qemu-devel@nongnu.org; Victor Kamensky (kamensky) Cc: Khem Raj; Richard Henderson; Aleksandar Rikalo; Aleksandar Markovic; Jiaxun Yang; Aurelien Jarno; Richard Purdie; Philippe Mathieu-Daudé Subject: [RFC PATCH 0/3] target/mips: Make the number of TLB entries a CPU property Yocto developers have expressed interest in running MIPS32 CPU with custom number of TLB: https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg03428.html Help them by making the number of TLB entries a CPU property, keeping our set of CPU definitions in sync with real hardware. Please test/review, Phil. Philippe Mathieu-Daudé (3): target/mips: Make cpu_mips_realize_env() propagate Error target/mips: Store number of TLB entries in CPUMIPSState target/mips: Make the number of TLB entries a CPU property target/mips/cpu.h| 1 + target/mips/internal.h | 10 +- target/mips/cpu.c| 12 ++-- target/mips/translate.c | 16 ++-- target/mips/translate_init.c.inc | 2 +- 5 files changed, 35 insertions(+), 6 deletions(-) -- 2.26.2
