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

[Roman Kisel]

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@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxxxxxxxx>

--
Thank you,
Roman