lkml.org 
[lkml]   [2021]   [Jul]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 03/14] KVM: arm64: Continue stage-2 map when re-creating mappings
    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@google.com> 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.

    > > 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.

    Cheers,
    Quentin

    \
     
     \ /
      Last update: 2021-07-19 15:32    [W:2.081 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site