Re: [PATCH 07/13] x86/virt/tdx: Add SEAMCALL wrapper tdh_mem_sept_add() to add SEPT pages

Edgecombe, Rick P
2025-01-08T01:43:51Z

On Tue, 2025-01-07 at 17:20 -0800, Dave Hansen wrote:
> On 1/7/25 17:12, Yan Zhao wrote:
> > So, why does this bitfields definition make things worse?
>
> Look at the kernel page table management. Why don't we use bitfields for
> _that_? Look at the link I sent. Bitfields can cause some really goofy
> unexpected behavior if you pass them around like they were a full type.

Huh, so this enum is unsafe for reading out the individual fields because if
shifting them, it will perform the shift with the size of the source bit field
size. It is safe in the way it is being used in these patches, which is to
encode a u64. But if we ever started to use tdx_sept_gpa_mapping_info to process
output from a SEAMCALL, or something, we could set ourselves up for the same
problem as the SEV bug.

Let's not open code the encoding in each SEAMCALL though. What about replacing
it with just a helper that encodes the u64 gpa from two args: gfn and tdx_level.
We could add some specific over-size behavior for the fields, but I'd think it
would be ok to keep it simple. Maybe something like this:

static u64 encode_gpa_mapping_info(gfn_t gfn, unsigned int tdx_level)
{
u64 val = 0;

val |= level;
val |= gfn << TDX_MAPPING_INFO_GFN_SHIFT;

return val;
}