Re: [PATCH] hw/vfio/pci-quirks: Fix broken legacy IGD passthrough

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 6/10/20 9:59 AM, Thomas Huth wrote:
> On 10/06/2020 09.53, Philippe Mathieu-Daudé wrote:
>> On 6/10/20 9:50 AM, Thomas Huth wrote:
>>> On 10/06/2020 09.31, Philippe Mathieu-Daudé wrote:
>>>> On 6/10/20 5:51 AM, Thomas Huth wrote:
>>>>> The #ifdef CONFIG_VFIO_IGD in pci-quirks.c is not working since the
>>>>> required header config-devices.h is not included, so that the legacy
>>>>> IGD passthrough is currently broken. Let's include the right header
>>>>> to fix this issue.
>>>>>
>>>>> Buglink: https://bugs.launchpad.net/qemu/+bug/1882784
>>>>> Fixes: 29d62771c81d8fd244a67c14a1d968c268d3fb19
>>>>>        ("hw/vfio: Move the IGD quirk code to a separate file")
>>>>
>>>> What about shorter tag?
>>>>
>>>> Fixes: 29d62771c81 ("vfio: Move the IGD quirk code to a separate file")
>>>
>>> I always forget whether to use the short or the long version for
>>> "Fixes:" ... this can hopefully be fixed (if necessary) when the patch
>>> gets picked up.
>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>  hw/vfio/pci-quirks.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>>>>> index f2155ddb1d..3158390db1 100644
>>>>> --- a/hw/vfio/pci-quirks.c
>>>>> +++ b/hw/vfio/pci-quirks.c
>>>>> @@ -11,6 +11,7 @@
>>>>>   */
>>>>>  
>>>>>  #include "qemu/osdep.h"
>>>>> +#include "config-devices.h"
>>>>
>>>> I've been wondering how we can avoid that mistake in the
>>>> future, but can find anything beside human review.
>>>
>>> I think in the long term, we should include config-devices.h in osdep.h,
>>> just like config-host.h and config-target.h is already included there.
>>> Everything else is just too confusing. But then we should also add a
>>> mechanism to poison the switches from config-devices.h in common code...
>>
>> We only need it for the files under hw/, right?
> 
> qtest.c in the main directory includes it, too.

hw/ and qtests could include "hw/hw.h" instead of affecting all the
codebase via "qemu/osdep.h".

*but* we have qdevs in target/ too... so either we think about cleaning
them, or we accept it is a lost cause (micro-architecture bits are tied
with hardware) and are doomed.

> 
>>> thus this likely needs some work and discussion of the patch first, so I
>>> think we should go with this change to pci-quirks.c here first to get
>>> the regression fixed ASAP.
>>
>> Sure, I'm not objecting that.
> 
> Sure, I just wanted to make sure that whoever (Alex?) picks up this
> patch does not wait for that other solution instead.
> 
>  Thomas
>