lkml.org 
[lkml]   [2022]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [Patch v3 07/14] x86/hyperv: Change vTOM handling to use standard coco mechanisms
    Date
    From: Dave Hansen <dave.hansen@intel.com> Sent: Tuesday, November 22, 2022 10:30 AM
    >
    > On 11/22/22 10:22, Michael Kelley (LINUX) wrote:
    > > Anyway, that's where I think this should go. Does it make sense?
    > > Other thoughts?
    >
    > I think hard-coding the C-bit behavior and/or position to a vendor was
    > probably a bad idea. Even the comment:
    >
    > u64 cc_mkenc(u64 val)
    > {
    > /*
    > * Both AMD and Intel use a bit in the page table to indicate
    > * encryption status of the page.
    > *
    > * - for AMD, bit *set* means the page is encrypted
    > * - for Intel *clear* means encrypted.
    > */
    >
    > doesn't make a lot of sense now. Maybe we should just have a:
    >
    > CC_ATTR_ENC_SET
    >
    > which gets set for the "AMD" behavior and is clear for the "Intel"
    > behavior. Hyper-V code running on AMD can set the attribute to get teh
    > "Intel" behavior.
    >
    > That sure beats having a Hyper-V vendor.

    To me, figuring out the encryption bit polarity and position isn't
    the hard part to set up. We've got three technologies: TDX,
    AMD "C-bit", and arguably, AMD vTOM. Future silicon and architectural
    enhancements will most likely be variations and improvements on
    these rather than something completely new (not that I'm necessarily
    aware of what the processor vendors may have planned). The AMD
    "C-bit" technology already has sub-cases (SME, SEV, SEV-ES, SEV-SNP)
    because of the architectural history. Any of the technologies may
    get additional subcases over time. Whether those subcases are
    represented as new CC_VENDOR_* options, or CC_ATTR_* options
    on an existing CC_VENDOR_* should be driven by the level of
    commonality with what already exists. There's no hard-and-fast
    rule. Just do whatever makes the most sense.

    I'm proposing AMD vTOM as a separate CC_VENDOR_AMD_VTOM
    because it is rather different from CC_VENDOR_AMD, and not just
    in the polarity and position of the encryption bit. But if we really
    wanted to just make it a sub-case of CC_VENDOR_AMD, I could
    probably be convinced. The key is cleanly handling all the other
    attributes like CC_ATTR_GUEST_STATE_ENCRYPT,
    CC_ATTR_ACCESS_IOAPIC_ENCRYPTED (that I add in this patch set),
    CC_ATTR_GUEST_UNROLL_STRING_IO, etc. where we want to avoid
    too many hacky special cases. The current code structure where the
    CC_VENDOR_* selection determines the CC_ATTR_* values seems
    to work OK.

    Anyway, that's my thinking. CC_VENDOR_HYPERV goes away, but
    the behavior is still essentially the same when replaced by
    CC_VENDOR_AMD_VTOM.

    Michael
    \
     
     \ /
      Last update: 2022-11-22 23:04    [W:4.284 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site