On Thu, Oct 27, 2022 at 2:47 PM Eugenio Perez Martin
<epere...@redhat.com> wrote:
>
> On Thu, Oct 27, 2022 at 6:32 AM Jason Wang <jasow...@redhat.com> wrote:
> >
> >
> > 在 2022/10/26 17:53, Eugenio Pérez 写道:
> > > Now that qemu can handle and emulate it if the vdpa backend does not
> > > support it we can offer it always.
> > >
> > > Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> >
> >
> > I may miss something but isn't more easier to simply remove the
> > _F_STATUS from vdpa_feature_bits[]?
> >
>
> How is that? if we remove it, the guest cannot ack it so it cannot
> access the net status, isn't it?

My understanding is that the bits stored in the vdpa_feature_bits[]
are the features that must be explicitly supported by the vhost
device. So if we remove _F_STATUS, Qemu vhost code won't validate if
vhost-vdpa device has this support:

uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
                            uint64_t features)
{
    const int *bit = feature_bits;
    while (*bit != VHOST_INVALID_FEATURE_BIT) {
        uint64_t bit_mask = (1ULL << *bit);
        if (!(hdev->features & bit_mask)) {
            features &= ~bit_mask;
        }
        bit++;
    }
    return features;
}

Thanks



>
> The goal with this patch series is to let the guest access the status
> always, even if the device doesn't support _F_STATUS.
>
> > Thanks
> >
> >
> > > ---
> > >   include/net/vhost-vdpa.h |  1 +
> > >   hw/net/vhost_net.c       | 16 ++++++++++++++--
> > >   net/vhost-vdpa.c         |  3 +++
> > >   3 files changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/net/vhost-vdpa.h b/include/net/vhost-vdpa.h
> > > index b81f9a6f2a..cfbcce6427 100644
> > > --- a/include/net/vhost-vdpa.h
> > > +++ b/include/net/vhost-vdpa.h
> > > @@ -17,5 +17,6 @@
> > >   struct vhost_net *vhost_vdpa_get_vhost_net(NetClientState *nc);
> > >
> > >   extern const int vdpa_feature_bits[];
> > > +extern const uint64_t vhost_vdpa_net_added_feature_bits;
> > >
> > >   #endif /* VHOST_VDPA_H */
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index d28f8b974b..7c15cc6e8f 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -109,10 +109,22 @@ static const int *vhost_net_get_feature_bits(struct 
> > > vhost_net *net)
> > >       return feature_bits;
> > >   }
> > >
> > > +static uint64_t vhost_net_add_feature_bits(struct vhost_net *net)
> > > +{
> > > +    if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> > > +        return vhost_vdpa_net_added_feature_bits;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >   uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t 
> > > features)
> > >   {
> > > -    return vhost_get_features(&net->dev, vhost_net_get_feature_bits(net),
> > > -            features);
> > > +    uint64_t ret = vhost_get_features(&net->dev,
> > > +                                      vhost_net_get_feature_bits(net),
> > > +                                      features);
> > > +
> > > +    return ret | vhost_net_add_feature_bits(net);
> > >   }
> > >   int vhost_net_get_config(struct vhost_net *net,  uint8_t *config,
> > >                            uint32_t config_len)
> > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > index 6d64000202..24d2857593 100644
> > > --- a/net/vhost-vdpa.c
> > > +++ b/net/vhost-vdpa.c
> > > @@ -99,6 +99,9 @@ static const uint64_t vdpa_svq_device_features =
> > >       BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> > >       BIT_ULL(VIRTIO_NET_F_STANDBY);
> > >
> > > +const uint64_t vhost_vdpa_net_added_feature_bits =
> > > +    BIT_ULL(VIRTIO_NET_F_STATUS);
> > > +
> > >   VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
> > >   {
> > >       VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> >
>


Reply via email to