linux-kernel - Re: [PATCH v5 3/5] hyperv: Enable the hypercall output page for the VTL mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <17dfb71a-119c-4906-bc22-4f65fb28676b@linux.microsoft.com>
Date: Tue, 7 Jan 2025 15:11:15 -0800
From: Roman Kisel <romank@...ux.microsoft.com>
To: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
Cc: hpa@...or.com, kys@...rosoft.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, decui@...rosoft.com,
 eahariha@...ux.microsoft.com, haiyangz@...rosoft.com, mingo@...hat.com,
 mhklinux@...look.com, nunodasneves@...ux.microsoft.com, tglx@...utronix.de,
 tiala@...rosoft.com, wei.liu@...nel.org, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, x86@...nel.org, apais@...rosoft.com,
 benhill@...rosoft.com, ssengar@...rosoft.com, sunilmut@...rosoft.com,
 vdso@...bites.dev
Subject: Re: [PATCH v5 3/5] hyperv: Enable the hypercall output page for the
 VTL mode



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.

[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, &reg_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