Messages in this thread | | | Date | Wed, 6 Sep 2023 16:29:56 +0000 | From | Oliver Upton <> | Subject | Re: [PATCH] KVM: arm64: Fix soft-lockup on relaxing PTE permission |
| |
Gavin,
On Wed, Sep 06, 2023 at 08:26:24AM +1000, Gavin Shan wrote:
[...]
> It seems I didn't make it clear enough. The reason why I had the concern > to avoid reading ctr_el0 is we read ctr_el0 for twice in the following path, > but I doubt if anybody cares. Since it's a hot path, each bit of performance > gain will count. > > invalidate_icache_guest_page > __invalidate_icache_guest_page // first read on ctr_el0, with your code changes > icache_inval_pou(va, va + size) > invalidate_icache_by_line > icache_line_size // second read on ctr_el0
That can be addressed by shoving the check deep into invalidate_icache_by_line, which would benefit _all_ use cases of I-cache invalidation by VA. I haven't completely made up my mind about that, though, because of the consequences of a global invalidation.
> > > @size is guranteed to be PAGE_SIZE or PMD_SIZE aligned. Maybe > > > we can just aggressively do something like below, disregarding the icache thrashing. > > > In this way, the code is further simplified. > > > > > > if (size > PAGE_SIZE) { > > > icache_inval_all_pou(); > > > } else { > > > icache_inval_pou((unsigned long)va, > > > (unsigned long)va + size); > > > } // parantheses is still needed > > > > This could work too but we already have a kernel heuristic for limiting > > the amount of broadcast invalidations, which is MAX_TLBI_OPS. I don't > > want to introduce a second, KVM-specific hack to address the exact same > > thing. > > > > Ok. I was confused at the first glance since TLB isn't relevant to icache. > I think it's fine to reuse MAX_TLBI_OPS here, but a comment may be needed. > Oliver, could you please send a formal patch for your changes?
Yeah, I think I may have said it before, but this thing needs to be called 'MAX_DVM_OPS'. I-cache invalidations and TLB invalidations become DVMOps (Distributed Virtual Memory) in terms of CHI, which pile up at the miscellaneous node in the mesh.
Give me a day or two to convince myself of the right way to go about this and I'll send out what I have.
-- Thanks, Oliver
| |