On Thu, 13 Sep 2018, Brijesh Singh wrote: > On 09/13/2018 11:22 AM, Thomas Gleixner wrote: > > I really have to ask why this whole change to sme_encrypt_kernel() is > > required at all. > > The main reason behind making the changes in sme_encrypt_kernel() was > to perverse the initialized values (if any). Since we are adding > __decrypted attribute hence I thought it would make sense to support > the usecase where in future someone may use this attribute on initialized > variables. > > Now the question is, do we really need this to fix the regression? the > answer is NO. Since there is only one user of this new attribute and > lucky it does not initialize the variables hence it is safe to move the > variable in .bss..decrypted section and memset(0). And even if it would it usually trivial enought to make it run time initialized. > > 1) Add a .bss..decrypted section which is PMD aligned and mark the > > kvmclock hv_clock_boot and wallclock struct with __bss_decrypted. > > > 2) Fixup the .bss..decrypted section PMD in the SEV case at the end of > > sme_encrypt_kernel() and do a memset(0) on that. That does not require > > any of the restructuring, really. > > > > Somewhere during the discussion, I was asked to make sure that > __decrypted attribute can be used by others in future and don't tie it > with just the kvmclock. Well, .bss..decrypted can be used by other things and we really only want to do the .data..decrypted thing when there is a fundamental reason to do so. > To fix the regression we don't need to have this complexity. I am okay > to implement your proposal. I would like to fix this regression sooner > than later ;) > > > > 3) Have a function which is called after the page allocator is up which > > does: > > > > I am glad you are pointing this one. In my initial patch I was doing a > kmalloc() of page-size, during review we did discussed to do allocation > once using num_possible_cpus()[like what you have proposed]. But then > discussion moved towards saving memory and that when the secondary array > concept came into the picture. Since .data..decrypted has some unused memory > hence we were getting creative in reusing it. Sorry I didn't pay attention to that, was busy with other urgent stuff. > > That should be a halfways slim sized and non instrusive changeset. > > > > I am okay to implement and test your recommendation, if anyone > disagree then please let me know. thanks It results in simpler code and if the whole thing ends up allocating a few KB too much due to the page allocations then so be it. It's not going to waste 256K in one go. Thanks, tglx
