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

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

On 6/10/20 11:03 AM, Thomas Huth wrote:
> On 10/06/2020 10.25, Philippe Mathieu-Daudé wrote:
>> 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".
> 
> I don't think that's a good idea - in that case, you have to make sure
> to include hw/hw.h everywhere again, so you don't gain that much
> compared to including config-devices.h directly everywhere. osdep.h is
> our header that has to be included everywhere, so if we want to make
> sure that these defines are available everywhere, we have to include it
> from osdep.h.
> Apart from that, hw/hw.h just contains one more prototype - which likely
> should be renamed to cpu_hw_error() and moved to a cpu header instead,
> so that we can finally delete hw/hw.h completely.

Yes, this discussion implied the "hw/hw.h" cleanup talked elsewhere is done.