> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Monday, December 21, 2020 5:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> Subject: [PATCH 11/40] net/virtio: validate features at bus level
>
> This patch provides a new callback for the bus type
> to validate negotiated features are compatible with it.
>
> Only user for now is PCI modern bus type, which implies
> that the device supports Virtio 1.0+.
>
> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> ---
> drivers/net/virtio/virtio_ethdev.c | 11 +++++------
> drivers/net/virtio/virtio_pci.c | 19 +++++++++++++++++++
> drivers/net/virtio/virtio_pci.h | 1 +
> drivers/net/virtio/virtio_user_ethdev.c | 7 +++++++
> 4 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 00aa38e4ef..91a93b2b6e 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1315,17 +1315,16 @@ virtio_negotiate_features(struct virtio_hw *hw,
> uint64_t req_features)
> PMD_INIT_LOG(DEBUG, "features after negotiate = %" PRIx64,
> hw->guest_features);
>
> - if (hw->bus_type == VIRTIO_BUS_PCI_MODERN && !vtpci_with_feature(hw,
> VIRTIO_F_VERSION_1)) {
> - PMD_INIT_LOG(ERR,
> - "VIRTIO_F_VERSION_1 features is not enabled.");
> + if (VTPCI_OPS(hw)->features_ok(hw) < 0) {
> + PMD_INIT_LOG(ERR, "Features not OK at bus level\n");
> return -1;
> }
>
> - if (hw->bus_type == VIRTIO_BUS_PCI_MODERN || hw->bus_type ==
> VIRTIO_BUS_USER) {
> + if (vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) {
> vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_FEATURES_OK);
> +
> if (!(vtpci_get_status(hw) & VIRTIO_CONFIG_STATUS_FEATURES_OK))
> {
> - PMD_INIT_LOG(ERR,
> - "failed to set FEATURES_OK status!");
> + PMD_INIT_LOG(ERR, "Failed to set FEATURES_OK status!");
> return -1;
> }
> }
> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
> index 599d8afa6b..3de7980b4f 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -151,6 +151,12 @@ legacy_set_features(struct virtio_hw *hw, uint64_t
> features)
> VIRTIO_PCI_GUEST_FEATURES);
> }
>
> +static int
> +legacy_features_ok(struct virtio_hw *hw __rte_unused)
> +{
> + return 0;
> +}
> +
> static uint8_t
> legacy_get_status(struct virtio_hw *hw)
> {
> @@ -259,6 +265,7 @@ const struct virtio_pci_ops legacy_ops = {
> .set_status = legacy_set_status,
> .get_features = legacy_get_features,
> .set_features = legacy_set_features,
> + .features_ok = legacy_features_ok,
> .get_isr = legacy_get_isr,
> .set_config_irq = legacy_set_config_irq,
> .set_queue_irq = legacy_set_queue_irq,
> @@ -332,6 +339,17 @@ modern_set_features(struct virtio_hw *hw, uint64_t
> features)
> &hw->common_cfg->guest_feature);
> }
>
> +static int
> +modern_features_ok(struct virtio_hw *hw)
> +{
> + if (!vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) {
> + PMD_INIT_LOG(ERR, "Version 1+ required with modern devices\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static uint8_t
> modern_get_status(struct virtio_hw *hw)
> {
> @@ -475,6 +493,7 @@ const struct virtio_pci_ops modern_ops = {
> .set_status = modern_set_status,
> .get_features = modern_get_features,
> .set_features = modern_set_features,
> + .features_ok = modern_features_ok,
> .get_isr = modern_get_isr,
> .set_config_irq = modern_set_config_irq,
> .set_queue_irq = modern_set_queue_irq,
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 4f7d0e479e..22c21e6896 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -227,6 +227,7 @@ struct virtio_pci_ops {
>
> uint64_t (*get_features)(struct virtio_hw *hw);
> void (*set_features)(struct virtio_hw *hw, uint64_t features);
> + int (*features_ok)(struct virtio_hw *hw);
>
> uint8_t (*get_isr)(struct virtio_hw *hw);
>
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index f9a2dbae71..c93e0e43f5 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -327,6 +327,12 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t
> features)
> dev->features = features & dev->device_features;
> }
>
> +static int
> +virtio_user_features_ok(struct virtio_hw *hw __rte_unused)
> +{
> + return 0;
> +}
> +
> static uint8_t
> virtio_user_get_isr(struct virtio_hw *hw __rte_unused)
> {
> @@ -479,6 +485,7 @@ const struct virtio_pci_ops virtio_user_ops = {
> .set_status = virtio_user_set_status,
> .get_features = virtio_user_get_features,
> .set_features = virtio_user_set_features,
> + .features_ok = virtio_user_features_ok,
> .get_isr = virtio_user_get_isr,
> .set_config_irq = virtio_user_set_config_irq,
> .set_queue_irq = virtio_user_set_queue_irq,
> --
> 2.29.2
Reviewed-by: Chenbo Xia <chenbo....@intel.com>
