Re: [PATCH] xen/pass-through: don't create needless register group

[Chuck Zmudzinski <brchuckz@netscape.net>]

On 6/17/22 9:07 AM, Anthony PERARD wrote:
> On Fri, Jun 10, 2022 at 12:23:35PM -0400, Chuck Zmudzinski wrote:
>> Currently we are creating a register group for the Intel IGD OpRegion
>> for every device we pass through, but the XEN_PCI_INTEL_OPREGION
>> register group is only valid for an Intel IGD. Add a check to make
>> sure the device is an Intel IGD and a check that the administrator has
>> enabled gfx_passthru in the xl domain configuration. Require both checks
>> to be true before creating the register group. Use the existing
>> is_igd_vga_passthrough() function to check for a graphics device from
>> any vendor and that the administrator enabled gfx_passthru in the xl
>> domain configuration, but further require that the vendor be Intel,
>> because only Intel IGD devices have an Intel OpRegion. These are the
>> same checks hvmloader and libxl do to determine if the Intel OpRegion
>> needs to be mapped into the guest's memory.
>>
>> Signed-off-by: Chuck Zmudzinski <brchuckz@aol.com>
>> ---
>>   hw/xen/xen_pt_config_init.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
>> index c5c4e943a8..ffd915654c 100644
>> --- a/hw/xen/xen_pt_config_init.c
>> +++ b/hw/xen/xen_pt_config_init.c
>> @@ -2037,6 +2037,10 @@ void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp)
>>            * therefore the size should be 0xff.
>>            */
> Could you move that comment? I think it would make more sense to comment
> the "reg_grp_offset=XEN_PCI_INTEL_OPREGION" line now that the `if` block
> also skip setting up the group on non-intel devices.

OK. I just e-mailed interested parties v2 that moves the comment
and mentions that the comment is moved in the commit message.

Best Regards,

Chuck

>
>>           if (xen_pt_emu_reg_grps[i].grp_id == XEN_PCI_INTEL_OPREGION) {
>> +            if (!is_igd_vga_passthrough(&s->real_device) ||
>> +                s->real_device.vendor_id != PCI_VENDOR_ID_INTEL) {
>> +                continue;
>> +            }
>>               reg_grp_offset = XEN_PCI_INTEL_OPREGION;
>>           }
> Thanks,
>