Re: [Qemu-devel] [PATCH v4 02/10] kvm: remove hard dependency on pci

[Pierre Morel <pmorel@linux.vnet.ibm.com>]

On 22/08/2017 11:04, Cornelia Huck wrote:
> On Mon, 21 Aug 2017 18:02:12 +0200
> Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:
> 
>> On 21/08/2017 11:16, Cornelia Huck wrote:
>>> The msi routing code in kvm calls some pci functions: provide
>>> some stubs to enable builds without pci.
>>>
>>> Also, to make this more obvious, guard them via a pci_available boolean
>>> (which also can be reused in other places).
>>>
>>> Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn")
>>> Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions")
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>    accel/kvm/kvm-all.c  |  6 +++---
>>>    hw/pci/pci-stub.c    | 15 +++++++++++++++
>>>    hw/pci/pci.c         |  2 ++
>>>    include/hw/pci/pci.h |  2 ++
>>>    4 files changed, 22 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>> index 46ce479dc3..f85553a851 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
>>>        int virq;
>>>        MSIMessage msg = {0, 0};
>>>
>>> -    if (dev) {
>>> +    if (pci_available && dev) {
>>>            msg = pci_get_msi_message(dev, vector);
>>>        }
>>
>> Hi Conny,
>>
>> I did not find a case where pci_available is false and dev is true.
>> but anyway, sure is sure.
> 
> It makes things more obvious, I think.
> 
>>> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
>>> index ecad664946..ace43821ca 100644
>>> --- a/hw/pci/pci-stub.c
>>> +++ b/hw/pci/pci-stub.c
>>> @@ -23,10 +23,12 @@
>>>    #include "monitor/monitor.h"
>>>    #include "qapi/qmp/qerror.h"
>>>    #include "hw/pci/pci.h"
>>> +#include "hw/pci/msi.h"
>>
>> I think you forgot that...
>>
>>>    #include "qmp-commands.h"
>>>    #include "hw/pci/msi.h"
>>
>> ...you already have it included here. Didn't you ?
> 
> Hum, once should really be enough.
> 
>> otherwise LGTM
> 
> Thanks. Can I translate that into a tag? :)
> 

Yes,

Reviewed-by: Pierre Morel<pmorel@linux.vnet.ibm.com>




-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany