On Mon, 19 Jul 2021 14:32:10 +0100, Quentin Perret <qperret@xxxxxxxxxx> wrote: > > On Monday 19 Jul 2021 at 13:14:48 (+0100), Marc Zyngier wrote: > > On Mon, 19 Jul 2021 11:47:24 +0100, > > Quentin Perret <qperret@xxxxxxxxxx> wrote: > > > > > > The stage-2 map walkers currently return -EAGAIN when re-creating > > > identical mappings or only changing access permissions. This allows to > > > optimize mapping pages for concurrent (v)CPUs faulting on the same > > > page. > > > > > > While this works as expected when touching one page-table leaf at a > > > time, this can lead to difficult situations when mapping larger ranges. > > > Indeed, a large map operation can fail in the middle if an existing > > > mapping is found in the range, even if it has compatible attributes, > > > hence leaving only half of the range mapped. > > > > I'm curious of when this can happen. We normally map a single leaf at > > a time, and we don't have a way to map multiple leaves at once: we > > either use the VMA base size or try to upgrade it to a THP, but the > > result is always a single leaf entry. What changed? > > Nothing _yet_ :-) > > The 'share' hypercall introduced near the end of the series allows to > share multiple physically contiguous pages in one go -- this is mostly > to allow sharing data-structures that are larger than a page. > > So if one of the pages happens to be already mapped by the time the > hypercall is issued, mapping the range with the right SW bits becomes > difficult as kvm_pgtable_stage2_map() will fail halfway through, which > is tricky to handle. > > This patch shouldn't change anything for existing users that only map > things that are nicely aligned at block/page granularity, but should > make the life of new users easier, so that seemed like a win. Right, but this is on a different path, right? Guests can never fault multiple mappings at once, and it takes you a host hypercall to perform this "multiple leaves at once". Is there any way we can restrict this to the hypercall? Or even better, keep the hypercall as a "one page at a time" thing? I can't imagine it being performance critical (it is a once-off, and only used over a rather small region of memory). Then, the called doesn't have to worry about the page already being mapped or not. This would also match the behaviour of what I do on the MMIO side. Or do you anticipate much gain from this being able to use block mappings? > > > > To avoid having to deal with such failures in the caller, don't > > > interrupt the map operation when hitting existing PTEs, but make sure to > > > still return -EAGAIN so that user_mem_abort() can mark the page dirty > > > when needed. > > > > I don't follow you here: if you return -EAGAIN for a writable mapping, > > we don't account for the page to be dirty on the assumption that > > nothing has been mapped. But if there is a way to map more than a > > single entry and to get -EAGAIN at the same time, then we're bound to > > lose data on page eviction. > > > > Can you shed some light on this? > > Sure. For guests, hitting the -EAGAIN case means we've lost the race > with another vCPU that faulted the same page. In this case the other > vCPU either mapped the page RO, which means that our vCPU will then get > a permission fault next time we run it which will lead to the page being > marked dirty, or the other vCPU mapped the page RW in which case it > already marked the page dirty for us and we can safely re-enter the > guest without doing anything else. > > So what I meant by "still return -EAGAIN so that user_mem_abort() can > mark the page dirty when needed" is "make sure to mark the page dirty > only when necessary: if winning the race and marking the page RW, or > in the permission fault path". That is, by keeping the -EAGAIN I want to > make sure we don't mark the page dirty twice. (This might fine, but this > would be new behaviour, and it was not clear that would scale well to > many vCPUs faulting the same page). > > I see how this wording can be highly confusing though, I'll and re-word > for the next version. I indeed found it pretty confusing. A reword would be much appreciated. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
