lkml.org 
[lkml]   [2024]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v19 027/130] KVM: TDX: Define TDX architectural definitions
    From

    > +/*
    > + * TDX SEAMCALL API function leaves
    > + */
    > +#define TDH_VP_ENTER 0
    > +#define TDH_MNG_ADDCX 1
    > +#define TDH_MEM_PAGE_ADD 2
    > +#define TDH_MEM_SEPT_ADD 3
    > +#define TDH_VP_ADDCX 4
    > +#define TDH_MEM_PAGE_RELOCATE 5

    I don't think the "RELOCATE" is needed in this patchset?

    > +#define TDH_MEM_PAGE_AUG 6
    > +#define TDH_MEM_RANGE_BLOCK 7
    > +#define TDH_MNG_KEY_CONFIG 8
    > +#define TDH_MNG_CREATE 9
    > +#define TDH_VP_CREATE 10
    > +#define TDH_MNG_RD 11
    > +#define TDH_MR_EXTEND 16
    > +#define TDH_MR_FINALIZE 17
    > +#define TDH_VP_FLUSH 18
    > +#define TDH_MNG_VPFLUSHDONE 19
    > +#define TDH_MNG_KEY_FREEID 20
    > +#define TDH_MNG_INIT 21
    > +#define TDH_VP_INIT 22
    > +#define TDH_MEM_SEPT_RD 25
    > +#define TDH_VP_RD 26
    > +#define TDH_MNG_KEY_RECLAIMID 27
    > +#define TDH_PHYMEM_PAGE_RECLAIM 28
    > +#define TDH_MEM_PAGE_REMOVE 29
    > +#define TDH_MEM_SEPT_REMOVE 30
    > +#define TDH_SYS_RD 34
    > +#define TDH_MEM_TRACK 38
    > +#define TDH_MEM_RANGE_UNBLOCK 39
    > +#define TDH_PHYMEM_CACHE_WB 40
    > +#define TDH_PHYMEM_PAGE_WBINVD 41
    > +#define TDH_VP_WR 43
    > +#define TDH_SYS_LP_SHUTDOWN 44

    And LP_SHUTDOWN is certainly not needed.

    Could you check whether there are others that are not needed?

    Perhaps we should just include macros that got used, but anyway.

    [...]

    > +
    > +/*
    > + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B.
    > + */

    Why is this comment applied to TDX_MAX_VCPUS?

    > +#define TDX_MAX_VCPUS (~(u16)0)

    And is (~(16)0) an architectural value defined by TDX spec, or just SW
    value that you just put here for convenience?

    I mean, is it possible that different version of TDX module have
    different implementation of MAX_CPU, e.g., module 1.0 only supports X
    but module 1.5 increases to Y where Y > X?

    Anyway, looks you can safely move this to the patch to enable CAP_MAX_CPU?

    > +
    > +struct td_params {
    > + u64 attributes;
    > + u64 xfam;
    > + u16 max_vcpus;
    > + u8 reserved0[6];
    > +
    > + u64 eptp_controls;
    > + u64 exec_controls;
    > + u16 tsc_frequency;
    > + u8 reserved1[38];
    > +
    > + u64 mrconfigid[6];
    > + u64 mrowner[6];
    > + u64 mrownerconfig[6];
    > + u64 reserved2[4];
    > +
    > + union {
    > + DECLARE_FLEX_ARRAY(struct tdx_cpuid_value, cpuid_values);
    > + u8 reserved3[768];

    I am not sure you need the 'reseved3[768]', unless you need to make
    sieof(struct td_params) return 1024?

    > + };
    > +} __packed __aligned(1024); > +

    [...]

    > +
    > +#define TDX_MD_ELEMENT_SIZE_8BITS 0
    > +#define TDX_MD_ELEMENT_SIZE_16BITS 1
    > +#define TDX_MD_ELEMENT_SIZE_32BITS 2
    > +#define TDX_MD_ELEMENT_SIZE_64BITS 3
    > +
    > +union tdx_md_field_id {
    > + struct {
    > + u64 field : 24;
    > + u64 reserved0 : 8;
    > + u64 element_size_code : 2;
    > + u64 last_element_in_field : 4;
    > + u64 reserved1 : 3;
    > + u64 inc_size : 1;
    > + u64 write_mask_valid : 1;
    > + u64 context : 3;
    > + u64 reserved2 : 1;
    > + u64 class : 6;
    > + u64 reserved3 : 1;
    > + u64 non_arch : 1;
    > + };
    > + u64 raw;
    > +};

    Could you clarify why we need such detailed definition? For metadata
    element size you can use simple '&' and '<<' to get the result.

    > +
    > +#define TDX_MD_ELEMENT_SIZE_CODE(_field_id) \
    > + ({ union tdx_md_field_id _fid = { .raw = (_field_id)}; \
    > + _fid.element_size_code; })
    > +
    > +#endif /* __KVM_X86_TDX_ARCH_H */

    \
     
     \ /
      Last update: 2024-05-27 15:58    [W:3.801 / U:1.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site