[v4] s390x: zPCI detangling

[Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

Posted by Cornelia Huck 8 years, 6 months ago

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 <cohuck@redhat.com>
---
 target/s390x/kvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 9de165d8b1..d8db1cbf6e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2424,6 +2424,12 @@ 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)) {
+        /* How can we get here without pci enabled? */
+        g_assert(false);
+        return -ENODEV;
+    }
+
     pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
     if (!pbdev) {
         DPRINTF("add_msi_route no dev\n");
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

Posted by David Hildenbrand 8 years, 6 months ago

On 04.08.2017 13:29, 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 <cohuck@redhat.com>
> ---
>  target/s390x/kvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 9de165d8b1..d8db1cbf6e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2424,6 +2424,12 @@ 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)) {
> +        /* How can we get here without pci enabled? */
> +        g_assert(false);
> +        return -ENODEV;
> +    }
> +

/* How can we get here without pci enabled? */
g_assert(s390_has_feat(S390_FEAT_ZPCI));

?

>      pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
>      if (!pbdev) {
>          DPRINTF("add_msi_route no dev\n");
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

Posted by Cornelia Huck 8 years, 6 months ago

On Fri, 4 Aug 2017 15:18:14 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 04.08.2017 13:29, 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 <cohuck@redhat.com>
> > ---
> >  target/s390x/kvm.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 9de165d8b1..d8db1cbf6e 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -2424,6 +2424,12 @@ 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)) {
> > +        /* How can we get here without pci enabled? */
> > +        g_assert(false);
> > +        return -ENODEV;
> > +    }
> > +  
> 
> /* How can we get here without pci enabled? */
> g_assert(s390_has_feat(S390_FEAT_ZPCI));
> 
> ?

What I don't like about this is that it implicitly relies on
s390_pci_find_dev_by_idx() doing the correct thing if asserts are off -
and it is non-obvious that we'll get pbdev == NULL in that case.

> 
> >      pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
> >      if (!pbdev) {
> >          DPRINTF("add_msi_route no dev\n");
> >   
> 
>