On Tue, Jan 07, 2025 at 11:48:12AM -0800, Dave Hansen wrote:
> On 1/2/25 13:59, Edgecombe, Rick P wrote:
> > union tdx_sept_gpa_mapping_info {
> > struct {
> > u64 level : 3;
> > u64 reserved1 : 9;
> > u64 gfn : 40;
> > u64 reserved2 : 12;
> > };
> > u64 full;
> > };
>
> This is functionally OK, but seeing bitfields on a value that's probably
> going to get shifted around makes me nervous because of:
This is defined according to the TDX spec.
e.g. in TDH.MEM.SEPT.ADD:
RCX | EPT mapping information:
----|---------------------------------------------------------------------------
| Bits | Name | Description
|------|----------|---------------------------------------------------------
| 2:0 | Level | Level of the non-leaf Secure EPT entry that will map the
| | | new Secure EPT page - see 3.6.1
| | | Level must between 1 and 3 for a 4-level EPT or between
| | | 1 and 4 for a 5-level EPT.
|------|----------|---------------------------------------------------------
| 11:3 | Reserved | Reserved: must be 0
|------|----------|---------------------------------------------------------
| 51:12| GPA | Bits 51:12 of the guest physical address of to be mapped
| | | for the new Secure EPT page Depending on the level, the
| | | following least significant bits must be 0:
| | | Level 1 (EPT): Bits 20:12
| | | Level 2 (EPD): Bits 29:12
| | | Level 3 (EPDPT): Bits 38:12
| | | Level 4 (EPML4): Bits 47:12
|------|----------|---------------------------------------------------------
| 63:52| Reserved | Reserved: must be 0
So, why does this bitfields definition make things worse?
> > https://lore.kernel.org/lkml/20231111020019.553664-1-michael.roth@xxxxxxx/
> I wouldn't NAK it just for this, but it's also not how I would code it up.
