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

[Alex Williamson]

On Wed, 10 Jun 2020 09:59:35 +0200
Thomas Huth <thuth@redhat.com> 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.
> 
> >> 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.

Yep, I'll grab it, I've got one less-trivial patch to send a pull
request for anyway.  Thanks,

Alex