On 12/5/24 06:36, Alex Williamson wrote:
> On Tue, 3 Dec 2024 21:35:41 +0800
> Tomita Moeko <tomitamo...@gmail.com> wrote:
>
>> Define the igd device generations according to i915 kernel driver to
>> avoid confusion, and adjust comment placement to clearly reflect the
>> relationship between ids and devices.
>>
>> The condition of how GTT stolen memory size is calculated is changed
>> accordingly as GGMS is in multiple of 2 starting from gen 8.
>>
>> Signed-off-by: Tomita Moeko <tomitamo...@gmail.com>
>> ---
>> hw/vfio/igd.c | 44 ++++++++++++++++++++++----------------------
>> 1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index 6ba3045bf3..2ede72d243 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -59,33 +59,33 @@
>> */
>> static int igd_gen(VFIOPCIDevice *vdev)
>> {
>> - if ((vdev->device_id & 0xfff) == 0xa84) {
>> - return 8; /* Broxton */
>> + /*
>> + * Device IDs for Broxton/Apollo Lake are 0x0a84, 0x1a84, 0x1a85, 0x5a84
>> + * and 0x5a85
>> + */
>
> Your comment from review of v1 would be useful here that we can't use
> the test below, at least for 0x0a84, because it conflicts with Haswell.
> I'd forgotten that.
Sure I will add it to comments in v3.
> Since we're being more strict about what we support now, it may make
> sense to list specific IDs rather than this sloppy match, as Corvin
> suggested, but that could be a follow-on. Thanks,
>
> Alex
I personally prefer do it later, it take some time to think a proper
way to maintain such a long device id list.
>> + if ((vdev->device_id & 0xffe) == 0xa84) {
>> + return 9;
>> }
>>
>> switch (vdev->device_id & 0xff00) {
>> - /* SandyBridge, IvyBridge, ValleyView, Haswell */
>> - case 0x0100:
>> - case 0x0400:
>> - case 0x0a00:
>> - case 0x0c00:
>> - case 0x0d00:
>> - case 0x0f00:
>> + case 0x0100: /* SandyBridge, IvyBridge */
>> return 6;
>> - /* BroadWell, CherryView, SkyLake, KabyLake */
>> - case 0x1600:
>> - case 0x1900:
>> - case 0x2200:
>> - case 0x5900:
>> + case 0x0400: /* Haswell */
>> + case 0x0a00: /* Haswell */
>> + case 0x0c00: /* Haswell */
>> + case 0x0d00: /* Haswell */
>> + case 0x0f00: /* Valleyview/Bay Trail */
>> + return 7;
>> + case 0x1600: /* Broadwell */
>> + case 0x2200: /* Cherryview */
>> return 8;
>> - /* CoffeeLake */
>> - case 0x3e00:
>> + case 0x1900: /* Skylake */
>> + case 0x5900: /* Kaby Lake */
>> + case 0x3e00: /* Coffee Lake */
>> return 9;
>> - /* ElkhartLake */
>> - case 0x4500:
>> + case 0x4500: /* Elkhart Lake */
>> return 11;
>> - /* TigerLake */
>> - case 0x9A00:
>> + case 0x9A00: /* Tiger Lake */
>> return 12;
>> }
>>
>> @@ -258,7 +258,7 @@ static int vfio_igd_gtt_max(VFIOPCIDevice *vdev)
>>
>> gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch));
>> ggms = (gmch >> (gen < 8 ? 8 : 6)) & 0x3;
>> - if (gen > 6) {
>> + if (gen >= 8) {
>> ggms = 1 << ggms;
>> }
>>
>> @@ -668,7 +668,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int
>> nr)
>>
>> /* Determine the size of stolen memory needed for GTT */
>> ggms_mb = (gmch >> (gen < 8 ? 8 : 6)) & 0x3;
>> - if (gen > 6) {
>> + if (gen >= 8) {
>> ggms_mb = 1 << ggms_mb;
>> }
>>
>
