From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilia Mirkin Subject: Re: [PATCH] amdgpu: fix asic initialization for virtualized environments Date: Sat, 11 Jun 2016 00:17:12 -0400 Message-ID: References: <20160611034216.22192-1-andres.rodriguez@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0602927604==" Return-path: Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id A61156E24A for ; Sat, 11 Jun 2016 04:17:13 +0000 (UTC) Received: by mail-io0-x242.google.com with SMTP id d2so4014004iof.1 for ; Fri, 10 Jun 2016 21:17:13 -0700 (PDT) In-Reply-To: <20160611034216.22192-1-andres.rodriguez@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Andres Rodriguez Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0602927604== Content-Type: multipart/alternative; boundary=001a11426c16f5fbf90534f8eda0 --001a11426c16f5fbf90534f8eda0 Content-Type: text/plain; charset=UTF-8 On Jun 11, 2016 12:09 AM, "Andres Rodriguez" wrote: > > When executing in a PCI passthrough based virtuzliation environemnt, the > hypervisor will usually attempt to send a PCIe bus reset signal to the > ASIC when the VM reboots. In this scenario, the card is not correctly > initialized, but we still consider it to be posted. Therefore, in a > passthrough based environemnt we should always post the card to guarantee > it is in a good state for driver initialization. > > However, if we are operating in SR-IOV mode it is up to the GIM driver > to manage the asic state, therefore we should not post the card (and > shouldn't be able to do it either). > > Signed-off-by: Andres Rodriguez > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 7 +++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 16 +++++++++++++++- > drivers/gpu/drm/amd/amdgpu/cik.c | 7 +++++++ > drivers/gpu/drm/amd/amdgpu/vi.c | 15 +++++++++++++++ > 4 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 26fe670..fe71dea 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1835,6 +1835,8 @@ struct amdgpu_asic_funcs { > /* MM block clocks */ > int (*set_uvd_clocks)(struct amdgpu_device *adev, u32 vclk, u32 dclk); > int (*set_vce_clocks)(struct amdgpu_device *adev, u32 evclk, u32 ecclk); > + /* query virtual capabilities */ > + u32 (*get_virtual_caps)(struct amdgpu_device *adev); > }; > > /* > @@ -1932,8 +1934,12 @@ void amdgpu_cgs_destroy_device(struct cgs_device *cgs_device); > > > /* GPU virtualization */ > +#define AMDGPU_VIRT_CAPS_SRIOV_EN (1 << 0) > +#define AMDGPU_VIRT_CAPS_IS_VF (1 << 1) > struct amdgpu_virtualization { > bool supports_sr_iov; > + bool is_virtual; > + u32 caps; > }; > > /* > @@ -2226,6 +2232,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring) > #define amdgpu_asic_get_xclk(adev) (adev)->asic_funcs->get_xclk((adev)) > #define amdgpu_asic_set_uvd_clocks(adev, v, d) (adev)->asic_funcs->set_uvd_clocks((adev), (v), (d)) > #define amdgpu_asic_set_vce_clocks(adev, ev, ec) (adev)->asic_funcs->set_vce_clocks((adev), (ev), (ec)) > +#define amdgpu_asic_get_virtual_caps(adev) ((adev)->asic_funcs->get_virtual_caps((adev))) > #define amdgpu_asic_get_gpu_clock_counter(adev) (adev)->asic_funcs->get_gpu_clock_counter((adev)) > #define amdgpu_asic_read_disabled_bios(adev) (adev)->asic_funcs->read_disabled_bios((adev)) > #define amdgpu_asic_read_bios_from_rom(adev, b, l) (adev)->asic_funcs->read_bios_from_rom((adev), (b), (l)) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index b494212..629e4e3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1392,6 +1392,14 @@ static int amdgpu_resume(struct amdgpu_device *adev) > return 0; > } > > +static bool amdgpu_device_is_virtual(void) > +{ > +#ifdef CONFIG_X86 > + return boot_cpu_has(X86_FEATURE_HYPERVISOR); > +#else > + return false Missing semicolon here... > +#endif > +} > > /** > * amdgpu_device_has_dal_support - check if dal is supported > @@ -1560,8 +1568,14 @@ int amdgpu_device_init(struct amdgpu_device *adev, > adev->virtualization.supports_sr_iov = > amdgpu_atombios_has_gpu_virtualization_table(adev); > > + /* Check if we are executing in a virtualized environment */ > + adev->virtualization.is_virtual = amdgpu_device_is_virtual(); > + adev->virtualization.caps = amdgpu_asic_get_virtual_caps(adev); > + > /* Post card if necessary */ > - if (!amdgpu_card_posted(adev)) { > + if (!amdgpu_card_posted(adev) || > + (adev->virtualization.is_virtual && > + !adev->virtualization.caps & AMDGPU_VIRT_CAPS_SRIOV_EN)) { > if (!adev->bios) { > dev_err(adev->dev, "Card not posted and no BIOS - ignoring\n"); > return -EINVAL; > diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c > index 40f4fda..907bb28 100644 > --- a/drivers/gpu/drm/amd/amdgpu/cik.c > +++ b/drivers/gpu/drm/amd/amdgpu/cik.c > @@ -963,6 +963,12 @@ static bool cik_read_bios_from_rom(struct amdgpu_device *adev, > return true; > } > > +static u32 cik_get_virtual_caps(struct amdgpu_device *adev) > +{ > + /* CIK does not support SR-IOV */ > + return 0; > +} > + > static const struct amdgpu_allowed_register_entry cik_allowed_read_registers[] = { > {mmGRBM_STATUS, false}, > {mmGB_ADDR_CONFIG, false}, > @@ -2176,6 +2182,7 @@ static const struct amdgpu_asic_funcs cik_asic_funcs = > .get_xclk = &cik_get_xclk, > .set_uvd_clocks = &cik_set_uvd_clocks, > .set_vce_clocks = &cik_set_vce_clocks, > + .get_virtual_caps = &cik_get_virtual_caps, > /* these should be moved to their own ip modules */ > .get_gpu_clock_counter = &gfx_v7_0_get_gpu_clock_counter, > .wait_for_mc_idle = &gmc_v7_0_mc_wait_for_idle, > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c > index 1ac0c91..6a7d531 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vi.c > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c > @@ -422,6 +422,20 @@ static bool vi_read_bios_from_rom(struct amdgpu_device *adev, > return true; > } > > +static u32 vi_get_virtual_caps(struct amdgpu_device *adev) > +{ > + u32 caps = 0; > + u32 reg = RREG32(mmBIF_IOV_FUNC_IDENTIFIER); > + > + if (REG_GET_FIELD(reg, BIF_IOV_FUNC_IDENTIFIER, IOV_ENABLE)) > + caps |= AMDGPU_VIRT_CAPS_SRIOV_EN; > + > + if (REG_GET_FIELD(reg, BIF_IOV_FUNC_IDENTIFIER, FUNC_IDENTIFIER)) > + caps |= AMDGPU_VIRT_CAPS_IS_VF; > + > + return caps; > +} > + > static const struct amdgpu_allowed_register_entry tonga_allowed_read_registers[] = { > {mmGB_MACROTILE_MODE7, true}, > }; > @@ -1452,6 +1466,7 @@ static const struct amdgpu_asic_funcs vi_asic_funcs = > .get_xclk = &vi_get_xclk, > .set_uvd_clocks = &vi_set_uvd_clocks, > .set_vce_clocks = &vi_set_vce_clocks, > + .get_virtual_caps = &vi_get_virtual_caps, > /* these should be moved to their own ip modules */ > .get_gpu_clock_counter = &gfx_v8_0_get_gpu_clock_counter, > .wait_for_mc_idle = &gmc_v8_0_mc_wait_for_idle, > -- > 2.8.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel --001a11426c16f5fbf90534f8eda0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Jun 11, 2016 12:09 AM, "Andres Rodriguez" <andres.rodriguez@amd.com> wrote:
>
> When executing in a PCI passthrough based virtuzliation environemnt, t= he
> hypervisor will usually attempt to send a PCIe bus reset signal to the=
> ASIC when the VM reboots. In this scenario, the card is not correctly<= br> > initialized, but we still consider it to be posted. Therefore, in a > passthrough based environemnt we should always post the card to guaran= tee
> it is in a good state for driver initialization.
>
> However, if we are operating in SR-IOV mode it is up to the GIM driver=
> to manage the asic state, therefore we should not post the card (and > shouldn't be able to do it either).
>
> Signed-off-by: Andres Rodriguez <andres.rodriguez@amd.com>
> ---
> =C2=A0drivers/gpu/drm/amd/amdgpu/amdgpu.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 |= =C2=A0 7 +++++++
> =C2=A0drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 16 +++++++++++++++-=
> =C2=A0drivers/gpu/drm/amd/amdgpu/cik.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0|=C2=A0 7 +++++++
> =C2=A0drivers/gpu/drm/amd/amdgpu/vi.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 | 15 +++++++++++++++
> =C2=A04 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd= /amdgpu/amdgpu.h
> index 26fe670..fe71dea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1835,6 +1835,8 @@ struct amdgpu_asic_funcs {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* MM block clocks */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 int (*set_uvd_clocks)(struct amdgpu_device= *adev, u32 vclk, u32 dclk);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 int (*set_vce_clocks)(struct amdgpu_device= *adev, u32 evclk, u32 ecclk);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0/* query virtual capabilities */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 (*get_virtual_caps)(struct amdgpu_devi= ce *adev);
> =C2=A0};
>
> =C2=A0/*
> @@ -1932,8 +1934,12 @@ void amdgpu_cgs_destroy_device(struct cgs_devic= e *cgs_device);
>
>
> =C2=A0/* GPU virtualization */
> +#define AMDGPU_VIRT_CAPS_SRIOV_EN=C2=A0 =C2=A0 =C2=A0 =C2=A0(1 <&l= t; 0)
> +#define AMDGPU_VIRT_CAPS_IS_VF=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (1 &= lt;< 1)
> =C2=A0struct amdgpu_virtualization {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 bool supports_sr_iov;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0bool is_virtual;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 caps;
> =C2=A0};
>
> =C2=A0/*
> @@ -2226,6 +2232,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *rin= g)
> =C2=A0#define amdgpu_asic_get_xclk(adev) (adev)->asic_funcs->get= _xclk((adev))
> =C2=A0#define amdgpu_asic_set_uvd_clocks(adev, v, d) (adev)->asic_f= uncs->set_uvd_clocks((adev), (v), (d))
> =C2=A0#define amdgpu_asic_set_vce_clocks(adev, ev, ec) (adev)->asic= _funcs->set_vce_clocks((adev), (ev), (ec))
> +#define amdgpu_asic_get_virtual_caps(adev) ((adev)->asic_funcs->= ;get_virtual_caps((adev)))
> =C2=A0#define amdgpu_asic_get_gpu_clock_counter(adev) (adev)->asic_= funcs->get_gpu_clock_counter((adev))
> =C2=A0#define amdgpu_asic_read_disabled_bios(adev) (adev)->asic_fun= cs->read_disabled_bios((adev))
> =C2=A0#define amdgpu_asic_read_bios_from_rom(adev, b, l) (adev)->as= ic_funcs->read_bios_from_rom((adev), (b), (l))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/= drm/amd/amdgpu/amdgpu_device.c
> index b494212..629e4e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1392,6 +1392,14 @@ static int amdgpu_resume(struct amdgpu_device *= adev)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0;
> =C2=A0}
>
> +static bool amdgpu_device_is_virtual(void)
> +{
> +#ifdef CONFIG_X86
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0return boot_cpu_has(X86_FEATURE_HYPERVISOR= );
> +#else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0return false

Missing semicolon here...

> +#endif
> +}
>
> =C2=A0/**
> =C2=A0 * amdgpu_device_has_dal_support - check if dal is supported
> @@ -1560,8 +1568,14 @@ int amdgpu_device_init(struct amdgpu_device *ad= ev,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 adev->virtualization.supports_sr_iov = =3D
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 amdgpu_atombio= s_has_gpu_virtualization_table(adev);
>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0/* Check if we are executing in a virtuali= zed environment */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0adev->virtualization.is_virtual =3D amd= gpu_device_is_virtual();
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0adev->virtualization.caps =3D amdgpu_as= ic_get_virtual_caps(adev);
> +
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Post card if necessary */
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!amdgpu_card_posted(adev)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!amdgpu_card_posted(adev) ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(adev->virtualization.is_= virtual &&
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !adev->virtualization.ca= ps & AMDGPU_VIRT_CAPS_SRIOV_EN)) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!adev->= bios) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 dev_err(adev->dev, "Card not posted and no BIOS - ign= oring\n");
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/am= dgpu/cik.c
> index 40f4fda..907bb28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik.c
> @@ -963,6 +963,12 @@ static bool cik_read_bios_from_rom(struct amdgpu_= device *adev,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return true;
> =C2=A0}
>
> +static u32 cik_get_virtual_caps(struct amdgpu_device *adev)
> +{
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0/* CIK does not support SR-IOV */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> +}
> +
> =C2=A0static const struct amdgpu_allowed_register_entry cik_allowed_re= ad_registers[] =3D {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 {mmGRBM_STATUS, false},
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 {mmGB_ADDR_CONFIG, false},
> @@ -2176,6 +2182,7 @@ static const struct amdgpu_asic_funcs cik_asic_f= uncs =3D
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .get_xclk =3D &cik_get_xclk,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .set_uvd_clocks =3D &cik_set_uvd_clock= s,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .set_vce_clocks =3D &cik_set_vce_clock= s,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0.get_virtual_caps =3D &cik_get_virtual= _caps,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* these should be moved to their own ip m= odules */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .get_gpu_clock_counter =3D &gfx_v7_0_g= et_gpu_clock_counter,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .wait_for_mc_idle =3D &gmc_v7_0_mc_wai= t_for_idle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amd= gpu/vi.c
> index 1ac0c91..6a7d531 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -422,6 +422,20 @@ static bool vi_read_bios_from_rom(struct amdgpu_d= evice *adev,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return true;
> =C2=A0}
>
> +static u32 vi_get_virtual_caps(struct amdgpu_device *adev)
> +{
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 caps =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 reg =3D RREG32(mmBIF_IOV_FUNC_IDENTIFI= ER);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (REG_GET_FIELD(reg, BIF_IOV_FUNC_IDENTI= FIER, IOV_ENABLE))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0caps |=3D AMDG= PU_VIRT_CAPS_SRIOV_EN;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (REG_GET_FIELD(reg, BIF_IOV_FUNC_IDENTI= FIER, FUNC_IDENTIFIER))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0caps |=3D AMDG= PU_VIRT_CAPS_IS_VF;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0return caps;
> +}
> +
> =C2=A0static const struct amdgpu_allowed_register_entry tonga_allowed_= read_registers[] =3D {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 {mmGB_MACROTILE_MODE7, true},
> =C2=A0};
> @@ -1452,6 +1466,7 @@ static const struct amdgpu_asic_funcs vi_asic_fu= ncs =3D
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .get_xclk =3D &vi_get_xclk,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .set_uvd_clocks =3D &vi_set_uvd_clocks= ,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .set_vce_clocks =3D &vi_set_vce_clocks= ,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0.get_virtual_caps =3D &vi_get_virtual_= caps,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* these should be moved to their own ip m= odules */
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .get_gpu_clock_counter =3D &gfx_v8_0_g= et_gpu_clock_counter,
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 .wait_for_mc_idle =3D &gmc_v8_0_mc_wai= t_for_idle,
> --
> 2.8.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.fre= edesktop.org
> h= ttps://lists.freedesktop.org/mailman/listinfo/dri-devel

--001a11426c16f5fbf90534f8eda0-- --===============0602927604== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0602927604==--