diff mbox series

[v2,13/22] hw/pvrdma: Make sure PCI function 0 is vmxnet3

Message ID 20181108160818.5485-14-yuval.shaia@oracle.com (mailing list archive)
State New, archived
Headers show
Series Add support for RDMA MAD | expand

Commit Message

Yuval Shaia Nov. 8, 2018, 4:08 p.m. UTC
Guest driver enforces it, we should also.

Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
---
 hw/rdma/vmw/pvrdma.h      | 2 ++
 hw/rdma/vmw/pvrdma_main.c | 3 +++
 2 files changed, 5 insertions(+)

Comments

Marcel Apfelbaum Nov. 10, 2018, 6:27 p.m. UTC | #1
On 11/8/18 6:08 PM, Yuval Shaia wrote:
> Guest driver enforces it, we should also.
>
> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
> ---
>   hw/rdma/vmw/pvrdma.h      | 2 ++
>   hw/rdma/vmw/pvrdma_main.c | 3 +++
>   2 files changed, 5 insertions(+)
>
> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> index b019cb843a..10a3c4fb7c 100644
> --- a/hw/rdma/vmw/pvrdma.h
> +++ b/hw/rdma/vmw/pvrdma.h
> @@ -20,6 +20,7 @@
>   #include "hw/pci/pci.h"
>   #include "hw/pci/msix.h"
>   #include "chardev/char-fe.h"
> +#include "hw/net/vmxnet3_defs.h"
>   
>   #include "../rdma_backend_defs.h"
>   #include "../rdma_rm_defs.h"
> @@ -85,6 +86,7 @@ typedef struct PVRDMADev {
>       RdmaBackendDev backend_dev;
>       RdmaDeviceResources rdma_dev_res;
>       CharBackend mad_chr;
> +    VMXNET3State *func0;
>   } PVRDMADev;
>   #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
>   
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index ac8c092db0..fa6468d221 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -576,6 +576,9 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
>           return;
>       }
>   
> +    /* Break if not vmxnet3 device in slot 0 */
> +    dev->func0 = VMXNET3(pci_get_function_0(pdev));
> +

I don't see the error code flow in case VMXNET3 is not func 0.
Am I missing something?


Thanks,
Marcel

>       memdev_root = object_resolve_path("/objects", NULL);
>       if (memdev_root) {
>           object_child_foreach(memdev_root, pvrdma_check_ram_shared, &ram_shared);
Yuval Shaia Nov. 11, 2018, 7:45 a.m. UTC | #2
On Sat, Nov 10, 2018 at 08:27:44PM +0200, Marcel Apfelbaum wrote:
> 
> 
> On 11/8/18 6:08 PM, Yuval Shaia wrote:
> > Guest driver enforces it, we should also.
> > 
> > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
> > ---
> >   hw/rdma/vmw/pvrdma.h      | 2 ++
> >   hw/rdma/vmw/pvrdma_main.c | 3 +++
> >   2 files changed, 5 insertions(+)
> > 
> > diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> > index b019cb843a..10a3c4fb7c 100644
> > --- a/hw/rdma/vmw/pvrdma.h
> > +++ b/hw/rdma/vmw/pvrdma.h
> > @@ -20,6 +20,7 @@
> >   #include "hw/pci/pci.h"
> >   #include "hw/pci/msix.h"
> >   #include "chardev/char-fe.h"
> > +#include "hw/net/vmxnet3_defs.h"
> >   #include "../rdma_backend_defs.h"
> >   #include "../rdma_rm_defs.h"
> > @@ -85,6 +86,7 @@ typedef struct PVRDMADev {
> >       RdmaBackendDev backend_dev;
> >       RdmaDeviceResources rdma_dev_res;
> >       CharBackend mad_chr;
> > +    VMXNET3State *func0;
> >   } PVRDMADev;
> >   #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index ac8c092db0..fa6468d221 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -576,6 +576,9 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
> >           return;
> >       }
> > +    /* Break if not vmxnet3 device in slot 0 */
> > +    dev->func0 = VMXNET3(pci_get_function_0(pdev));
> > +
> 
> I don't see the error code flow in case VMXNET3 is not func 0.
> Am I missing something?

