[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SN6PR02MB4157ECC740A762C9C3B9EA6AD475A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Wed, 11 Jun 2025 23:06:51 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>
CC: "kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>, "catalin.marinas@....com"
<catalin.marinas@....com>, "will@...nel.org" <will@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>, "mingo@...hat.com"
<mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "hpa@...or.com"
<hpa@...or.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
"kw@...ux.com" <kw@...ux.com>, "manivannan.sadhasivam@...aro.org"
<manivannan.sadhasivam@...aro.org>, "robh@...nel.org" <robh@...nel.org>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "jinankjain@...ux.microsoft.com"
<jinankjain@...ux.microsoft.com>, "skinsburskii@...ux.microsoft.com"
<skinsburskii@...ux.microsoft.com>, "mrathor@...ux.microsoft.com"
<mrathor@...ux.microsoft.com>, "x86@...nel.org" <x86@...nel.org>
Subject: RE: [PATCH 2/4] Drivers: hv: Use nested hypercall for post message
and signal event
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Tuesday, June 10, 2025 4:52 PM
>
> When running nested, these hypercalls must be sent to the L0 hypervisor
> or vmbus will fail.
s/vmbus/VMBus/
>
> Add ARM64 stubs for the nested hypercall helpers to not break
> compilation (nested is still only supported in x86).
>
> Signed-off-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> ---
> arch/arm64/include/asm/mshyperv.h | 10 ++++++++++
> drivers/hv/connection.c | 3 +++
> drivers/hv/hv.c | 3 +++
> 3 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mshyperv.h
> b/arch/arm64/include/asm/mshyperv.h
> index b721d3134ab6..893d6a2e8dab 100644
> --- a/arch/arm64/include/asm/mshyperv.h
> +++ b/arch/arm64/include/asm/mshyperv.h
> @@ -53,6 +53,16 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
> return hv_get_msr(reg);
> }
>
> +static inline u64 hv_do_nested_hypercall(u64 control, void *input, void *output)
> +{
> + return U64_MAX;
> +}
> +
> +static inline u64 hv_do_fast_nested_hypercall8(u64 control, u64 input1)
> +{
> + return U64_MAX;
> +}
I think the definitions of hv_do_nested_hypercall() and
hv_do_fast_nested_hypercall8() are architecture independent. All
they do is add the HV_HYPERCALL_NESTED flag, which when
implemented for ARM64, will presumably be the same flag as
currently defined for x86. As such, couldn't the definitions of
hv_do_nested_hypercall() and hv_do_fast_nested_hypercall8()
be moved to asm-generic/mshyperv.h? Then stubs would not
be needed for ARM64. These two functions would never be
called on ARM64 because hv_nested is never true on ARM64
(at least for now), but the code would compile. And if either
function was erroneously called on ARM64, presumably
Hyper-V would return an error because HV_HYPERCALL_NESTED
is set.
> +
> /* SMCCC hypercall parameters */
> #define HV_SMCCC_FUNC_NUMBER 1
> #define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index be490c598785..992022bc770c 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -518,6 +518,9 @@ void vmbus_set_event(struct vmbus_channel *channel)
> channel->sig_event, 0);
> else
> WARN_ON_ONCE(1);
> + } else if (hv_nested) {
> + hv_do_fast_nested_hypercall8(HVCALL_SIGNAL_EVENT,
> + channel->sig_event);
> } else {
> hv_do_fast_hypercall8(HVCALL_SIGNAL_EVENT, channel->sig_event);
> }
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..99b73e779bf0 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -84,6 +84,9 @@ int hv_post_message(union hv_connection_id connection_id,
> sizeof(*aligned_msg));
> else
> status = HV_STATUS_INVALID_PARAMETER;
> + } else if (hv_nested) {
> + status = hv_do_nested_hypercall(HVCALL_POST_MESSAGE,
> + aligned_msg, NULL);
> } else {
> status = hv_do_hypercall(HVCALL_POST_MESSAGE,
> aligned_msg, NULL);
Are HVCALL_SIGNAL_EVENT and HVCALL_POST_MESSAGE the only two
hypercalls that are ever expected to need a "nested" version? I'm
wondering if the function hv_do_nested_hypercall() and
hv_do_fast_nested_hypercall8() could be dropped entirely, and just
pass the first argument to hv_do_hypercall() or hv_do_fast_hypercall8()
as <hypercall_name> | HV_HYPERCALL_NESTED. For only two cases, a
little bit of open coding might be preferable to the overhead of defining
functions just to wrap the or'ing of HV_HYPERCALL_NESTED.
The code above could then look like:
} else {
u64 control = HVCALL_POST_MESSAGE;
control |= hv_nested ? HV_HYPERCALL_NESTED : 0;
status = hv_do_hypercall(control, aligned_msg, NULL);
}
Again, ARM64 is implicitly handled because hv_nested is never set.
This is just a suggestion. It's motivated by the fact that we already have
three flavors of hypercall for HVCALL_SIGNAL_EVENT and
HVCALL_POST_MESSAGE, and I was looking for a way to avoid adding
a fourth flavor. But it's a marginal win, and if you prefer to keep the
inline functions, I'm OK with that.
Michael
Powered by blists - more mailing lists