On 14/08/2025 2:12 pm, Jan Beulich wrote:
> On 08.08.2025 22:23, Andrew Cooper wrote:
>> @@ -42,17 +46,76 @@ struct cpu_user_regs
>> */
>>
>> union { uint64_t rip; uint32_t eip; uint16_t ip; };
>> - uint16_t cs, _pad0[1];
>> - uint8_t saved_upcall_mask; /* PV (v)rflags.IF == !saved_upcall_mask */
>> - uint8_t _pad1[3];
>> + union {
>> + struct {
>> + uint16_t cs;
>> + unsigned long :16;
>> + uint8_t saved_upcall_mask; /* PV (v)rflags.IF ==
>> !saved_upcall_mask */
> Would this better be reproduced ...
>
>> + };
>> + unsigned long csx;
>> + struct {
>> + /*
>> + * Bits 0 thru 31 control ERET{U,S} behaviour, and is state of
>> the
>> + * interrupted context.
>> + */
>> + uint16_t cs;
>> + unsigned int sl:2; /* Stack Level */
>> + bool wfe:1; /* Wait-for-ENDBRANCH state */
> ... here as well, just like you reproduce "cs"?
saved_upcall_mask is a property of an in-guest IRET frame only. It is
only produced in create_bounce_frame, and never consumed by Xen.
It needs to exist in this structure so asm-offsets.c can generate a
constant.
Also, be aware that there are new features being planned which rely on FRED.
>
>> + } fred_cs;
>> + };
>> union { uint64_t rflags; uint32_t eflags; uint16_t flags; };
>> union { uint64_t rsp; uint32_t esp; uint16_t sp; uint8_t spl;
>> };
>> - uint16_t ss, _pad2[3];
>> + union {
>> + uint16_t ss;
>> + unsigned long ssx;
> What use do you foresee for this and "csx"?
That also came from Linux. I'm using it to zero the control metadata so
ERETU behaves more like IRET.
>
>> + struct {
>> + /*
>> + * Bits 0 thru 31 control ERET{U,S} behaviour, and is state
>> about
>> + * the event which occured.
>> + */
>> + uint16_t ss;
>> + bool sti:1; /* Was blocked-by-STI, and not
>> cancelled */
>> + bool swint:1; /* Was a SYSCALL/SYSENTER/INT $N */
>> + bool nmi:1; /* Was an NMI. */
>> + unsigned long :13;
>> +
>> + /*
>> + * Bits 32 thru 63 are ignored by ERET{U,S} and are informative
>> + * only.
>> + */
>> + uint8_t vector;
>> + unsigned long :8;
>> + unsigned int type:4; /* X86_ET_* */
>> + unsigned long :4;
>> + bool enclave:1; /* Event taken in SGX mode */
>> + bool lm:1; /* Was in Long Mode */
> The bit indicates 64-bit mode aiui, not long mode (without which FRED isn't
> even
> available).
Oh, yes. This is something that changed across revisions, and I wrote
this patch to an older spec.
It's %cs.l of the interrupted context, so I probably should just drop the m.
>
>> --- a/xen/arch/x86/include/asm/current.h
>> +++ b/xen/arch/x86/include/asm/current.h
>> @@ -38,6 +38,8 @@ struct vcpu;
>>
>> struct cpu_info {
>> struct cpu_user_regs guest_cpu_user_regs;
>> + struct fred_info _fred; /* Only used when FRED is active. */
> Any particular need for the leading underscore?
Somewhat, yes. It's not safe to reference this field, except for
loading MSR_PL0_RSP.
Everyone else should use cpu_regs_fred_info() to get the fred_info,
which has a safety ASSERT().
~Andrew
