Re: [PATCH] x86/CPU/AMD, mm: Extend with mem_encrypt=sme option

[Tom Lendacky]

On 10/1/2017 12:16 PM, Borislav Petkov wrote:
> On Sun, Oct 01, 2017 at 12:00:31PM -0500, Brijesh Singh wrote:
> > When SEV feature is disabled, KVM will not be able to launch any SEV
> > guests. When SEV support is available, KVM can enable it in a specific
> > VM by setting SEV bit before executing the VMRUN instruction.
>
> So I want to be able to disable SEV and the whole code that comes with
> it in the *host*.

The ability to launch an SEV guest from the host is controlled by the SEV
feature bit.  When that is cleared (which happens in cpu/amd.c), then the
host will not be able to launch any SEV guests.

The confusion may come from the variable name.  The name sev_enabled
should probably be changed to sev_active since this variable is used as
follows:

  sme_me_mask == 0 			=> Neither SME nor SEV is active
  sme_me_mask != 0 && !sev_enabled	=> SME is active (host mode)
  sme_me_mask != 0 && sev_enabled	=> SEV is active (guest mode)

> > Guest OS:
> > --------
> > Checks the MSR_AMD64_SEV to determine if SEV feature is enabled. Please
> > note that the MSR is a read-only. IOW, MSR is not intercepted by the
> > hypervisor.
> >
> > Currently, mem_encrypt=xxx and CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT
> > is don't care. We can not depend on the command line because when SEV is
> > enabled in a VM then instruction fetch will be decrypted by the
> > hardware. If we want then we can perform the comparison between the SEV
> > state obtained through MSR with user supplied command line and trigger
> > BUG() if they don't match.
>
> And when we have supplied mem_encrypt=sme to the *host* cmdline, it
> should be impossible to start SEV guests. IOW, that feature mask test
> should not happen and I should do instead:

I think we're talking about the same thing.  You want sev_enabled to
indicate whether you can launch an SEV guest.  We would still need an
sev_active variable to distinguish between SME and SEV during kernel
execution when the sme_me_mask is non-zero.  Currently, the SEV feature
bit acts as "sev_enabled" and the sev_enabled variable acts as
"sev_active" in this scenario.

Thanks,
Tom

>          } else if (!strncmp(buffer, cmd_sme, sizeof(buffer))) {
>                  sme_only = true;
> 		sev_enabled = false;
>          }
>
> Or, respectively, not set it here as it is false already but set it at
> the end of the function like this:
>
>          if (sme_only)
>                  return;
>
> 	sev_enabled = true;
> }
>
> Hmmm?