[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 01/22] x86/include/asm/intel-txt.h: constants and accessors for TXT registers and heap
On 06.07.2025 17:57, Sergii Dmytruk wrote: > On Wed, Jul 02, 2025 at 04:29:18PM +0200, Jan Beulich wrote: >> Btw, a brief rev log would be nice here. I saw you have something in the >> cover letter, but having to look in two places isn't very helpful. > > I don't really know how to effectively maintain 23 logs at the same time > given that changing one patch has cascading effects on the rest. I'd > suggest using `git diff-range` instead, commands for which I can include > in cover letters for convenience. Well, no, doing this per patch is possible and relevant. For cascading effects their mentioning in a revlog can be pretty brief. >>> +#define _txt(x) __va(x) >>> +#endif >> >> Without any uses the correctness of the above is hard to judge. > > The _txt() macro is used right below: Well, the comment was meant a little indirectly: The correctness in particular wrt the __EARLY_SLAUNCH__ distinction. >>> +/* >>> + * Always use private space as some of registers are either read-only or >>> not >>> + * present in public space. >>> + */ >>> +static inline uint64_t txt_read(unsigned int reg_no) >>> +{ >>> + volatile uint64_t *reg = _txt(TXT_PRIV_CONFIG_REGS_BASE + reg_no); >>> + return *reg; >>> +} >>> + >>> +static inline void txt_write(unsigned int reg_no, uint64_t val) >>> +{ >>> + volatile uint64_t *reg = _txt(TXT_PRIV_CONFIG_REGS_BASE + reg_no); >>> + *reg = val; >>> +} > >>> + * This serves as TXT register barrier after writing to >>> + * TXTCR_CMD_UNLOCK_MEM_CONFIG. Must be done to ensure that any future >>> + * chipset operations see the write. >>> + */ >>> + (void)txt_read(TXTCR_ESTS); >> >> I don't think the cast is needed. > > It's not needed, but I think that explicitly discarding unused return > value is a generally good practice even when there is a comment. In the context of Misra there has been discussion about doing so. But in our present code base you will find such as the exception, not the rule. >>> + txt_write(TXTCR_CMD_RESET, 1); >>> + unreachable(); >> >> What guarantees the write to take immediate effect? That is, shouldn't there >> be e.g. an infinite loop here, just in case? > > I'll return infinite loop from v2. Tried adding `halt()` as Ross > suggests, but including <asm/system.h> doesn't work in the early code > (something about compat headers and missing expansion of things like > __DeFiNe__). Yeah, untangling that may be a little involved. Open-coding halt() is an option, as long as you clearly indicate it as such (for e.g. grep to still find that instance). Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |