linux-kernel - Re: [PATCH RESEND v2 03/17] KVM: X86: Don't track dirty for KVM_SET_[TSS_ADDR|IDENTITY_MAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191223172737.GA81196@xz-x1>
Date:   Mon, 23 Dec 2019 12:27:37 -0500
From:   Peter Xu <peterx@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        Christophe de Dinechin <dinechin@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>
Subject: Re: [PATCH RESEND v2 03/17] KVM: X86: Don't track dirty for
 KVM_SET_[TSS_ADDR|IDENTITY_MAP_ADDR]

On Sat, Dec 21, 2019 at 02:51:52PM +0100, Paolo Bonzini wrote:
> On 21/12/19 02:49, Peter Xu wrote:
> > Originally, we have three code paths that can dirty a page without
> > vcpu context for X86:
> > 
> >   - init_rmode_identity_map
> >   - init_rmode_tss
> >   - kvmgt_rw_gpa
> > 
> > init_rmode_identity_map and init_rmode_tss will be setup on
> > destination VM no matter what (and the guest cannot even see them), so
> > it does not make sense to track them at all.
> > 
> > To do this, a new parameter is added to kvm_[write|clear]_guest_page()
> > to show whether we would like to track dirty bits for the operations.
> > With that, pass in "false" to this new parameter for any guest memory
> > write of the ioctls (KVM_SET_TSS_ADDR, KVM_SET_IDENTITY_MAP_ADDR).
> 
> We can also return the hva from x86_set_memory_region and
> __x86_set_memory_region.

Yes.  Though it is a bit tricky in that then we'll also need to make
sure to take slots_lock or srcu to protect that hva (say, we must drop
that hva reference before we release the locks, otherwise the hva
could gone under us, iiuc).  So if we want to do that we'd better
comment on that hva value very explicitly, just in case some future
callers of __x86_set_memory_region could cache it somewhere.

(Side topic: I feel like the srcu_read_lock() pair in
 init_rmode_identity_map() is redundant..)

-- 
Peter Xu