On Fri, Aug 27, 2021 at 7:51 AM Tom Lendacky <thomas.lendacky@xxxxxxx> wrote:
>
> On 8/26/21 10:12 PM, Zixuan Wang wrote:
> > +++ b/lib/x86/amd_sev.c
> > @@ -0,0 +1,77 @@
> > +/*
> > + * AMD SEV support in KVM-Unit-Tests
> > + *
> > + * Copyright (c) 2021, Google Inc
> > + *
> > + * Authors:
> > + * Zixuan Wang <zixuanwang@xxxxxxxxxx>
> > + *
> > + * SPDX-License-Identifier: LGPL-2.0-or-later
> > + */
> > +
> > +#include "amd_sev.h"
> > +#include "x86/processor.h"
> > +
> > +static unsigned long long amd_sev_c_bit_pos;
>
> This can be a unsigned short since this is just the bit position, not the
> mask.
>
I agree. I will update it in the next version.
> > +
> > +bool amd_sev_enabled(void)
> > +{
> > + struct cpuid cpuid_out;
> > + static bool sev_enabled;
> > + static bool initialized = false;
> > +
> > + /* Check CPUID and MSR for SEV status and store it for future function calls. */
> > + if (!initialized) {
> > + sev_enabled = false;
> > + initialized = true;
> > +
> > + /* Test if we can query SEV features */
> > + cpuid_out = cpuid(CPUID_FN_LARGEST_EXT_FUNC_NUM);
> > + if (cpuid_out.a < CPUID_FN_ENCRYPT_MEM_CAPAB) {
> > + return sev_enabled;
> > + }
> > +
> > + /* Test if SEV is supported */
> > + cpuid_out = cpuid(CPUID_FN_ENCRYPT_MEM_CAPAB);
> > + if (!(cpuid_out.a & SEV_SUPPORT_MASK)) {
> > + return sev_enabled;
> > + }
> > +
> > + /* Test if SEV is enabled */
> > + if (!(rdmsr(MSR_SEV_STATUS) & SEV_ENABLED_MASK)) {
> > + return sev_enabled;
> > + }
> > +
> > + sev_enabled = true;
>
> Maybe just make this a bit easier to read by doing:
>
> if (rdmsr(MSR_SEV_STATUS & SEV_ENABLED_MASK)
> sev_enabled = true;
>
> No need to return early since you are at the end of the if statement. Just
> my opinion, though, not a big deal.
>
> Thanks,
> Tom
>
I agree, I will update it in the next version. Thank you for the comments!
Best regards,
Zixuan
