From: Dave Hansen <dave.hansen@linux.intel.com>
To: dave.hansen@linux.intel.com
Subject: Re: [PATCH v9 20/23] x86/virt/tdx: Reject updates during compatibility-sensitive operations
In-Reply-To: <20260513151045.1420990-21-chao.gao@intel.com>
References: <20260513151045.1420990-21-chao.gao@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

[Draft review generated by x86-maintainer-agent applying Dave_Hansen.txt
 rules to the v9 series.  Strip this banner before sending.]

> +#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE BIT(16)

This bit ends up OR'd into `args.rcx`, which is u64.  Use BIT_ULL(16)
to match the underlying type.  Dave_Hansen.txt RULE 7 -- not strictly
a magic number but the BIT/BIT_ULL choice should follow the
destination width.

> +	if ((err & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
> +		return -EBUSY;

TDX_SEAMCALL_STATUS_MASK currently lives in
arch/x86/kvm/vmx/tdx_errno.h (KVM-private).  If the base-commit
hasn't moved it to a shared header yet, this patch won't build for
the host-side code.  Either:

  - add a prep patch up front that lifts TDX_SEAMCALL_STATUS_MASK
    (and the companion error codes it pairs with) into asm/tdx.h or
    a new arch/x86/include/asm/tdx_errno.h, or
  - confirm in the cover letter which base patch makes that available.

The alternatives section -- "rejected fail-TD-build because KVM uABI
risk; rejected leak-through because invisible" -- is the right shape
(RULE 1).  Thanks for moving that here from the v8 thread.

The new -EBUSY -> FW_UPLOAD_ERR_BUSY mapping in tdx_fw_write() is the
reason the switch in 10/23 was a switch and not an if/else.  Worth a
note in 10/23's changelog or a comment to that effect (see 10/23
review).