[SLOF] [PATCH v2 14/19] virtio: 1.0 features set/get register accessor

[Thomas Huth]

On 20.01.2016 13:10, Nikunj A Dadhania wrote:
> The new specification has a 64-bit feature register, support and mark
> the older routine as legacy.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>  lib/libvirtio/virtio.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  lib/libvirtio/virtio.h |  2 ++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
> index c2dd7dc..b88aa61 100644
> --- a/lib/libvirtio/virtio.c
> +++ b/lib/libvirtio/virtio.c
> @@ -314,26 +314,71 @@ void virtio_set_status(struct virtio_device *dev, int status)
>  
>  /**
>   * Set guest feature bits
> + * Legacy virtio interface
>   */
>  void virtio_set_guest_features(struct virtio_device *dev, int features)
>  
>  {
> -	if (dev->type == VIRTIO_TYPE_PCI) {
> +	if (dev->type == VIRTIO_TYPE_PCI && !dev->is_modern) {
>  		ci_write_32(dev->base+VIRTIOHDR_GUEST_FEATURES, bswap_32(features));
>  	}
>  }
>  
>  /**
>   * Get host feature bits
> + * Legacy virtio interface
>   */
>  void virtio_get_host_features(struct virtio_device *dev, int *features)
>  
>  {
> -	if (dev->type == VIRTIO_TYPE_PCI && features) {
> +	if (dev->type == VIRTIO_TYPE_PCI  && !dev->is_modern && features) {
>  		*features = bswap_32(ci_read_32(dev->base+VIRTIOHDR_DEVICE_FEATURES));
>  	}
>  }
>  
> +/**
> + * Set guest feature bits - virtio 1.0 interface
> + */
> +void virtio_set_guest_features_long(struct virtio_device *dev, uint64_t features)
> +{
> +	if (dev->type == VIRTIO_TYPE_PCI && dev->is_modern) {
> +		uint32_t f1 = (features >> 32) & 0xFFFFFFFF;
> +		uint32_t f0 = features & 0xFFFFFFFF;
> +		void *addr = dev->common.addr;
> +
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, drv_features_sel),
> +			    bswap_32(1));
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, drv_features),
> +			    bswap_32(f1));
> +
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, drv_features_sel),
> +			    bswap_32(0));
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, drv_features),
> +			    bswap_32(f0));
> +	}
> +}
> +
> +/**
> + * Get host feature bits - virtio 1.0 interface
> + */
> +void virtio_get_host_features_long(struct virtio_device *dev, uint64_t *features)
> +{
> +	if (dev->type == VIRTIO_TYPE_PCI && dev->is_modern && features) {
> +		uint32_t f0 = 0, f1 = 0;
> +		void *addr = dev->common.addr;
> +
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, dev_features_sel),
> +			    bswap_32(1));
> +		f1 = ci_read_32(addr +
> +				offset_of(struct virtio_dev_common, dev_features));
> +		ci_write_32(addr + offset_of(struct virtio_dev_common, dev_features_sel),
> +			    bswap_32(0));
> +		f0 = ci_read_32(addr +
> +				offset_of(struct virtio_dev_common, dev_features));
> +
> +		*features = ((uint64_t)le32_to_cpu(f1) << 32) | le32_to_cpu(f0);
> +	}
> +}

Could you please use cpu_to_le32() instead of bswap_32() in the above
two function? In the (unlikely) case that this code ever runs on a
little endian CPU, it would be better to use cpu_to_le32() right from
the start, I think...

 Thomas