On 26.07.2017 10:20, Cornelia Huck wrote:
> On Wed, 26 Jul 2017 09:09:06 +0200
> Thomas Huth <th...@redhat.com> wrote:
>
>> On 25.07.2017 17:33, Cornelia Huck wrote:
>>> If we don't provide pci, we cannot have a pci device for which we
>>> have to translate to adapter routes: just return -ENODEV.
>>>
>>> Signed-off-by: Cornelia Huck <coh...@redhat.com>
>>> ---
>>> target/s390x/kvm.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index dc3f940b95..fb3e21a3a4 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2424,6 +2424,11 @@ int kvm_arch_fixup_msi_route(struct
>>> kvm_irq_routing_entry *route,
>>> uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;
>>>
>>> + if (!s390_has_feat(S390_FEAT_ZPCI)) {
>>> + DPRINTF("fixup_msi_route on non-pci machine?!\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
>>> if (!pbdev) {
>>> DPRINTF("add_msi_route no dev\n");
>>>
>>
>> Is this additional check really needed here? I'd rather expect
>> s390_pci_find_dev_by_idx() to return NULL here already, so we should
>> already be fine, shouldn't we?
>
> Yes, the end result is the same, but (1) better safe than sorry and (2)
> I can add a debug print here.
>
> I had actually considered throwing an error here, as this function
> really should not be called for !pci. Opinions?
At least the current DPRINTF will go unnoticed in 99% of all cases since
it is not compiled in by default. So I'd say either do a proper
error_report() or even g_assert() here, or simply drop the patch.
Thomas
