On Thu, Oct 27, 2022 at 2:47 PM Eugenio Perez Martin
<[email protected]> wrote:
>
> On Thu, Oct 27, 2022 at 6:32 AM Jason Wang <[email protected]> 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 <[email protected]>
> >
> >
> > 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);
> >
>