Re: [PATCH v3 5/5] hyperv: Do not overlap the hvcall IO areas in hv_vtl_apicid_to_vp_id()

[Roman Kisel <romank@linux.microsoft.com>]

On 12/26/2024 2:01 PM, Easwar Hariharan wrote:
> On 12/26/2024 1:31 PM, Roman Kisel wrote:
>> The Top-Level Functional Specification for Hyper-V, Section 3.6 [1, 2],
>> disallows overlapping of the input and output hypercall areas, and
>> hv_vtl_apicid_to_vp_id() overlaps them.
>>
>> Use the output hypercall page of the current vCPU for the hypercall.
>>
>> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/hypercall-interface
>> [2] https://github.com/MicrosoftDocs/Virtualization-Documentation/tree/main/tlfs
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>>   arch/x86/hyperv/hv_vtl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
>> index 04775346369c..ec5716960162 100644
>> --- a/arch/x86/hyperv/hv_vtl.c
>> +++ b/arch/x86/hyperv/hv_vtl.c
>> @@ -189,7 +189,7 @@ static int hv_vtl_apicid_to_vp_id(u32 apic_id)
>>   	input->partition_id = HV_PARTITION_ID_SELF;
>>   	input->apic_ids[0] = apic_id;
>>   
>> -	output = (u32 *)input;
>> +	output = (u32*)*this_cpu_ptr(hyperv_pcpu_output_arg);
>                       ^
> Nit: I believe the space is preferred, but I won't insist on respinning
> it for that.
I think we can drop the whole type cast thing (as the other patch does).
Type coercion will do what is needed, and the type cast just appears to
be noise I guess.

> 
> It's a good idea to give credit to Michael with a Reported-by tag, and
> maybe a Closes: tag with a link to his email.
My bad, I should have definitely done that! Will certainly do!

> 
> As with the Fixes tag for patch 2, you don't need to respin the series
> and can just reply to this thread.
"Fixes" means something when the commit is present in the Linus'es tree
as I understood from the Wei's reply 
(https://lore.kernel.org/lkml/Z2OIsFUXcjVXpqtw@liuwe-devbox-debian-v2/):

" This commit is not in the mainline kernel yet, so this tag is not
needed.

It will most likely to be wrong since I will need to rebase the
hyperv-next branch.

I can fold this patch into the original patch and leave your
Signed-off-by there."

I'll check if the commit has been pulled into the mainline tree.

> 
> Otherwise, looks good to me.
> 
> Reviewed-by: Easwar Hariharan <eahariha@linux.microsoft.com>

-- 
Thank you,
Roman