On 1/6/21 10:33 AM, David Marchand wrote:
> 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.
Yes, maybe overkill. I will remove.
Thanks,
Maxime
>> 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)
>> {
>
>
