On 1/6/21 10:33 AM, David Marchand wrote:

On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
[off-list ref] wrote:

quoted

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");

We have a log which gives more context in the modern features_ok() callback.
I don't think we need both log messages.

Yes, maybe overkill. I will remove.

Thanks,
Maxime

quoted

                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

[snip]

quoted

@@ -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)
 {