[04/40] net/virtio: introduce PCI device metadata

  This patch initiate refactoring of Virtio PCI, by introducing
a new device structure for PCI-specific metadata.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 2 +-
 drivers/net/virtio/virtio_pci.h    | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
  

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, December 21, 2020 5:13 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
> amorenoz@redhat.com; david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 04/40] net/virtio: introduce PCI device metadata
> 
> This patch initiate refactoring of Virtio PCI, by introducing
> a new device structure for PCI-specific metadata.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 2 +-
>  drivers/net/virtio/virtio_pci.h    | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 3ace25ac80..99a5a1bb88 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -2151,7 +2151,7 @@ static int eth_virtio_pci_probe(struct rte_pci_driver
> *pci_drv __rte_unused,
>  	if (vdpa == 1)
>  		return 1;
> 
> -	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct virtio_hw),
> +	return rte_eth_dev_pci_generic_probe(pci_dev, sizeof(struct
> virtio_pci_dev),
>  		eth_virtio_dev_init);
>  }
> 
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index b35a596169..8d3dc0e22e 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -289,6 +289,11 @@ struct virtio_hw {
>  	struct virtqueue **vqs;
>  };
> 
> +struct virtio_pci_dev {
> +	struct virtio_hw hw;
> +};
> +
> +#define virtio_pci_get_dev(hw) container_of(hw, struct virtio_pci_dev, hw)
> 
>  /*
>   * While virtio_hw is stored in shared memory, this structure stores
> --
> 2.29.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch initiate refactoring of Virtio PCI, by introducing
> a new device structure for PCI-specific metadata.

This works, but this patch seems artificial.

The eth_virtio_dev_init expects dev->data->dev_private to be a virtio_hw object.
You can introduce this later in the series when really needed.
  

On 1/5/21 10:16 PM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch initiate refactoring of Virtio PCI, by introducing
>> a new device structure for PCI-specific metadata.
> 
> This works, but this patch seems artificial.
> 
> The eth_virtio_dev_init expects dev->data->dev_private to be a virtio_hw object.
> You can introduce this later in the series when really needed.
> 
> 
I propose to squash it into patch 5, which moves PCI specific init to a
dedicated file.

Is that OK for you?

Thanks,
Maxime
  

On Thu, Jan 14, 2021 at 12:05 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> On 1/5/21 10:16 PM, David Marchand wrote:
> > On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
> > <maxime.coquelin@redhat.com> wrote:
> >>
> >> This patch initiate refactoring of Virtio PCI, by introducing
> >> a new device structure for PCI-specific metadata.
> >
> > This works, but this patch seems artificial.
> >
> > The eth_virtio_dev_init expects dev->data->dev_private to be a virtio_hw object.
> > You can introduce this later in the series when really needed.
> >
> >
> I propose to squash it into patch 5, which moves PCI specific init to a
> dedicated file.
>
> Is that OK for you?

Or leave this commit like this but add a check on the virtio_hw field
being the first field of the bus specific objects.
Something like:
RTE_BUILD_BUG_ON(offsetof(struct virtio_pci_dev, hw) != 0);

+ the same with virtio_user in the relevant patch.
  

On 1/14/21 3:40 PM, David Marchand wrote:
> On Thu, Jan 14, 2021 at 12:05 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>> On 1/5/21 10:16 PM, David Marchand wrote:
>>> On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
>>> <maxime.coquelin@redhat.com> wrote:
>>>>
>>>> This patch initiate refactoring of Virtio PCI, by introducing
>>>> a new device structure for PCI-specific metadata.
>>>
>>> This works, but this patch seems artificial.
>>>
>>> The eth_virtio_dev_init expects dev->data->dev_private to be a virtio_hw object.
>>> You can introduce this later in the series when really needed.
>>>
>>>
>> I propose to squash it into patch 5, which moves PCI specific init to a
>> dedicated file.
>>
>> Is that OK for you?
> 
> Or leave this commit like this but add a check on the virtio_hw field
> being the first field of the bus specific objects.
> Something like:
> RTE_BUILD_BUG_ON(offsetof(struct virtio_pci_dev, hw) != 0);
> 
> + the same with virtio_user in the relevant patch.
> 

I like the idea, will be done in v2.

Thanks for the suggestion,
Maxime