[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 8/9] s390x/kvm: msi route fixup for non-pci
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v3 8/9] s390x/kvm: msi route fixup for non-pci |
Date: |
Wed, 26 Jul 2017 10:37:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 26.07.2017 10:25, Thomas Huth wrote:
> On 26.07.2017 10:20, Cornelia Huck wrote:
>> On Wed, 26 Jul 2017 09:09:06 +0200
>> Thomas Huth <address@hidden> 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 <address@hidden>
>>>> ---
>>>> 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
>
I'd vote for g_assert() or simply dropping it.
--
Thanks,
David
- Re: [Qemu-devel] [PATCH v3 4/9] s390x/pci: do not advertise pci on non-pci builds, (continued)
- [Qemu-devel] [PATCH v3 5/9] s390x/ccw: create s390 phb conditionally, Cornelia Huck, 2017/07/25
- [Qemu-devel] [PATCH v3 6/9] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/07/25
- [Qemu-devel] [PATCH v3 7/9] s390x/pci: fence off instructions for non-pci, Cornelia Huck, 2017/07/25
- [Qemu-devel] [PATCH v3 8/9] s390x/kvm: msi route fixup for non-pci, Cornelia Huck, 2017/07/25
- [Qemu-devel] [PATCH v3 9/9] s390x: refine pci dependencies, Cornelia Huck, 2017/07/25
- Re: [Qemu-devel] [PATCH v3 0/9] s390x: zPCI detangling, Christian Borntraeger, 2017/07/26