[SLOF] [PATCH v1 17/27] virtio: make virtio_{get, read}_config 1.0 aware

[Alexey Kardashevskiy]

On 01/13/2016 10:17 PM, Nikunj A Dadhania wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>   lib/libvirtio/virtio.c | 49 +++++++++++++++++++++++++++++--------------------
>   1 file changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
> index 866ffb8..3bd8c67 100644
> --- a/lib/libvirtio/virtio.c
> +++ b/lib/libvirtio/virtio.c
> @@ -480,35 +480,43 @@ void virtio_get_host_features_long(struct virtio_device *dev, uint64_t *features
>   uint64_t virtio_get_config(struct virtio_device *dev, int offset, int size)
>   {
>   	uint64_t val = ~0ULL;
> +	uint32_t hi, lo;
>   	void *confbase;
>
> -	switch (dev->type) {
> -	 case VIRTIO_TYPE_PCI:
> +	if (dev->type != VIRTIO_TYPE_PCI)
> +		return val;
> +
> +	if (dev->is_modern)
> +		confbase = dev->device.addr;
> +	else
>   		confbase = dev->base+VIRTIOHDR_DEVICE_CONFIG;
> -		break;
> -	 default:
> -		return ~0ULL;
> -	}
> +
>   	switch (size) {
> -	 case 1:
> +	case 1:

Too many unrelated changes :-/


>   		val = ci_read_8(confbase+offset);
>   		break;
> -	 case 2:
> +	case 2:
>   		val = ci_read_16(confbase+offset);
> +		if (dev->is_modern)
> +			val = le16_to_cpu(val);
>   		break;
> -	 case 4:
> +	case 4:
>   		val = ci_read_32(confbase+offset);
> +		if (dev->is_modern)
> +			val = le32_to_cpu(val);
>   		break;
> -	 case 8:
> +	case 8:
>   		/* We don't support 8 bytes PIO accesses
>   		 * in qemu and this is all PIO
>   		 */
> -		val = ci_read_32(confbase+offset);
> -		val <<= 32;
> -		val |= ci_read_32(confbase+offset+4);
> +		lo = ci_read_32(confbase+offset);
> +		hi = ci_read_32(confbase+offset+4);


Does it have to be 2 separate 32bit reads or ci_read_64 can be used?


> +		if (dev->is_modern)
> +			val = (uint64_t)le32_to_cpu(hi) << 32 | le32_to_cpu(lo);
> +		else
> +			val = (uint64_t)hi << 32 | lo;


Instead of changing the entite "case 8", you could do:

	if (dev->is_modern)
		val = le64_to_cpu(val);


and that's it, is not it?


>   		break;
>   	}
> -
>   	return val;
>   }
>
> @@ -522,13 +530,14 @@ int __virtio_read_config(struct virtio_device *dev, void *dst,
>   	unsigned char *buf = dst;
>   	int i;
>
> -	switch (dev->type) {
> -	 case VIRTIO_TYPE_PCI:
> -		confbase = dev->base+VIRTIOHDR_DEVICE_CONFIG;
> -		break;
> -	 default:
> +	if (dev->type != VIRTIO_TYPE_PCI)
>   		return 0;
> -	}
> +
> +	if (dev->is_modern)
> +		confbase = dev->device.addr;
> +	else
> +		confbase = dev->base+VIRTIOHDR_DEVICE_CONFIG;
> +
>   	for (i = 0; i < len; i++)
>   		buf[i] = ci_read_8(confbase + offset + i);
>   	return len;
>


-- 
Alexey