Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220411135939.906700-1-yongqiang.sun@amd.com>
 <f06ce5e3-158b-88a8-06f0-53b88c4fad45@molgen.mpg.de> <CADnq5_PSZX+11meHYn9CR3A4LiseGo30TwbkOuPgTQsfbYxmCg@mail.gmail.com>
 <88760cd1-d1ed-8952-d061-15e238b2ec57@molgen.mpg.de>
In-Reply-To: <88760cd1-d1ed-8952-d061-15e238b2ec57@molgen.mpg.de>
From:   Alex Deucher <alexdeucher@gmail.com>
Date:   Tue, 12 Apr 2022 10:12:56 -0400
Message-ID: <CADnq5_MdSn0_X6Bqd-Ljv_QzVpgtBYXUkkALEdFasCiiXx=o7g@mail.gmail.com>
Subject: Re: [PATCH] drm/amd/amdgpu: Not request init data for MS_HYPERV with vega10
To:     Paul Menzel <pmenzel@molgen.mpg.de>
Cc:     Yongqiang Sun <yongqiang.sun@amd.com>,
        Jingwen Chen <jingwen.chen2@amd.com>,
        "Luo, Zhigang" <zhigang.luo@amd.com>,
        amd-gfx list <amd-gfx@lists.freedesktop.org>,
        "monk.liu" <monk.liu@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Apr 12, 2022 at 4:01 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> [Cc: +x86 folks]
>
> Dear Alex, dear x86 folks,
>
>
> x86 folks, can you think of alternatives to access `X86_HYPER_MS_HYPERV`
> from `arch/x86/include/asm/hypervisor.h` without any preprocessor ifdef-e=
ry?

I don't really see what problem that solves.  X86_HYPER_MS_HYPERV is
an X86 thing.  Why do we need a processor agnostic way to handle it?
Any code related to that is X86 specific.

Alex

>
>
> Am 11.04.22 um 18:28 schrieb Alex Deucher:
> > On Mon, Apr 11, 2022 at 11:28 AM Paul Menzel wrote:
>
> [=E2=80=A6]
>
> >> Am 11.04.22 um 15:59 schrieb Yongqiang Sun:
> >>> MS_HYPERV with vega10 doesn't have the interface to process
> >>> request init data msg.
> >>
> >> Should some Hyper-V folks be added to the reviewers list too?
> >>
> >>> Check hypervisor type to not send the request for MS_HYPERV.
> >>
> >> Please add a blank line between paragraphs.
> >>
> >>> Signed-off-by: Yongqiang Sun <yongqiang.sun@amd.com>
> >>> ---
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 12 ++++++++++--
> >>>    1 file changed, 10 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/d=
rm/amd/amdgpu/amdgpu_virt.c
> >>> index 933c41f77c92..56b130ec44a9 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >>> @@ -23,6 +23,10 @@
> >>>
> >>>    #include <linux/module.h>
> >>>
> >>> +#ifdef CONFIG_X86
> >>> +#include <asm/hypervisor.h>
> >>> +#endif
> >>> +
> >>>    #include <drm/drm_drv.h>
> >>>
> >>>    #include "amdgpu.h"
> >>> @@ -721,8 +725,12 @@ void amdgpu_detect_virtualization(struct amdgpu_=
device *adev)
> >>>                        break;
> >>>                case CHIP_VEGA10:
> >>>                        soc15_set_virt_ops(adev);
> >>> -                     /* send a dummy GPU_INIT_DATA request to host o=
n vega10 */
> >>> -                     amdgpu_virt_request_init_data(adev);
> >>> +#ifdef CONFIG_X86
> >>> +                     /* not send GPU_INIT_DATA with MS_HYPERV*/
> >>> +                     if (hypervisor_is_type(X86_HYPER_MS_HYPERV) =3D=
=3D false)
> >>> +#endif
> >>
> >> Why guard everything with CONFIG_X86? (If it=E2=80=99s needed, it shou=
ld be done
> >> in C code.)
> >
> > X86_HYPER_MS_HYPERV only available on x86.
>
> Sorry, I missed the X86 dependency when quickly looking at the Hyper-V
> stub IOMMU driver `drivers/iommu/hyperv-iommu.c`, but missed that
> `HYPERV_IOMMU` has `depends on HYPERV && X86`.
>
>
> Kind regards,
>
> Paul
>
>
> >>> +                             /* send a dummy GPU_INIT_DATA request t=
o host on vega10 */
> >>> +                             amdgpu_virt_request_init_data(adev);
> >>>                        break;
> >>>                case CHIP_VEGA20:
> >>>                case CHIP_ARCTURUS:
> >>
> >>
> >> Kind regards,
> >>
> >> Paul