| Message ID | 20241203133548.38252-4-tomitamoeko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfio/igd: Enable legacy mode on more devices | expand |
On Tue, 2024-12-03 at 21:35 +0800, Tomita Moeko wrote: > CAUTION: External Email!! > Add helper functions igd_gtt_memory_size() and igd_stolen_size() for > calculating GTT stolen memory and Data stolen memory size in bytes, > and use macros to replace the hardware-related magic numbers for > better readability. > > Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com> > --- > hw/vfio/igd.c | 99 ++++++++++++++++++++++++++++----------------------- > 1 file changed, 55 insertions(+), 44 deletions(-) > > diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c > index 2ede72d243..b5bfdc6580 100644 > --- a/hw/vfio/igd.c > +++ b/hw/vfio/igd.c > @@ -106,6 +106,51 @@ typedef struct VFIOIGDQuirk { > #define IGD_BDSM 0x5c /* Base Data of Stolen Memory */ > #define IGD_BDSM_GEN11 0xc0 /* Base Data of Stolen Memory of gen 11 and later > */ > > +#define IGD_GMCH_GEN6_GMS_SHIFT 3 /* SNB_GMCH in i915 */ > +#define IGD_GMCH_GEN6_GMS_MASK 0x1f > +#define IGD_GMCH_GEN6_GGMS_SHIFT 8 > +#define IGD_GMCH_GEN6_GGMS_MASK 0x3 > +#define IGD_GMCH_GEN8_GMS_SHIFT 8 /* BDW_GMCH in i915 */ > +#define IGD_GMCH_GEN8_GMS_MASK 0xff > +#define IGD_GMCH_GEN8_GGMS_SHIFT 6 > +#define IGD_GMCH_GEN8_GGMS_MASK 0x3 > + > +static uint64_t igd_gtt_memory_size(int gen, uint16_t gmch) > +{ > + uint64_t ggms; > + > + if (gen < 8) { > + ggms = (gmch >> IGD_GMCH_GEN6_GGMS_SHIFT) & IGD_GMCH_GEN6_GGMS_MASK; > + } else { > + ggms = (gmch >> IGD_GMCH_GEN8_GGMS_SHIFT) & IGD_GMCH_GEN8_GGMS_MASK; > + ggms *= 2; > + } > + > + return ggms * MiB; > +} > + > +static uint64_t igd_stolen_memory_size(int gen, uint32_t gmch) > +{ > + uint64_t gms; > + > + if (gen < 8) { > + gms = (gmch >> IGD_GMCH_GEN6_GMS_SHIFT) & IGD_GMCH_GEN6_GMS_MASK; > + } else { > + gms = (gmch >> IGD_GMCH_GEN8_GMS_SHIFT) & IGD_GMCH_GEN8_GMS_MASK; > + } > + > + if (gen < 9) { > + return gms * 32 * MiB; > + } else { > + if (gms < 0xf0) { > + return gms * 32 * MiB; > + } else { > + return (gms - 0xf0 + 1) * 4 * MiB; > + } > + } > + > + return 0; > +} > > /* > * The rather short list of registers that we copy from the host devices. > @@ -254,17 +299,10 @@ static int vfio_pci_igd_lpc_init(VFIOPCIDevice *vdev, > static int vfio_igd_gtt_max(VFIOPCIDevice *vdev) > { > uint32_t gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, > sizeof(gmch)); > - int ggms, gen = igd_gen(vdev); > - > - gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch)); > - ggms = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; > - if (gen >= 8) { > - ggms = 1 << ggms; > - } > - > - ggms *= MiB; > + int gen = igd_gen(vdev); > + uint64_t ggms_size = igd_gtt_memory_size(gen, gmch); > > - return (ggms / (4 * KiB)) * (gen < 8 ? 4 : 8); > + return (ggms_size / (4 * KiB)) * (gen < 8 ? 4 : 8); > } > > /* > @@ -471,30 +509,6 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int > nr) > QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); > } > > -static int igd_get_stolen_mb(int gen, uint32_t gmch) > -{ > - int gms; > - > - if (gen < 8) { > - gms = (gmch >> 3) & 0x1f; > - } else { > - gms = (gmch >> 8) & 0xff; > - } > - > - if (gen < 9) { > - if (gms > 0x10) { > - error_report("Unsupported IGD GMS value 0x%x", gms); > - return 0; > - } > - return gms * 32; > - } else { > - if (gms < 0xf0) > - return gms * 32; > - else > - return (gms - 0xf0) * 4 + 4; > - } > -} > - > void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > { > g_autofree struct vfio_region_info *rom = NULL; > @@ -504,7 +518,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int > nr) > VFIOQuirk *quirk; > VFIOIGDQuirk *igd; > PCIDevice *lpc_bridge; > - int i, ret, ggms_mb, gms_mb = 0, gen; > + int i, ret, gen; > + uint64_t ggms_size, gms_size; > uint64_t *bdsm_size; > uint32_t gmch; > uint16_t cmd_orig, cmd; > @@ -666,13 +681,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int > nr) > > QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); > > - /* Determine the size of stolen memory needed for GTT */ > - ggms_mb = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; > - if (gen >= 8) { > - ggms_mb = 1 << ggms_mb; > - } > - > - gms_mb = igd_get_stolen_mb(gen, gmch); > + ggms_size = igd_gtt_memory_size(gen, gmch); > + gms_size = igd_stolen_memory_size(gen, gmch); > > /* > * Request reserved memory for stolen memory via fw_cfg. VM firmware > @@ -683,7 +693,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int > nr) > * config offset 0x5C. > */ > bdsm_size = g_malloc(sizeof(*bdsm_size)); > - *bdsm_size = cpu_to_le64((ggms_mb + gms_mb) * MiB); > + *bdsm_size = cpu_to_le64(ggms_size + gms_size); > fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size", > bdsm_size, sizeof(*bdsm_size)); > > @@ -734,5 +744,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int > nr) > vdev- > >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAAADIUcZasZ36mwAAABM0 > 01Yig6LNmON7mS202pDuIRRNT- > tIucgsQHKYe8WitBfoANcxBM2L6i4Sg4fLLMkXL_LDVUSEswEqsunuGsAr4DjgOogXtNMivhsyeaj9 > xYl9AgydV4QqrKMV29P7y3uAuqQcYz1GacVJRg1 ); > } > > - trace_vfio_pci_igd_bdsm_enabled(vdev- > >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAAADIUcZasZ36mwAAABM0 > 01Yig6LNmON7mS202pDuIRRNT- > tIucgsQHKYe8WitBfoANcxBM2L6i4Sg4fLLMkXL_LDVUSEswEqsunuGsAr4DjgOogXtNMivhsyeaj9 > xYl9AgydV4QqrKMV29P7y3uAuqQcYz1GacVJRg1 , ggms_mb + gms_mb); > + trace_vfio_pci_igd_bdsm_enabled(vdev- > >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAAADIUcZasZ36mwAAABM0 > 01Yig6LNmON7mS202pDuIRRNT- > tIucgsQHKYe8WitBfoANcxBM2L6i4Sg4fLLMkXL_LDVUSEswEqsunuGsAr4DjgOogXtNMivhsyeaj9 > xYl9AgydV4QqrKMV29P7y3uAuqQcYz1GacVJRg1 , > + (ggms_size + gms_size) / MiB); > } Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
On Tue, 3 Dec 2024 21:35:42 +0800 Tomita Moeko <tomitamoeko@gmail.com> wrote: > Add helper functions igd_gtt_memory_size() and igd_stolen_size() for > calculating GTT stolen memory and Data stolen memory size in bytes, > and use macros to replace the hardware-related magic numbers for > better readability. > > Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com> > --- > hw/vfio/igd.c | 99 ++++++++++++++++++++++++++++----------------------- > 1 file changed, 55 insertions(+), 44 deletions(-) > > diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c > index 2ede72d243..b5bfdc6580 100644 > --- a/hw/vfio/igd.c > +++ b/hw/vfio/igd.c > @@ -106,6 +106,51 @@ typedef struct VFIOIGDQuirk { > #define IGD_BDSM 0x5c /* Base Data of Stolen Memory */ > #define IGD_BDSM_GEN11 0xc0 /* Base Data of Stolen Memory of gen 11 and later */ > > +#define IGD_GMCH_GEN6_GMS_SHIFT 3 /* SNB_GMCH in i915 */ > +#define IGD_GMCH_GEN6_GMS_MASK 0x1f > +#define IGD_GMCH_GEN6_GGMS_SHIFT 8 > +#define IGD_GMCH_GEN6_GGMS_MASK 0x3 > +#define IGD_GMCH_GEN8_GMS_SHIFT 8 /* BDW_GMCH in i915 */ > +#define IGD_GMCH_GEN8_GMS_MASK 0xff > +#define IGD_GMCH_GEN8_GGMS_SHIFT 6 > +#define IGD_GMCH_GEN8_GGMS_MASK 0x3 > + > +static uint64_t igd_gtt_memory_size(int gen, uint16_t gmch) > +{ > + uint64_t ggms; > + > + if (gen < 8) { > + ggms = (gmch >> IGD_GMCH_GEN6_GGMS_SHIFT) & IGD_GMCH_GEN6_GGMS_MASK; > + } else { > + ggms = (gmch >> IGD_GMCH_GEN8_GGMS_SHIFT) & IGD_GMCH_GEN8_GGMS_MASK; > + ggms *= 2; I tried to ask whether this was a bug fix in the previous iteration, but I think it was overlooked. These are not the same: ggms *= 2; ggms = 1 << ggms; Comparing the 4th processor generation datasheet[1] to that of the 5th generation processor[2], I see: 4th: 0x0 = No Preallocated Memory 0x1 = 1MB of Preallocated Memory 0x2 = 2MB of Preallocated Memory 0x3 = Reserved 5th: 0x0 = No Preallocated Memory 0x1 = 2MB of Preallocated Memory 0x2 = 4MB of Preallocated Memory 0x3 = 8MB of Preallocated Memory In your update, we'd get ggms values of 2, 4, and 6, which is incorrect. The existing code is correct to use the ggms value as the exponent, 2^1 = 2, 2^2 = 4, 2^3 = 8. It does seem there's a bug at zero though since 2^0 = 1, so maybe we should pull out the fix to a separate patch: if (ggms) { ggms = 1 << ggms; } Thanks, Alex [1]https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-desktop-vol-2-datasheet.pdf (3.1.13) [2]https://www.intel.com/content/www/us/en/content-details/330835/5th-generation-intel-core-processor-family-volume-2-of-2-datasheet.html (3.1.13) > + } > + > + return ggms * MiB; > +} > + > +static uint64_t igd_stolen_memory_size(int gen, uint32_t gmch) > +{ > + uint64_t gms; > + > + if (gen < 8) { > + gms = (gmch >> IGD_GMCH_GEN6_GMS_SHIFT) & IGD_GMCH_GEN6_GMS_MASK; > + } else { > + gms = (gmch >> IGD_GMCH_GEN8_GMS_SHIFT) & IGD_GMCH_GEN8_GMS_MASK; > + } > + > + if (gen < 9) { > + return gms * 32 * MiB; > + } else { > + if (gms < 0xf0) { > + return gms * 32 * MiB; > + } else { > + return (gms - 0xf0 + 1) * 4 * MiB; > + } > + } > + > + return 0; > +} > > /* > * The rather short list of registers that we copy from the host devices. > @@ -254,17 +299,10 @@ static int vfio_pci_igd_lpc_init(VFIOPCIDevice *vdev, > static int vfio_igd_gtt_max(VFIOPCIDevice *vdev) > { > uint32_t gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch)); > - int ggms, gen = igd_gen(vdev); > - > - gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch)); > - ggms = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; > - if (gen >= 8) { > - ggms = 1 << ggms; > - } > - > - ggms *= MiB; > + int gen = igd_gen(vdev); > + uint64_t ggms_size = igd_gtt_memory_size(gen, gmch); > > - return (ggms / (4 * KiB)) * (gen < 8 ? 4 : 8); > + return (ggms_size / (4 * KiB)) * (gen < 8 ? 4 : 8); > } > > /* > @@ -471,30 +509,6 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) > QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); > } > > -static int igd_get_stolen_mb(int gen, uint32_t gmch) > -{ > - int gms; > - > - if (gen < 8) { > - gms = (gmch >> 3) & 0x1f; > - } else { > - gms = (gmch >> 8) & 0xff; > - } > - > - if (gen < 9) { > - if (gms > 0x10) { > - error_report("Unsupported IGD GMS value 0x%x", gms); > - return 0; > - } > - return gms * 32; > - } else { > - if (gms < 0xf0) > - return gms * 32; > - else > - return (gms - 0xf0) * 4 + 4; > - } > -} > - > void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > { > g_autofree struct vfio_region_info *rom = NULL; > @@ -504,7 +518,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > VFIOQuirk *quirk; > VFIOIGDQuirk *igd; > PCIDevice *lpc_bridge; > - int i, ret, ggms_mb, gms_mb = 0, gen; > + int i, ret, gen; > + uint64_t ggms_size, gms_size; > uint64_t *bdsm_size; > uint32_t gmch; > uint16_t cmd_orig, cmd; > @@ -666,13 +681,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > > QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); > > - /* Determine the size of stolen memory needed for GTT */ > - ggms_mb = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; > - if (gen >= 8) { > - ggms_mb = 1 << ggms_mb; > - } > - > - gms_mb = igd_get_stolen_mb(gen, gmch); > + ggms_size = igd_gtt_memory_size(gen, gmch); > + gms_size = igd_stolen_memory_size(gen, gmch); > > /* > * Request reserved memory for stolen memory via fw_cfg. VM firmware > @@ -683,7 +693,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > * config offset 0x5C. > */ > bdsm_size = g_malloc(sizeof(*bdsm_size)); > - *bdsm_size = cpu_to_le64((ggms_mb + gms_mb) * MiB); > + *bdsm_size = cpu_to_le64(ggms_size + gms_size); > fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size", > bdsm_size, sizeof(*bdsm_size)); > > @@ -734,5 +744,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > vdev->vbasedev.name); > } > > - trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb); > + trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, > + (ggms_size + gms_size) / MiB); > }
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c index 2ede72d243..b5bfdc6580 100644 --- a/hw/vfio/igd.c +++ b/hw/vfio/igd.c @@ -106,6 +106,51 @@ typedef struct VFIOIGDQuirk { #define IGD_BDSM 0x5c /* Base Data of Stolen Memory */ #define IGD_BDSM_GEN11 0xc0 /* Base Data of Stolen Memory of gen 11 and later */ +#define IGD_GMCH_GEN6_GMS_SHIFT 3 /* SNB_GMCH in i915 */ +#define IGD_GMCH_GEN6_GMS_MASK 0x1f +#define IGD_GMCH_GEN6_GGMS_SHIFT 8 +#define IGD_GMCH_GEN6_GGMS_MASK 0x3 +#define IGD_GMCH_GEN8_GMS_SHIFT 8 /* BDW_GMCH in i915 */ +#define IGD_GMCH_GEN8_GMS_MASK 0xff +#define IGD_GMCH_GEN8_GGMS_SHIFT 6 +#define IGD_GMCH_GEN8_GGMS_MASK 0x3 + +static uint64_t igd_gtt_memory_size(int gen, uint16_t gmch) +{ + uint64_t ggms; + + if (gen < 8) { + ggms = (gmch >> IGD_GMCH_GEN6_GGMS_SHIFT) & IGD_GMCH_GEN6_GGMS_MASK; + } else { + ggms = (gmch >> IGD_GMCH_GEN8_GGMS_SHIFT) & IGD_GMCH_GEN8_GGMS_MASK; + ggms *= 2; + } + + return ggms * MiB; +} + +static uint64_t igd_stolen_memory_size(int gen, uint32_t gmch) +{ + uint64_t gms; + + if (gen < 8) { + gms = (gmch >> IGD_GMCH_GEN6_GMS_SHIFT) & IGD_GMCH_GEN6_GMS_MASK; + } else { + gms = (gmch >> IGD_GMCH_GEN8_GMS_SHIFT) & IGD_GMCH_GEN8_GMS_MASK; + } + + if (gen < 9) { + return gms * 32 * MiB; + } else { + if (gms < 0xf0) { + return gms * 32 * MiB; + } else { + return (gms - 0xf0 + 1) * 4 * MiB; + } + } + + return 0; +} /* * The rather short list of registers that we copy from the host devices. @@ -254,17 +299,10 @@ static int vfio_pci_igd_lpc_init(VFIOPCIDevice *vdev, static int vfio_igd_gtt_max(VFIOPCIDevice *vdev) { uint32_t gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch)); - int ggms, gen = igd_gen(vdev); - - gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch)); - ggms = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; - if (gen >= 8) { - ggms = 1 << ggms; - } - - ggms *= MiB; + int gen = igd_gen(vdev); + uint64_t ggms_size = igd_gtt_memory_size(gen, gmch); - return (ggms / (4 * KiB)) * (gen < 8 ? 4 : 8); + return (ggms_size / (4 * KiB)) * (gen < 8 ? 4 : 8); } /* @@ -471,30 +509,6 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); } -static int igd_get_stolen_mb(int gen, uint32_t gmch) -{ - int gms; - - if (gen < 8) { - gms = (gmch >> 3) & 0x1f; - } else { - gms = (gmch >> 8) & 0xff; - } - - if (gen < 9) { - if (gms > 0x10) { - error_report("Unsupported IGD GMS value 0x%x", gms); - return 0; - } - return gms * 32; - } else { - if (gms < 0xf0) - return gms * 32; - else - return (gms - 0xf0) * 4 + 4; - } -} - void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) { g_autofree struct vfio_region_info *rom = NULL; @@ -504,7 +518,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) VFIOQuirk *quirk; VFIOIGDQuirk *igd; PCIDevice *lpc_bridge; - int i, ret, ggms_mb, gms_mb = 0, gen; + int i, ret, gen; + uint64_t ggms_size, gms_size; uint64_t *bdsm_size; uint32_t gmch; uint16_t cmd_orig, cmd; @@ -666,13 +681,8 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next); - /* Determine the size of stolen memory needed for GTT */ - ggms_mb = (gmch >> (gen < 8 ? 8 : 6)) & 0x3; - if (gen >= 8) { - ggms_mb = 1 << ggms_mb; - } - - gms_mb = igd_get_stolen_mb(gen, gmch); + ggms_size = igd_gtt_memory_size(gen, gmch); + gms_size = igd_stolen_memory_size(gen, gmch); /* * Request reserved memory for stolen memory via fw_cfg. VM firmware @@ -683,7 +693,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) * config offset 0x5C. */ bdsm_size = g_malloc(sizeof(*bdsm_size)); - *bdsm_size = cpu_to_le64((ggms_mb + gms_mb) * MiB); + *bdsm_size = cpu_to_le64(ggms_size + gms_size); fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size", bdsm_size, sizeof(*bdsm_size)); @@ -734,5 +744,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) vdev->vbasedev.name); } - trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb); + trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, + (ggms_size + gms_size) / MiB); }
Add helper functions igd_gtt_memory_size() and igd_stolen_size() for calculating GTT stolen memory and Data stolen memory size in bytes, and use macros to replace the hardware-related magic numbers for better readability. Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com> --- hw/vfio/igd.c | 99 ++++++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 44 deletions(-)