This commit implements a new function ifcvf_get_driver_feature() which read driver_features from virtio registers. To be less ambiguous, ifcvf_set_features() is renamed to ifcvf_set_driver_features(), and ifcvf_get_features() is renamed to ifcvf_get_dev_features() which returns the provisioned vDPA device features. Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> --- drivers/vdpa/ifcvf/ifcvf_base.c | 38 +++++++++++++++++---------------- drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++-- drivers/vdpa/ifcvf/ifcvf_main.c | 9 +++++--- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index 6c5650f73007..546e923bcd16 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -204,11 +204,29 @@ u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) return features; } -u64 ifcvf_get_features(struct ifcvf_hw *hw) +/* return provisioned vDPA dev features */ +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw) { return hw->dev_features; } +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw) +{ + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; + u32 features_lo, features_hi; + u64 features; + + vp_iowrite32(0, &cfg->device_feature_select); + features_lo = vp_ioread32(&cfg->guest_feature); + + vp_iowrite32(1, &cfg->device_feature_select); + features_hi = vp_ioread32(&cfg->guest_feature); + + features = ((u64)features_hi << 32) | features_lo; + + return features; +} + int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features) { if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) { @@ -275,7 +293,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64 offset, vp_iowrite8(*p++, hw->dev_cfg + offset + i); } -static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features) { struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; @@ -286,19 +304,6 @@ static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) vp_iowrite32(features >> 32, &cfg->guest_feature); } -static int ifcvf_config_features(struct ifcvf_hw *hw) -{ - ifcvf_set_features(hw, hw->req_features); - ifcvf_add_status(hw, VIRTIO_CONFIG_S_FEATURES_OK); - - if (!(ifcvf_get_status(hw) & VIRTIO_CONFIG_S_FEATURES_OK)) { - IFCVF_ERR(hw->pdev, "Failed to set FEATURES_OK status\n"); - return -EIO; - } - - return 0; -} - u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid) { struct ifcvf_lm_cfg __iomem *ifcvf_lm; @@ -387,9 +392,6 @@ int ifcvf_start_hw(struct ifcvf_hw *hw) ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE); ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER); - if (ifcvf_config_features(hw) < 0) - return -EINVAL; - ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK); return 0; diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index d545a9411143..cb19196c3ece 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -69,7 +69,6 @@ struct ifcvf_hw { phys_addr_t notify_base_pa; u32 notify_off_multiplier; u32 dev_type; - u64 req_features; u64 hw_features; /* provisioned device features */ u64 dev_features; @@ -122,7 +121,7 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw); void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); -u64 ifcvf_get_features(struct ifcvf_hw *hw); +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); @@ -137,4 +136,6 @@ int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area, u64 driver_area, u64 device_area); bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid); void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready); +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features); +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw); #endif /* _IFCVF_H_ */ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 1357c67014ab..4588484bd53d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -410,7 +410,7 @@ static u64 ifcvf_vdpa_get_device_features(struct vdpa_device *vdpa_dev) u64 features; if (type == VIRTIO_ID_NET || type == VIRTIO_ID_BLOCK) - features = ifcvf_get_features(vf); + features = ifcvf_get_dev_features(vf); else { features = 0; IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); @@ -428,7 +428,7 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat if (ret) return ret; - vf->req_features = features; + ifcvf_set_driver_features(vf, features); return 0; } @@ -436,8 +436,11 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat static u64 ifcvf_vdpa_get_driver_features(struct vdpa_device *vdpa_dev) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + u64 features; + + features = ifcvf_get_driver_features(vf); - return vf->req_features; + return features; } static u8 ifcvf_vdpa_get_status(struct vdpa_device *vdpa_dev) -- 2.39.1
Michael S. Tsirkin
2023-Apr-24 04:50 UTC
[PATCH 2/5] get_driver_features from virito registers
subj typo: virtio On Sat, Apr 01, 2023 at 04:48:51AM +0800, Zhu Lingshan wrote:> This commit implements a new function ifcvf_get_driver_feature() > which read driver_features from virtio registers. > > To be less ambiguous, ifcvf_set_features() is renamed to > ifcvf_set_driver_features(), and ifcvf_get_features() > is renamed to ifcvf_get_dev_features() which returns > the provisioned vDPA device features. > > Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> > --- > drivers/vdpa/ifcvf/ifcvf_base.c | 38 +++++++++++++++++---------------- > drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++-- > drivers/vdpa/ifcvf/ifcvf_main.c | 9 +++++--- > 3 files changed, 29 insertions(+), 23 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c > index 6c5650f73007..546e923bcd16 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.c > +++ b/drivers/vdpa/ifcvf/ifcvf_base.c > @@ -204,11 +204,29 @@ u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) > return features; > } > > -u64 ifcvf_get_features(struct ifcvf_hw *hw) > +/* return provisioned vDPA dev features */ > +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw) > { > return hw->dev_features; > } > > +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw) > +{ > + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; > + u32 features_lo, features_hi; > + u64 features; > + > + vp_iowrite32(0, &cfg->device_feature_select); > + features_lo = vp_ioread32(&cfg->guest_feature); > + > + vp_iowrite32(1, &cfg->device_feature_select); > + features_hi = vp_ioread32(&cfg->guest_feature); > + > + features = ((u64)features_hi << 32) | features_lo; > + > + return features; > +} > + > int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features) > { > if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) { > @@ -275,7 +293,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64 offset, > vp_iowrite8(*p++, hw->dev_cfg + offset + i); > } > > -static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) > +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features) > { > struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; > > @@ -286,19 +304,6 @@ static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) > vp_iowrite32(features >> 32, &cfg->guest_feature); > } > > -static int ifcvf_config_features(struct ifcvf_hw *hw) > -{ > - ifcvf_set_features(hw, hw->req_features); > - ifcvf_add_status(hw, VIRTIO_CONFIG_S_FEATURES_OK); > - > - if (!(ifcvf_get_status(hw) & VIRTIO_CONFIG_S_FEATURES_OK)) { > - IFCVF_ERR(hw->pdev, "Failed to set FEATURES_OK status\n"); > - return -EIO; > - } > - > - return 0; > -} > - > u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid) > { > struct ifcvf_lm_cfg __iomem *ifcvf_lm; > @@ -387,9 +392,6 @@ int ifcvf_start_hw(struct ifcvf_hw *hw) > ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE); > ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER); > > - if (ifcvf_config_features(hw) < 0) > - return -EINVAL; > - > ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK); > > return 0; > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h > index d545a9411143..cb19196c3ece 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.h > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h > @@ -69,7 +69,6 @@ struct ifcvf_hw { > phys_addr_t notify_base_pa; > u32 notify_off_multiplier; > u32 dev_type; > - u64 req_features; > u64 hw_features; > /* provisioned device features */ > u64 dev_features; > @@ -122,7 +121,7 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw); > void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); > void io_write64_twopart(u64 val, u32 *lo, u32 *hi); > void ifcvf_reset(struct ifcvf_hw *hw); > -u64 ifcvf_get_features(struct ifcvf_hw *hw); > +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw); > u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); > int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); > u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); > @@ -137,4 +136,6 @@ int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area, > u64 driver_area, u64 device_area); > bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid); > void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready); > +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features); > +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw); > #endif /* _IFCVF_H_ */ > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 1357c67014ab..4588484bd53d 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -410,7 +410,7 @@ static u64 ifcvf_vdpa_get_device_features(struct vdpa_device *vdpa_dev) > u64 features; > > if (type == VIRTIO_ID_NET || type == VIRTIO_ID_BLOCK) > - features = ifcvf_get_features(vf); > + features = ifcvf_get_dev_features(vf); > else { > features = 0; > IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); > @@ -428,7 +428,7 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat > if (ret) > return ret; > > - vf->req_features = features; > + ifcvf_set_driver_features(vf, features); > > return 0; > } > @@ -436,8 +436,11 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat > static u64 ifcvf_vdpa_get_driver_features(struct vdpa_device *vdpa_dev) > { > struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); > + u64 features; > + > + features = ifcvf_get_driver_features(vf); > > - return vf->req_features; > + return features; > } > > static u8 ifcvf_vdpa_get_status(struct vdpa_device *vdpa_dev) > -- > 2.39.1
? 2023/4/1 04:48, Zhu Lingshan ??:> This commit implements a new function ifcvf_get_driver_feature() > which read driver_features from virtio registers. > > To be less ambiguous, ifcvf_set_features() is renamed to > ifcvf_set_driver_features(), and ifcvf_get_features() > is renamed to ifcvf_get_dev_features() which returns > the provisioned vDPA device features. > > Signed-off-by: Zhu Lingshan <lingshan.zhu at intel.com> > --- > drivers/vdpa/ifcvf/ifcvf_base.c | 38 +++++++++++++++++---------------- > drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++-- > drivers/vdpa/ifcvf/ifcvf_main.c | 9 +++++--- > 3 files changed, 29 insertions(+), 23 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c > index 6c5650f73007..546e923bcd16 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.c > +++ b/drivers/vdpa/ifcvf/ifcvf_base.c > @@ -204,11 +204,29 @@ u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) > return features; > } > > -u64 ifcvf_get_features(struct ifcvf_hw *hw) > +/* return provisioned vDPA dev features */ > +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw) > { > return hw->dev_features; > } > > +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw) > +{ > + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; > + u32 features_lo, features_hi; > + u64 features; > + > + vp_iowrite32(0, &cfg->device_feature_select); > + features_lo = vp_ioread32(&cfg->guest_feature); > + > + vp_iowrite32(1, &cfg->device_feature_select); > + features_hi = vp_ioread32(&cfg->guest_feature); > + > + features = ((u64)features_hi << 32) | features_lo; > + > + return features; > +}This duplicates with the logic ifcvf_get_hw_features(), it would be simpler if we just do a rename. Thanks> + > int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features) > { > if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) { > @@ -275,7 +293,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64 offset, > vp_iowrite8(*p++, hw->dev_cfg + offset + i); > } > > -static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) > +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features) > { > struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; > > @@ -286,19 +304,6 @@ static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features) > vp_iowrite32(features >> 32, &cfg->guest_feature); > } > > -static int ifcvf_config_features(struct ifcvf_hw *hw) > -{ > - ifcvf_set_features(hw, hw->req_features); > - ifcvf_add_status(hw, VIRTIO_CONFIG_S_FEATURES_OK); > - > - if (!(ifcvf_get_status(hw) & VIRTIO_CONFIG_S_FEATURES_OK)) { > - IFCVF_ERR(hw->pdev, "Failed to set FEATURES_OK status\n"); > - return -EIO; > - } > - > - return 0; > -} > - > u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid) > { > struct ifcvf_lm_cfg __iomem *ifcvf_lm; > @@ -387,9 +392,6 @@ int ifcvf_start_hw(struct ifcvf_hw *hw) > ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE); > ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER); > > - if (ifcvf_config_features(hw) < 0) > - return -EINVAL; > - > ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK); > > return 0; > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h > index d545a9411143..cb19196c3ece 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_base.h > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h > @@ -69,7 +69,6 @@ struct ifcvf_hw { > phys_addr_t notify_base_pa; > u32 notify_off_multiplier; > u32 dev_type; > - u64 req_features; > u64 hw_features; > /* provisioned device features */ > u64 dev_features; > @@ -122,7 +121,7 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw); > void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); > void io_write64_twopart(u64 val, u32 *lo, u32 *hi); > void ifcvf_reset(struct ifcvf_hw *hw); > -u64 ifcvf_get_features(struct ifcvf_hw *hw); > +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw); > u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); > int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); > u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); > @@ -137,4 +136,6 @@ int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid, u64 desc_area, > u64 driver_area, u64 device_area); > bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid); > void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready); > +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features); > +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw); > #endif /* _IFCVF_H_ */ > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 1357c67014ab..4588484bd53d 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -410,7 +410,7 @@ static u64 ifcvf_vdpa_get_device_features(struct vdpa_device *vdpa_dev) > u64 features; > > if (type == VIRTIO_ID_NET || type == VIRTIO_ID_BLOCK) > - features = ifcvf_get_features(vf); > + features = ifcvf_get_dev_features(vf); > else { > features = 0; > IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); > @@ -428,7 +428,7 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat > if (ret) > return ret; > > - vf->req_features = features; > + ifcvf_set_driver_features(vf, features); > > return 0; > } > @@ -436,8 +436,11 @@ static int ifcvf_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 feat > static u64 ifcvf_vdpa_get_driver_features(struct vdpa_device *vdpa_dev) > { > struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); > + u64 features; > + > + features = ifcvf_get_driver_features(vf); > > - return vf->req_features; > + return features; > } > > static u8 ifcvf_vdpa_get_status(struct vdpa_device *vdpa_dev)