| 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
| ||
|
Message-ID:
<SN6PR02MB41571CB74AAAB29181443EE9D41BA@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Mon, 29 Sep 2025 17:57:18 +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-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"prapal@...ux.microsoft.com" <prapal@...ux.microsoft.com>,
"easwar.hariharan@...ux.microsoft.com"
<easwar.hariharan@...ux.microsoft.com>, "tiala@...rosoft.com"
<tiala@...rosoft.com>, "anirudh@...rudhrb.com" <anirudh@...rudhrb.com>,
"paekkaladevi@...ux.microsoft.com" <paekkaladevi@...ux.microsoft.com>,
"skinsburskii@...ux.microsoft.com" <skinsburskii@...ux.microsoft.com>
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>, Jinank Jain
<jinankjain@...ux.microsoft.com>
Subject: RE: [PATCH v4 5/5] mshv: Introduce new hypercall to map stats page
for L1VH partitions
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
>
> From: Jinank Jain <jinankjain@...ux.microsoft.com>
>
> Introduce HVCALL_MAP_STATS_PAGE2 which provides a map location (GPFN)
> to map the stats to. This hypercall is required for L1VH partitions,
> depending on the hypervisor version. This uses the same check as the
> state page map location; mshv_use_overlay_gpfn().
>
> Add mshv_map_vp_state_page() helpers to use this new hypercall or the
> old one depending on availability.
>
> For unmapping, the original HVCALL_UNMAP_STATS_PAGE works for both
> cases.
>
> Signed-off-by: Jinank Jain <jinankjain@...ux.microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> Reviewed-by: Easwar Hariharan <easwar.hariharan@...ux.microsoft.com>
> ---
> drivers/hv/mshv_root.h | 10 ++--
> drivers/hv/mshv_root_hv_call.c | 93 ++++++++++++++++++++++++++++++++--
> drivers/hv/mshv_root_main.c | 22 ++++----
> include/hyperv/hvgdk_mini.h | 1 +
> include/hyperv/hvhdk_mini.h | 7 +++
> 5 files changed, 113 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> index dbe2d1d0b22f..0dfccfbe6123 100644
> --- a/drivers/hv/mshv_root.h
> +++ b/drivers/hv/mshv_root.h
> @@ -297,11 +297,11 @@ int hv_call_connect_port(u64 port_partition_id, union hv_port_id port_id,
> int hv_call_disconnect_port(u64 connection_partition_id,
> union hv_connection_id connection_id);
> int hv_call_notify_port_ring_empty(u32 sint_index);
> -int hv_call_map_stat_page(enum hv_stats_object_type type,
> - const union hv_stats_object_identity *identity,
> - void **addr);
> -int hv_call_unmap_stat_page(enum hv_stats_object_type type,
> - const union hv_stats_object_identity *identity);
> +int hv_map_stats_page(enum hv_stats_object_type type,
> + const union hv_stats_object_identity *identity,
> + void **addr);
> +int hv_unmap_stats_page(enum hv_stats_object_type type, void *page_addr,
> + const union hv_stats_object_identity *identity);
> int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
> u64 page_struct_count, u32 host_access,
> u32 flags, u8 acquire);
> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index 98c6278ff151..5a805b3dec0b 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
> @@ -804,9 +804,51 @@ hv_call_notify_port_ring_empty(u32 sint_index)
> return hv_result_to_errno(status);
> }
>
> -int hv_call_map_stat_page(enum hv_stats_object_type type,
> - const union hv_stats_object_identity *identity,
> - void **addr)
> +static int hv_call_map_stats_page2(enum hv_stats_object_type type,
> + const union hv_stats_object_identity *identity,
> + u64 map_location)
> +{
> + unsigned long flags;
> + struct hv_input_map_stats_page2 *input;
> + u64 status;
> + int ret;
> +
> + if (!map_location || !mshv_use_overlay_gpfn())
> + return -EINVAL;
> +
> + do {
> + local_irq_save(flags);
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> + memset(input, 0, sizeof(*input));
> + input->type = type;
> + input->identity = *identity;
> + input->map_location = map_location;
> +
> + status = hv_do_hypercall(HVCALL_MAP_STATS_PAGE2, input, NULL);
> +
> + local_irq_restore(flags);
> +
> + ret = hv_result_to_errno(status);
> +
> + if (!ret)
> + break;
This logic is incorrect. If the hypercall returns
HV_STATUS_INSUFFICIENT_MEMORY, then errno -ENOMEM is immediately
returned to the caller without any opportunity to do hv_call_deposit_pages().
> +
> + if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
> + hv_status_debug(status, "\n");
> + break;
> + }
> +
> + ret = hv_call_deposit_pages(NUMA_NO_NODE,
> + hv_current_partition_id, 1);
> + } while (!ret);
> +
> + return ret;
> +}
> +
> +static int hv_call_map_stats_page(enum hv_stats_object_type type,
> + const union hv_stats_object_identity *identity,
> + void **addr)
> {
> unsigned long flags;
> struct hv_input_map_stats_page *input;
> @@ -845,8 +887,36 @@ int hv_call_map_stat_page(enum hv_stats_object_type type,
> return ret;
> }
>
> -int hv_call_unmap_stat_page(enum hv_stats_object_type type,
> - const union hv_stats_object_identity *identity)
> +int hv_map_stats_page(enum hv_stats_object_type type,
> + const union hv_stats_object_identity *identity,
> + void **addr)
> +{
> + int ret;
> + struct page *allocated_page = NULL;
> +
> + if (!addr)
> + return -EINVAL;
> +
> + if (mshv_use_overlay_gpfn()) {
> + allocated_page = alloc_page(GFP_KERNEL);
> + if (!allocated_page)
> + return -ENOMEM;
> +
> + ret = hv_call_map_stats_page2(type, identity,
> + page_to_pfn(allocated_page));
This should use page_to_hvpfn() per my comments in Patch 4 of this series.
> + *addr = page_address(allocated_page);
> + } else {
> + ret = hv_call_map_stats_page(type, identity, addr);
> + }
> +
> + if (ret && allocated_page)
> + __free_page(allocated_page);
Might want to do *addr = NULL after freeing the page so that the caller
can't erroneously reference the free page. Again, the current caller doesn't
do that, so current code isn't broken.
> +
> + return ret;
> +}
> +
> +static int hv_call_unmap_stats_page(enum hv_stats_object_type type,
> + const union hv_stats_object_identity *identity)
> {
> unsigned long flags;
> struct hv_input_unmap_stats_page *input;
> @@ -865,6 +935,19 @@ int hv_call_unmap_stat_page(enum hv_stats_object_type type,
> return hv_result_to_errno(status);
> }
>
> +int hv_unmap_stats_page(enum hv_stats_object_type type, void *page_addr,
> + const union hv_stats_object_identity *identity)
> +{
> + int ret;
> +
> + ret = hv_call_unmap_stats_page(type, identity);
> +
> + if (mshv_use_overlay_gpfn() && page_addr)
> + __free_page(virt_to_page(page_addr));
> +
> + return ret;
> +}
> +
> int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
> u64 page_struct_count, u32 host_access,
> u32 flags, u8 acquire)
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 2d0ad17acde6..71a8ab5db3b8 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -841,7 +841,8 @@ mshv_vp_release(struct inode *inode, struct file *filp)
> return 0;
> }
>
> -static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index)
> +static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index,
> + void *stats_pages[])
> {
> union hv_stats_object_identity identity = {
> .vp.partition_id = partition_id,
> @@ -849,10 +850,10 @@ static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index)
> };
>
> identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> - hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> + hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
>
> identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
> - hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> + hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
> }
>
> static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
> @@ -865,14 +866,14 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
> int err;
>
> identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> - err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
> - &stats_pages[HV_STATS_AREA_SELF]);
> + err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
> + &stats_pages[HV_STATS_AREA_SELF]);
> if (err)
> return err;
>
> identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
> - err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
> - &stats_pages[HV_STATS_AREA_PARENT]);
> + err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
> + &stats_pages[HV_STATS_AREA_PARENT]);
> if (err)
> goto unmap_self;
>
> @@ -880,7 +881,7 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>
> unmap_self:
> identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> - hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> + hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
> return err;
> }
>
> @@ -988,7 +989,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> kfree(vp);
> unmap_stats_pages:
> if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> - mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
> + mshv_vp_stats_unmap(partition->pt_id, args.vp_index, stats_pages);
> unmap_ghcb_page:
> if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
> hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
> @@ -1740,7 +1741,8 @@ static void destroy_partition(struct mshv_partition *partition)
> continue;
>
> if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> - mshv_vp_stats_unmap(partition->pt_id, vp->vp_index);
> + mshv_vp_stats_unmap(partition->pt_id, vp->vp_index,
> + (void **)vp->vp_stats_pages);
>
> if (vp->vp_register_page) {
> (void)hv_unmap_vp_state_page(partition->pt_id,
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index ff4325fb623a..f66565106d21 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -493,6 +493,7 @@ union hv_vp_assist_msr_contents { /*
> HV_REGISTER_VP_ASSIST_PAGE */
> #define HVCALL_GET_PARTITION_PROPERTY_EX 0x0101
> #define HVCALL_MMIO_READ 0x0106
> #define HVCALL_MMIO_WRITE 0x0107
> +#define HVCALL_MAP_STATS_PAGE2 0x0131
>
> /* HV_HYPERCALL_INPUT */
> #define HV_HYPERCALL_RESULT_MASK GENMASK_ULL(15, 0)
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index bf2ce27dfcc5..064bf735cab6 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -177,6 +177,13 @@ struct hv_input_map_stats_page {
> union hv_stats_object_identity identity;
> } __packed;
>
> +struct hv_input_map_stats_page2 {
> + u32 type; /* enum hv_stats_object_type */
> + u32 padding;
> + union hv_stats_object_identity identity;
> + u64 map_location;
> +} __packed;
> +
> struct hv_output_map_stats_page {
> u64 map_location;
> } __packed;
> --
> 2.34.1
>
Powered by blists - more mailing lists