> +
> +#define TDX_MAX_NR_CPUID_CONFIGS \
> + ((sizeof(struct tdsysinfo_struct) - \
> + offsetof(struct tdsysinfo_struct, cpuid_configs)) \
> + / sizeof(struct tdx_cpuid_config))
> +
> +struct tdx_capabilities {
> + u8 tdcs_nr_pages;
> + u8 tdvpx_nr_pages;
> +
> + u64 attrs_fixed0;
> + u64 attrs_fixed1;
> + u64 xfam_fixed0;
> + u64 xfam_fixed1;
> +
> + u32 nr_cpuid_configs;
> + struct tdx_cpuid_config cpuid_configs[TDX_MAX_NR_CPUID_CONFIGS];
> +};
> +
> +/* Capabilities of KVM + the TDX module. */
> +static struct tdx_capabilities tdx_caps;
I think you can introduce this tdx_capabilities in another patch.
As claimed this patch can just focus on initializing the TDX module. Whether
you need this tdx_capabilities or tdx_sysinfo is enough can be done in the patch
when they are really needed. It makes review easier otherwise people won't be
able to tell why tdx_capabilities is needed here.
> +
> +static int __init tdx_module_setup(void)
> +{
> + const struct tdsysinfo_struct *tdsysinfo;
> + int ret = 0;
> +
> + BUILD_BUG_ON(sizeof(*tdsysinfo) != 1024);
> + BUILD_BUG_ON(TDX_MAX_NR_CPUID_CONFIGS != 37);
> +
> + ret = tdx_enable();
> + if (ret) {
> + pr_info("Failed to initialize TDX module.\n");
> + return ret;
> + }
> +
> + tdsysinfo = tdx_get_sysinfo();
> + if (tdsysinfo->num_cpuid_config > TDX_MAX_NR_CPUID_CONFIGS)
> + return -EIO;
> +
> + tdx_caps = (struct tdx_capabilities) {
> + .tdcs_nr_pages = tdsysinfo->tdcs_base_size / PAGE_SIZE,
> + /*
> + * TDVPS = TDVPR(4K page) + TDVPX(multiple 4K pages).
> + * -1 for TDVPR.
> + */
> + .tdvpx_nr_pages = tdsysinfo->tdvps_base_size / PAGE_SIZE - 1,
> + .attrs_fixed0 = tdsysinfo->attributes_fixed0,
> + .attrs_fixed1 = tdsysinfo->attributes_fixed1,
> + .xfam_fixed0 = tdsysinfo->xfam_fixed0,
> + .xfam_fixed1 = tdsysinfo->xfam_fixed1,
> + .nr_cpuid_configs = tdsysinfo->num_cpuid_config,
> + };
> + if (!memcpy(tdx_caps.cpuid_configs, tdsysinfo->cpuid_configs,
> + tdsysinfo->num_cpuid_config *
> + sizeof(struct tdx_cpuid_config)))
> + return -EIO;
> +
> + pr_info("kvm: TDX is supported. x86 phys bits %d\n",
> + boot_cpu_data.x86_phys_bits);
What''s the benefit of print out x86_phys_bits? Looks a little bit weird here.
TDX host code will print out TDX private KeyID range. I think that is useful
enough?
> +
> + return 0;
> +}
> +
> +int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
> +{
> + int r;
> +
> + if (!enable_ept) {
> + pr_warn("Cannot enable TDX with EPT disabled\n");
> + return -EINVAL;
> + }
> +
> + /* MOVDIR64B instruction is needed. */
> + if (!static_cpu_has(X86_FEATURE_MOVDIR64B)) {
> + pr_warn("Cannot enable TDX with MOVDIR64B supported ");
^
without
> + return -ENODEV;
> + }
I think you should explain why MOVDIR64B is required, otherwise this just comes
out of blue.
Btw, is this absolutely required? TDX also supports Li-mode, which doesn't have
integrity check. So theoretically with Li-mode, normal zeroing is also OK but
doesn't need to use MOVDIR64B.
That being said, do we have a way to tell whether TDX works in Ci or Li mode?
