diff mbox series

hw/vfio/pci-quirks: Fix broken legacy IGD passthrough

Message ID 20200610035102.20192-1-thuth@redhat.com
State New
Headers show
Series hw/vfio/pci-quirks: Fix broken legacy IGD passthrough | expand

Commit Message

Thomas Huth June 10, 2020, 3:51 a.m. UTC
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")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/vfio/pci-quirks.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Philippe Mathieu-Daudé June 10, 2020, 7:31 a.m. UTC | #1
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")

> 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.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  #include "exec/memop.h"
>  #include "qemu/units.h"
>  #include "qemu/error-report.h"
>
Thomas Huth June 10, 2020, 7:50 a.m. UTC | #2
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...
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.

 Thomas
Philippe Mathieu-Daudé June 10, 2020, 7:53 a.m. UTC | #3
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?

> 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.

> 
>  Thomas
>
Thomas Huth June 10, 2020, 7:59 a.m. UTC | #4
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.

 Thomas
Philippe Mathieu-Daudé June 10, 2020, 8:25 a.m. UTC | #5
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
>
Thomas Huth June 10, 2020, 9:03 a.m. UTC | #6
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.

 Thomas
Philippe Mathieu-Daudé June 10, 2020, 10:08 a.m. UTC | #7
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.
Laurent Vivier June 10, 2020, 1:16 p.m. UTC | #8
Le 10/06/2020 à 09:50, Thomas Huth a écrit :
> 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.

you can add in your ~/.gitconfig

[alias]
        showfix = log -1 --format=fixes
[pretty]
        fixes = Fixes: %h (\"%s\")%nCc: %ae

and then:

$ git showfix 29d62771c81d8fd244a67c14a1d968c268d3fb19
Fixes: 29d62771c81d ("hw/vfio: Move the IGD quirk code to a separate file")

Thanks,
Laurent
Thomas Huth June 10, 2020, 2:10 p.m. UTC | #9
On 10/06/2020 15.16, Laurent Vivier wrote:
> Le 10/06/2020 à 09:50, Thomas Huth a écrit :
>> 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.
> 
> you can add in your ~/.gitconfig
> 
> [alias]
>         showfix = log -1 --format=fixes
> [pretty]
>         fixes = Fixes: %h (\"%s\")%nCc: %ae
> 
> and then:
> 
> $ git showfix 29d62771c81d8fd244a67c14a1d968c268d3fb19
> Fixes: 29d62771c81d ("hw/vfio: Move the IGD quirk code to a separate file")

That's nice, thanks! I've put it now in my gitconfig :-)

 Thomas
Alex Williamson June 11, 2020, 5:35 p.m. UTC | #10
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
Philippe Mathieu-Daudé Aug. 12, 2020, 8:26 a.m. UTC | #11
On Wed, Jun 10, 2020 at 3:17 PM Laurent Vivier <laurent@vivier.eu> wrote:
> Le 10/06/2020 à 09:50, Thomas Huth a écrit :
> > 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.
>
> you can add in your ~/.gitconfig
>
> [alias]
>         showfix = log -1 --format=fixes
> [pretty]
>         fixes = Fixes: %h (\"%s\")%nCc: %ae

From git-show (1):

           ·   %ae: author email
           ·   %aE: author email (respecting .mailmap, see
git-shortlog(1) or git-blame(1))

Since we have .mailmap, the %aE form is highly recommended:

With %ae:
$ git showfix 3e3d5815cbc1fdbf33adbe55f63ede3acead886f
Fixes: 3e3d5815cb ("NAND Flash memory emulation and ECC calculation
helpers for use by NAND controllers.")
Cc: balrog@c046a42c-6fe2-441c-8c8c-71466251a162

With %aE:
$ git showfix 3e3d5815cbc1fdbf33adbe55f63ede3acead886f
Fixes: 3e3d5815cb ("NAND Flash memory emulation and ECC calculation
helpers for use by NAND controllers.")
Cc: balrogg@gmail.com

>
> and then:
>
> $ git showfix 29d62771c81d8fd244a67c14a1d968c268d3fb19
> Fixes: 29d62771c81d ("hw/vfio: Move the IGD quirk code to a separate file")
>
> Thanks,
> Laurent
>
diff mbox series

Patch

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"
 #include "exec/memop.h"
 #include "qemu/units.h"
 #include "qemu/error-report.h"