Yes, this is a dynamic cast that will break the process when fail to cast.

This is the error message that you will get in case that device on function
0 is not vmxnet3:

pvrdma_main.c:589:pvrdma_realize: Object 0x557b959841a0 is not an instance of type vmxnet3

> 
> 
> Thanks,
> Marcel
> 
> >       memdev_root = object_resolve_path("/objects", NULL);
> >       if (memdev_root) {
> >           object_child_foreach(memdev_root, pvrdma_check_ram_shared, &ram_shared);
>
Marcel Apfelbaum Nov. 17, 2018, 11:41 a.m. UTC | #3
On 11/11/18 9:45 AM, Yuval Shaia wrote:
> On Sat, Nov 10, 2018 at 08:27:44PM +0200, Marcel Apfelbaum wrote:
>>
>> On 11/8/18 6:08 PM, Yuval Shaia wrote:
>>> Guest driver enforces it, we should also.
>>>
>>> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
>>> ---
>>>    hw/rdma/vmw/pvrdma.h      | 2 ++
>>>    hw/rdma/vmw/pvrdma_main.c | 3 +++
>>>    2 files changed, 5 insertions(+)
>>>
>>> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
>>> index b019cb843a..10a3c4fb7c 100644
>>> --- a/hw/rdma/vmw/pvrdma.h
>>> +++ b/hw/rdma/vmw/pvrdma.h
>>> @@ -20,6 +20,7 @@
>>>    #include "hw/pci/pci.h"
>>>    #include "hw/pci/msix.h"
>>>    #include "chardev/char-fe.h"
>>> +#include "hw/net/vmxnet3_defs.h"
>>>    #include "../rdma_backend_defs.h"
>>>    #include "../rdma_rm_defs.h"
>>> @@ -85,6 +86,7 @@ typedef struct PVRDMADev {
>>>        RdmaBackendDev backend_dev;
>>>        RdmaDeviceResources rdma_dev_res;
>>>        CharBackend mad_chr;
>>> +    VMXNET3State *func0;
>>>    } PVRDMADev;
>>>    #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
>>> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
>>> index ac8c092db0..fa6468d221 100644
>>> --- a/hw/rdma/vmw/pvrdma_main.c
>>> +++ b/hw/rdma/vmw/pvrdma_main.c
>>> @@ -576,6 +576,9 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
>>>            return;
>>>        }
>>> +    /* Break if not vmxnet3 device in slot 0 */
>>> +    dev->func0 = VMXNET3(pci_get_function_0(pdev));
>>> +
>> I don't see the error code flow in case VMXNET3 is not func 0.
>> Am I missing something?
> Yes, this is a dynamic cast that will break the process when fail to cast.
>
> This is the error message that you will get in case that device on function
> 0 is not vmxnet3:
>
> pvrdma_main.c:589:pvrdma_realize: Object 0x557b959841a0 is not an instance of type vmxnet3

I am not sure we will see this error if QEMU is compiled in Release mode.
I think object_dynamic_cast_assert throws this error only if 
CONFIG_QOM_CAST_DEBUG
is set, and is possible the mentioned flag is not set in Release.

Thanks,
Marcel

