[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <175eedc5-d82a-4b3a-bce6-2caf625597ca@suse.com>
Date: Mon, 9 Jun 2025 11:02:49 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: Chao Gao <chao.gao@...el.com>
Cc: linux-coco@...ts.linux.dev, x86@...nel.org, kvm@...r.kernel.org,
seanjc@...gle.com, pbonzini@...hat.com, eddie.dong@...el.com,
kirill.shutemov@...el.com, dave.hansen@...el.com, dan.j.williams@...el.com,
kai.huang@...el.com, isaku.yamahata@...el.com, elena.reshetova@...el.com,
rick.p.edgecombe@...el.com, Farrah Chen <farrah.chen@...el.com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 03/20] x86/virt/seamldr: Introduce a wrapper for
P-SEAMLDR SEAMCALLs
On 6/9/25 10:53, Chao Gao wrote:
>>> +config INTEL_TDX_MODULE_UPDATE
>>> + bool "Intel TDX module runtime update"
>>> + depends on INTEL_TDX_HOST
>>> + help
>>> + This enables the kernel to support TDX module runtime update. This allows
>>> + the admin to upgrade the TDX module to a newer one without the need to
>>> + terminate running TDX guests.
>>> +
>>> + If unsure, say N.
>>> +
>>
>> WHy should this be conditional?
>>
>
> Good question. I don't have a strong reason, but here are my considerations:
>
> 1. Runtime updates aren't strictly necessary for TDX functionalities. Users can
> update the TDX module via BIOS updates and reboot if service downtime isn't
> a concern.
>
> 2. Selecting TDX module updates requires selecting FW_UPLOAD and FW_LOADER,
> which I think will significantly increase the kernel size if FW_UPLOAD/LOADER
> won't otherwise be selected.
If size is a consideration (but given the size of machines that are
likely to run CoCo guests I'd say it's not) then don't make this a
user-configurable option but rather make it depend on TDX being selected
and FW_UPLOAD/FW_LOADER being selected.
I'd rather keep the user visible options to a minimum, especially
something such as this update functionality.
But in any case I'd like to hear other opinions as well.
>
> It may or may not be wise to assume that most TDX users will enable TDX module
> updates. so, I'm taking a conservative approach by making it optional. The
> resulting code isn't that complex, as CONFIG_INTEL_TDX_MODULE_UPDATE
> appears in only two places:
>
> 1. in the Makefile:
>
> obj-y += seamcall.o tdx.o
> obj-$(CONFIG_INTEL_TDX_MODULE_UPDATE) += seamldr.o
>
> 2. in the seamldr.h:
>
> #ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
> extern struct attribute_group seamldr_group;
> #define SEAMLDR_GROUP (&seamldr_group)
> int get_seamldr_info(void);
> void seamldr_init(struct device *dev);
> #else
> #define SEAMLDR_GROUP NULL
> static inline int get_seamldr_info(void) { return 0; }
> static inline void seamldr_init(struct device *dev) { }
> #endif
>
> That said, I'm open to keeping or dropping the Kconfig option.
Powered by blists - more mailing lists