RE: [Patch v4 06/13] x86/hyperv: Change vTOM handling to use standard coco mechanisms

Michael Kelley (LINUX)
2023-01-09T17:37:12Z

From: Borislav Petkov <bp@alien8.de> Sent: Monday, January 9, 2023 8:39 AM
>
[snip]

> > diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> > index 49b44f8..c361c52 100644
> > --- a/arch/x86/coco/core.c
> > +++ b/arch/x86/coco/core.c
> > @@ -44,6 +44,24 @@ static bool intel_cc_platform_has(enum cc_attr attr)
> > static bool amd_cc_platform_has(enum cc_attr attr)
> > {
> > #ifdef CONFIG_AMD_MEM_ENCRYPT
> > +
> > + /*
> > + * Handle the SEV-SNP vTOM case where sme_me_mask must be zero,
> > + * and the other levels of SME/SEV functionality, including C-bit
> > + * based SEV-SNP, must not be enabled.
> > + */
> > + if (sev_status & MSR_AMD64_SNP_VTOM_ENABLED) {
>
> return amd_cc_platform_vtom();
>
> or so and then stick that switch in there.
>
> This way it looks kinda grafted in front and with a function call with a telling
> name it says it is a special case...
>
> > + switch (attr) {
> > + case CC_ATTR_GUEST_MEM_ENCRYPT:
> > + case CC_ATTR_MEM_ENCRYPT:
> > + case CC_ATTR_ACCESS_IOAPIC_ENCRYPTED:
> > + return true;
> > + default:
> > + return false;
> > + }
> > + }
>
> The rest looks kinda nice, I gotta say. I can't complain. :)
>

OK. I have no objection to putting that code in a separate "helper"
function. The only slight messiness is that the helper function must
be separately wrapped in #ifdef CONFIG_AMD_MEM_ENCRYPT, or
marked __maybe_unused.

Michael