On Tue, Jan 07, 2025 at 03:11:15PM -0800, Roman Kisel wrote:
>
>
> On 1/7/2025 11:18 AM, Stanislav Kinsburskii wrote:
> > On Mon, Jan 06, 2025 at 01:07:25PM -0800, Roman Kisel wrote:
> >
>
> [...]
>
> > My point is that the proposed fix looks more like an Underhill-tailored
> > bandage and doesn't take the needs of other stake holders into
> > consideration.
> The patch takes as much into consideration as present in the hyperv-next
> tree. Working on the open-source project seems to be harder otherwise.
> A bandage, or not, that's a matter of opinion. There's a been a break,
> here's the bandage.
>
> >
> > What is the urgency in merging of this particular change?
>
> The get_vtl function is broken thus blocking any further work on
> upstreaming VTL mode patches, ARM64 and more. That's not an urgent
> urgency where customers are in pain, more like the urgency of needing
> to take the trash out, and until that happens, continuing inhaling the
> fumes.
>
> The urgency of unblocking is to continue work on proposing VTL mode
> patches not to carry lots of out-of-tree code in the fork.
>
> There might be a future where the Hyper-V code offers an API surface
> covering needs of consumers like dom0 and VTLs whereby they maybe can
> be built as an out-of-tree modules so the opinions wouldn't clash as
> much.
>
> Avoiding using the output hypercall page leads to something like[1]
> and it looks quite complicated although that's the bare bones, lots
> of notes.
>
How is this related to the original discussion?
My concern was about the piece allocating of the output page guarded by
the VTL config option.
Thanks,
Stas
> [1]
>
> /*
> * Fast extended hypercall with 20 bytes of input and 16 bytes of
> * output for getting a VP register.
> *
> * NOTES:
> * 1. The function is __init only atm, so the XMM context isn't
> * used by the user mode.
> * 2. X86_64 only.
> * 3. Fast extended hypercalls may use XMM0..XMM6, and XMM is
> * architerctural on X86_64 yet the support should be enabled
> * in the CR's. Here, need RDX, R8 and XMM0 for input and RDX,
> * R8 for output
> * 4. No provisions for TDX and SEV-SNP for the sake of simplicity
> * (the hypervisor cannot see the guest registers in the
> * confidential VM), would need to fallback.
> * 5. The robust implementation would need to check if fast extended
> * hypercalls are available by checking the synthehtic CPUID leaves.
> * A separate leaf indicates fast output support.
> * It _almost_ certainly has to be, unless somehow disabled, hard
> * to see why that would be needed.
> */
> struct hv_u128 {
> u64 low_part;
> u64 high_part;
> } __packed;
>
> static __init u64 hv_vp_get_register_xfast(u32 reg_name,
> struct hv_u128 *value)
> {
> u64 control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_REGISTERS |
> HV_HYPERCALL_FAST_BIT;
> unsigned long flags;
> u64 hv_status;
>
> union {
> struct hv_get_vp_registers_input input;
> struct {
> u64 lo;
> u64 hi;
> } __packed as_u128;
> } hv_input;
> register u64 rdx asm("rdx");
> register u64 r8 asm("r8");
> register u64 r12 asm("r12");
>
> local_irq_save(flags);
>
> hv_input.as_u128.lo = hv_input.as_u128.hi = 0;
> hv_input.input.header.partitionid = HV_PARTITION_ID_SELF;
> hv_input.input.header.vpindex = HV_VP_INDEX_SELF;
> hv_input.input.header.inputvtl = 0;
>
> rdx = hv_input.as_u128.lo;
> r8 = hv_input.as_u128.hi;
> r12 = reg_name;
>
> __asm__ __volatile__(
> "subq $16, %%rsp\n"
> "movups %%xmm0, 16(%%rsp)\n"
> "movd %%r12, %%xmm0\n"
> CALL_NOSPEC
> "movups 16(%%rsp), %%xmm0\n"
> "addq $16, %%rsp\n"
> : "=a" (hv_status), ASM_CALL_CONSTRAINT,
> "+c" (control), "+r" (rdx), "+r" (r8)
> : THUNK_TARGET(hv_hypercall_pg), "r"(r12)
> : "cc", "memory", "r9", "r10", "r11");
>
> if (hv_result_success(hv_status)) {
> value->low_part = rdx;
> value->high_part = r8;
> }
>
> local_irq_restore(flags);
> return hv_status;
> }
>
> #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> u8 __init get_vtl(void)
> {
> struct hv_u128 reg_value;
> u64 ret = hv_vp_get_register_xfast(HV_REGISTER_VSM_VP_STATUS,
> ®_value);
>
> if (hv_result_success(ret)) {
> ret = reg_value.low_part & HV_VTL_MASK;
> } else {
> pr_err("Failed to get VTL(error: %lld) exiting...\n", ret);
> BUG();
> }
>
> return ret;
> }
> #endif
>
> >
> > Thanks,
> > Stas
>
> --
> Thank you,
> Roman
>
