[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <15532d34-0005-4cba-9fca-212c30db1f18@linux.microsoft.com>
Date: Fri, 5 Sep 2025 16:12:48 -0700
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To: Easwar Hariharan <easwar.hariharan@...ux.microsoft.com>
Cc: linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
mhklinux@...look.com, decui@...rosoft.com, paekkaladevi@...ux.microsoft.com,
Jinank Jain <jinankjain@...ux.microsoft.com>
Subject: Re: [PATCH 6/6] mshv: Introduce new hypercall to map stats page for
L1VH partitions
On 9/5/2025 12:50 PM, Easwar Hariharan wrote:
> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>> 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>
>> ---
>> drivers/hv/mshv_root.h | 10 ++--
>> drivers/hv/mshv_root_hv_call.c | 92 ++++++++++++++++++++++++++++++++--
>> drivers/hv/mshv_root_main.c | 25 +++++----
>> include/hyperv/hvgdk_mini.h | 1 +
>> include/hyperv/hvhdk_mini.h | 7 +++
>> 5 files changed, 115 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
>> index d7c9520ef788..d16a020ae0ee 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 1882cc90f2f5..44013751cfc1 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -804,6 +804,45 @@ hv_call_notify_port_ring_empty(u32 sint_index)
>> return hv_result_to_errno(status);
>> }
>>
>> +static int
>> +hv_call_map_stats_page2(enum hv_stats_object_type type,
>
> Again, one line please. Also below.
>
Ack
>> + 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);
>> + if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
>> + if (hv_result_success(status))
>> + break;
>> + hv_status_debug(status, "\n");
>> + return hv_result_to_errno(status);
>> + }
>> +
>> + ret = hv_call_deposit_pages(NUMA_NO_NODE,
>> + hv_current_partition_id, 1);
>> + } while (!ret);
>> +
>> + return ret;
>> +}
>> +
>> static int
>> hv_stats_get_area_type(enum hv_stats_object_type type,
>> const union hv_stats_object_identity *identity)
>> @@ -822,9 +861,10 @@ hv_stats_get_area_type(enum hv_stats_object_type type,
>> return -EINVAL;
>> }
>>
>> -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_page(enum hv_stats_object_type type,
>> + const union hv_stats_object_identity *identity,
>> + void **addr)
>
> ...
>
ack
>> {
>> unsigned long flags;
>> struct hv_input_map_stats_page *input;
>> @@ -880,8 +920,37 @@ 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));
>> + *addr = page_address(allocated_page);
>> + } else {
>> + ret = hv_call_map_stats_page(type, identity, addr);
>> + }
>> +
>> + if (ret && allocated_page)
>> + __free_page(allocated_page);
>> +
>> + return ret;
>> +}
>> +
>> +static int
>> +hv_call_unmap_stats_page(enum hv_stats_object_type type,
>
> ...
>
ack
>> + const union hv_stats_object_identity *identity)
>> {
>> unsigned long flags;
>> struct hv_input_unmap_stats_page *input;
>> @@ -900,6 +969,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 f91880cc9e29..1699423cc524 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -894,7 +894,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,
>> @@ -902,10 +903,13 @@ 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);
>> +
>> + if (stats_pages[HV_STATS_AREA_PARENT] == stats_pages[HV_STATS_AREA_SELF])
>> + return;
>>
>> 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,
>> @@ -918,14 +922,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]);
>
> Shouldn't this be in the previous patch where the other hv_call_map_stat_page() calls were
> converted, same below?
>
The previous patch converts hv_call_(un)map_vp_state_page to hv_(un)map_vp_state_page, i.e.
"state" not "stats".
>> 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;
>>
>> @@ -936,7 +940,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;
>> }
>>
>
> <snip>
Powered by blists - more mailing lists