Re: [PATCH 2/4] x86/virt/tdx: Advertise the CC_ATTR_HOST_MEM_INCOHERENT for TDX host

[Huang, Kai]

On 1/02/2024 1:11 am, Dave Hansen wrote:
> On 1/31/24 03:31, Huang, Kai wrote:
> > From: Kai Huang <kai.huang@xxxxxxxxx>
> >
> > On the TDX capable platform, during kexec() the old kernel needs to
> > flush dirty cachelines of all TDX private memory otherwise they may
> > silently corrupt the new kernel's memory.
> >
> > Advertise the new introduced CC_ATTR_HOST_MEM_INCOHERENT attribute for
> > TDX host platform so the cache will be flushed during kexec().
>
> So, you're setting a new bit, CC_ATTR_HOST_MEM_INCOHERENT.  The way I
> like to deal with these is to go back and look at the definition of
> CC_ATTR_HOST_MEM_INCOHERENT and see whether the changelog here convinces
> me that CC_ATTR_HOST_MEM_INCOHERENT is being set appropriately.  Bonus
> points if this changelog uses the same nomenclature as the comment
> describing CC_ATTR_HOST_MEM_INCOHERENT.
>
> How well does this match the comment above CC_ATTR_HOST_MEM_INCOHERENT?

I will try to improve the changelog: explain what does 
CC_ATTR_HOST_MEM_INCOHERENT mean, and why it is suitable for TDX host.

Please let me know if you have more comments.  Thanks.

> > Note theoretically cache flush is only needed when TDX module is
> > initialized, but the module initialization is done at runtime so just
> > advertise the CC attribute when the platform has TDX enabled.
>
> I find this really hard to parse.  Here's a rewrite, as usual:
>
> 	A TDX-host-capable system might not actually have any incoherent
> 	memory.  This can occur if a TDX module was never initialized or
> 	if the caches have been flushed since the last time TDX was
> 	used.  Ignore that case.  Eliminate the need for any locking and
> 	assume that any TDX-host-capable system might have incoherent
> 	memory by always setting CC_ATTR_HOST_MEM_INCOHERENT.

I appreciate, as usual.  :-)