On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
<maxime.coque...@redhat.com> wrote:
> 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.
> 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]
> @@ -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)
> {
--
David Marchand
