[PATCH] drm/i915/gvt: Fix incorrect PCI BARs reporting
Du, Changbin
changbin.du at intel.com
Fri Aug 18 02:26:36 UTC 2017
Please hold this one, see failure with OVMF. Need double check. Thanks.
On Thu, Aug 17, 2017 at 04:46:32PM +0800, changbin.du at intel.com wrote:
> From: Changbin Du <changbin.du at intel.com>
>
> Looking at our virtual PCI device, we can see surprising Region 4 and Region 5.
> 00:10.0 VGA compatible controller: Intel Corporation Sky Lake Integrated Graphics (rev 06) (prog-if 00 [VGA controller])
> ....
> Region 0: Memory at 140000000 (64-bit, non-prefetchable) [size=16M]
> Region 2: Memory at 180000000 (64-bit, prefetchable) [size=1G]
> Region 4: Memory at <ignored> (32-bit, non-prefetchable)
> Region 5: Memory at <ignored> (32-bit, non-prefetchable)
> Expansion ROM at febd6000 [disabled] [size=2K]
>
> The fact is that we only implemented BAR0 and BAR2. Surprising Region 4 and
> Region 5 are shown because we report their size as 0xffffffff. They should
> report size 0 instead.
>
> BTW, the physical GPU has a PIO BAR. GVTg hasn't implemented PIO access, so
> we ignored this BAR for vGPU device.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1458032
> Signed-off-by: Changbin Du <changbin.du at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/cfg_space.c | 112 +++++++++++++++--------------------
> 1 file changed, 47 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c
> index be0dd85..9f313eb 100644
> --- a/drivers/gpu/drm/i915/gvt/cfg_space.c
> +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
> @@ -211,8 +211,6 @@ static int emulate_pci_command_write(struct intel_vgpu *vgpu,
> static int emulate_pci_bar_write(struct intel_vgpu *vgpu, unsigned int offset,
> void *p_data, unsigned int bytes)
> {
> - unsigned int bar_index =
> - (rounddown(offset, 8) % PCI_BASE_ADDRESS_0) / 8;
> u32 new = *(u32 *)(p_data);
> bool lo = IS_ALIGNED(offset, 8);
> u64 size;
> @@ -220,69 +218,57 @@ static int emulate_pci_bar_write(struct intel_vgpu *vgpu, unsigned int offset,
> bool mmio_enabled =
> vgpu_cfg_space(vgpu)[PCI_COMMAND] & PCI_COMMAND_MEMORY;
>
> - if (WARN_ON(bar_index >= INTEL_GVT_PCI_BAR_MAX))
> - return -EINVAL;
> -
> + /*
> + * Power-up software can determine how much address
> + * space the device requires by writing a value of
> + * all 1's to the register and then reading the value
> + * back. The device will return 0's in all don't-care
> + * address bits.
> + */
> if (new == 0xffffffff) {
> - /*
> - * Power-up software can determine how much address
> - * space the device requires by writing a value of
> - * all 1's to the register and then reading the value
> - * back. The device will return 0's in all don't-care
> - * address bits.
> - */
> - size = vgpu->cfg_space.bar[bar_index].size;
> - if (lo) {
> - new = rounddown(new, size);
> - } else {
> - u32 val = vgpu_cfg_space(vgpu)[rounddown(offset, 8)];
> - /* for 32bit mode bar it returns all-0 in upper 32
> - * bit, for 64bit mode bar it will calculate the
> - * size with lower 32bit and return the corresponding
> - * value
> + switch (offset) {
> + case PCI_BASE_ADDRESS_0:
> + case PCI_BASE_ADDRESS_1:
> + size = vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_GTTMMIO].size;
> + intel_vgpu_write_pci_bar(vgpu, offset,
> + size >> (lo ? 0 : 32), lo);
> + /*
> + * Untrap the BAR, since guest hasn't configured a
> + * valid GPA
> */
> - if (val & PCI_BASE_ADDRESS_MEM_TYPE_64)
> - new &= (~(size-1)) >> 32;
> - else
> - new = 0;
> - }
> - /*
> - * Unmapp & untrap the BAR, since guest hasn't configured a
> - * valid GPA
> - */
> - switch (bar_index) {
> - case INTEL_GVT_PCI_BAR_GTTMMIO:
> ret = trap_gttmmio(vgpu, false);
> break;
> - case INTEL_GVT_PCI_BAR_APERTURE:
> + case PCI_BASE_ADDRESS_2:
> + case PCI_BASE_ADDRESS_3:
> + size = vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].size;
> + intel_vgpu_write_pci_bar(vgpu, offset,
> + size >> (lo ? 0 : 32), lo);
> ret = map_aperture(vgpu, false);
> break;
> + default:
> + /* Unimplemented BARs */
> + intel_vgpu_write_pci_bar(vgpu, offset, 0x0, false);
> }
> - intel_vgpu_write_pci_bar(vgpu, offset, new, lo);
> } else {
> - /*
> - * Unmapp & untrap the old BAR first, since guest has
> - * re-configured the BAR
> - */
> - switch (bar_index) {
> - case INTEL_GVT_PCI_BAR_GTTMMIO:
> - ret = trap_gttmmio(vgpu, false);
> + switch (offset) {
> + case PCI_BASE_ADDRESS_0:
> + case PCI_BASE_ADDRESS_1:
> + /*
> + * Untrap the old BAR first, since guest has
> + * re-configured the BAR
> + */
> + trap_gttmmio(vgpu, false);
> + intel_vgpu_write_pci_bar(vgpu, offset, new, lo);
> + ret = trap_gttmmio(vgpu, mmio_enabled);
> break;
> - case INTEL_GVT_PCI_BAR_APERTURE:
> - ret = map_aperture(vgpu, false);
> + case PCI_BASE_ADDRESS_2:
> + case PCI_BASE_ADDRESS_3:
> + map_aperture(vgpu, false);
> + intel_vgpu_write_pci_bar(vgpu, offset, new, lo);
> + ret = map_aperture(vgpu, mmio_enabled);
> break;
> - }
> - intel_vgpu_write_pci_bar(vgpu, offset, new, lo);
> - /* Track the new BAR */
> - if (mmio_enabled) {
> - switch (bar_index) {
> - case INTEL_GVT_PCI_BAR_GTTMMIO:
> - ret = trap_gttmmio(vgpu, true);
> - break;
> - case INTEL_GVT_PCI_BAR_APERTURE:
> - ret = map_aperture(vgpu, true);
> - break;
> - }
> + default:
> + intel_vgpu_write_pci_bar(vgpu, offset, new, lo);
> }
> }
> return ret;
> @@ -313,10 +299,7 @@ int intel_vgpu_emulate_cfg_write(struct intel_vgpu *vgpu, unsigned int offset,
> }
>
> switch (rounddown(offset, 4)) {
> - case PCI_BASE_ADDRESS_0:
> - case PCI_BASE_ADDRESS_1:
> - case PCI_BASE_ADDRESS_2:
> - case PCI_BASE_ADDRESS_3:
> + case PCI_BASE_ADDRESS_0 ... PCI_BASE_ADDRESS_5:
> if (WARN_ON(!IS_ALIGNED(offset, 4)))
> return -EINVAL;
> return emulate_pci_bar_write(vgpu, offset, p_data, bytes);
> @@ -358,7 +341,6 @@ void intel_vgpu_init_cfg_space(struct intel_vgpu *vgpu,
> struct intel_gvt *gvt = vgpu->gvt;
> const struct intel_gvt_device_info *info = &gvt->device_info;
> u16 *gmch_ctl;
> - int i;
>
> memcpy(vgpu_cfg_space(vgpu), gvt->firmware.cfg_space,
> info->cfg_space_size);
> @@ -385,13 +367,13 @@ void intel_vgpu_init_cfg_space(struct intel_vgpu *vgpu,
> */
> memset(vgpu_cfg_space(vgpu) + PCI_BASE_ADDRESS_1, 0, 4);
> memset(vgpu_cfg_space(vgpu) + PCI_BASE_ADDRESS_3, 0, 4);
> + memset(vgpu_cfg_space(vgpu) + PCI_BASE_ADDRESS_4, 0, 8);
> memset(vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION, 0, 4);
>
> - for (i = 0; i < INTEL_GVT_MAX_BAR_NUM; i++) {
> - vgpu->cfg_space.bar[i].size = pci_resource_len(
> - gvt->dev_priv->drm.pdev, i * 2);
> - vgpu->cfg_space.bar[i].tracked = false;
> - }
> + vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_GTTMMIO].size =
> + pci_resource_len(gvt->dev_priv->drm.pdev, 0);
> + vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].size =
> + pci_resource_len(gvt->dev_priv->drm.pdev, 2);
> }
>
> /**
> --
> 2.7.4
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Thanks,
Changbin Du
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170818/7ee4246b/attachment.sig>
More information about the intel-gvt-dev
mailing list