linux-kernel - Re: [PATCH v10 047/108] KVM: x86/tdp_mmu: Don't zap private pages for unsupported cases

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: <656755672fbbfa9c0085827a6eeeb36ab27c67a5.camel@intel.com>
Date:   Thu, 15 Dec 2022 23:27:58 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "isaku.yamahata@...il.com" <isaku.yamahata@...il.com>
CC:     "sean.j.christopherson@...el.com" <sean.j.christopherson@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "Shahar, Sagi" <sagis@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Aktas, Erdem" <erdemaktas@...gle.com>,
        "dmatlack@...gle.com" <dmatlack@...gle.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>
Subject: Re: [PATCH v10 047/108] KVM: x86/tdp_mmu: Don't zap private pages for
 unsupported cases

On Thu, 2022-12-15 at 14:46 -0800, Isaku Yamahata wrote:
> > Sorry why "private-shared conversion is done via KVM_MEMORY_ENCRYPT_REG"
> > results
> > in "we can ignore the requres from restrictedmem"?
> > 
> > If we punch a hole, the pages are de-allocated, correct?
> 
> With v10 UPM, we can have zap_private = true always.
> 
> With v9 UPM, the callback is triggered both for allocation and punch-hole
> without
> any further argument.  With v10 UPM, the callback is triggered only for
> punching
> hole.  

I honestly don't quite understand "why v9 UPM the callback is triggered for
allocation" (assuming the "callback" here you mean the invaidate notifier). 
Removing it seems to be right.

But, this is not the point.  The point is you need to be clear about how to use
"punch hole" and/or set_memory_attributes().

Now my assumption is "punch hole" can be done alone w/o set_memory_attributes().
For instance, perhaps a (I guess valid) case is hot-removal of memory from TDX
guest, where you don't need to set_memory_attributes().

However it does seem that set_memory_attributes(shared) should (or must) be done
after "punch hole".  And we need to document that clearly.

So it seems a more reasonable way is:

1) in "punch hole", you zap everything.
2) in set_memory_attribute() from private -> shared, you try to zap all private
pages anyway (it should be very quick if "punch hole" has been done), and you
can pr_warn_once() (or WARN()) if you found any private page still exists.

Did I miss anything?

But I am not sure about set_memory_attribute() from shared -> private.  Should
we do fallocate() before that?