[lkml]   [2018]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v7 0/5] x86: Fix SEV guest regression
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

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

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



 \ /
  Last update: 2018-09-13 20:48    [W:1.498 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site