>
>>
>> Thanks,
>> Marcel
>>
>>>        memdev_root = object_resolve_path("/objects", NULL);
>>>        if (memdev_root) {
>>>            object_child_foreach(memdev_root, pvrdma_check_ram_shared, &ram_shared);
Yuval Shaia Nov. 18, 2018, 9:16 a.m. UTC | #4
On Sat, Nov 17, 2018 at 01:41:41PM +0200, Marcel Apfelbaum wrote:
> 
> 
> On 11/11/18 9:45 AM, Yuval Shaia wrote:
> > On Sat, Nov 10, 2018 at 08:27:44PM +0200, Marcel Apfelbaum wrote:
> > > 
> > > On 11/8/18 6:08 PM, Yuval Shaia wrote:
> > > > Guest driver enforces it, we should also.
> > > > 
> > > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
> > > > ---
> > > >    hw/rdma/vmw/pvrdma.h      | 2 ++
> > > >    hw/rdma/vmw/pvrdma_main.c | 3 +++
> > > >    2 files changed, 5 insertions(+)
> > > > 
> > > > diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> > > > index b019cb843a..10a3c4fb7c 100644
> > > > --- a/hw/rdma/vmw/pvrdma.h
> > > > +++ b/hw/rdma/vmw/pvrdma.h
> > > > @@ -20,6 +20,7 @@
> > > >    #include "hw/pci/pci.h"
> > > >    #include "hw/pci/msix.h"
> > > >    #include "chardev/char-fe.h"
> > > > +#include "hw/net/vmxnet3_defs.h"
> > > >    #include "../rdma_backend_defs.h"
> > > >    #include "../rdma_rm_defs.h"
> > > > @@ -85,6 +86,7 @@ typedef struct PVRDMADev {
> > > >        RdmaBackendDev backend_dev;
> > > >        RdmaDeviceResources rdma_dev_res;
> > > >        CharBackend mad_chr;
> > > > +    VMXNET3State *func0;
> > > >    } PVRDMADev;
> > > >    #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
> > > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > > > index ac8c092db0..fa6468d221 100644
> > > > --- a/hw/rdma/vmw/pvrdma_main.c
> > > > +++ b/hw/rdma/vmw/pvrdma_main.c
> > > > @@ -576,6 +576,9 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
> > > >            return;
> > > >        }
> > > > +    /* Break if not vmxnet3 device in slot 0 */
> > > > +    dev->func0 = VMXNET3(pci_get_function_0(pdev));
> > > > +
> > > I don't see the error code flow in case VMXNET3 is not func 0.
> > > Am I missing something?
> > Yes, this is a dynamic cast that will break the process when fail to cast.
> > 
> > This is the error message that you will get in case that device on function
> > 0 is not vmxnet3:
> > 
> > pvrdma_main.c:589:pvrdma_realize: Object 0x557b959841a0 is not an instance of type vmxnet3
> 
> I am not sure we will see this error if QEMU is compiled in Release mode.
> I think object_dynamic_cast_assert throws this error only if
> CONFIG_QOM_CAST_DEBUG
> is set, and is possible the mentioned flag is not set in Release.
> 
> Thanks,
> Marcel

Done.
Thanks.

> 
> > 
> > > 
> > > Thanks,
> > > Marcel
> > > 
> > > >        memdev_root = object_resolve_path("/objects", NULL);
> > > >        if (memdev_root) {
> > > >            object_child_foreach(memdev_root, pvrdma_check_ram_shared, &ram_shared);
>
diff mbox series

Patch

diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
index b019cb843a..10a3c4fb7c 100644
--- a/hw/rdma/vmw/pvrdma.h
+++ b/hw/rdma/vmw/pvrdma.h
@@ -20,6 +20,7 @@ 
 #include "hw/pci/pci.h"
 #include "hw/pci/msix.h"
 #include "chardev/char-fe.h"
+#include "hw/net/vmxnet3_defs.h"
 
 #include "../rdma_backend_defs.h"
 #include "../rdma_rm_defs.h"
@@ -85,6 +86,7 @@  typedef struct PVRDMADev {
     RdmaBackendDev backend_dev;
     RdmaDeviceResources rdma_dev_res;
     CharBackend mad_chr;
+    VMXNET3State *func0;
 } PVRDMADev;
 #define PVRDMA_DEV(dev) OBJECT_CHECK(PVRDMADev, (dev), PVRDMA_HW_NAME)
 
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index ac8c092db0..fa6468d221 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -576,6 +576,9 @@  static void pvrdma_realize(PCIDevice *pdev, Error **errp)
         return;
     }
 
+    /* Break if not vmxnet3 device in slot 0 */
+    dev->func0 = VMXNET3(pci_get_function_0(pdev));
+
     memdev_root = object_resolve_path("/objects", NULL);
     if (memdev_root) {
         object_child_foreach(memdev_root, pvrdma_check_ram_shared, &ram_shared);