linux-kernel - Re: [PATCH v3 0/6] virtio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150414115053.78189c71.cornelia.huck@de.ibm.com>
Date:	Tue, 14 Apr 2015 11:50:53 +0200
From:	Cornelia Huck <cornelia.huck@...ibm.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Rusty Russell <rusty@...tcorp.com.au>,
	linux-kernel@...r.kernel.org, Pawel Moll <pawel.moll@....com>,
	virtio-dev@...ts.oasis-open.org,
	virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v3 0/6] virtio_balloon: virtio 1 support

On Tue, 14 Apr 2015 11:21:11 +0200
"Michael S. Tsirkin" <mst@...hat.com> wrote:

> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index f81b220..164e0c2 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -52,15 +52,31 @@ struct virtio_balloon_config {
>  #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
>  #define VIRTIO_BALLOON_S_NR       6
> 
> +/*
> + * Memory statistics structure.
> + * Driver fills an array of these structures and passes to device.
> + *
> + * NOTE: fields are laid out in a way that would make compiler add padding
> + * between and after fields, so we have to use compiler-specific attributes to
> + * pack it, to disable this padding. This also often causes compiler to
> + * generate suboptimal code.
> + *
> + * We maintain this for backwards compatibility, but don't follow this example.

s/this/the existing statistics structure/

> + *
> + * Do something like the below instead:

If you want to implement a similar structure, do...

Just that nobody gets the idea that they are supposed to implement new
balloon statistics ;)

> + *     struct virtio_balloon_stat {
> + *         __virtio16 tag;
> + *         __u8 reserved[6];
> + *         __virtio64 val;
> + *     };

(...)

> @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx,
>  			       u16 tag, u64 val)
>  {
>  	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
> -	if (virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) {
> -		vb->stats[idx].tag = cpu_to_le32(tag);
> -		vb->stats[idx].val = cpu_to_le64(val);
> -	} else {
> -		vb->legacy_stats[idx].tag = tag;
> -		vb->legacy_stats[idx].val = val;
> -	}
> +	vb->stats[idx].tag = cpu_to_virtio16(vb->vdev, tag);

Seems that nobody seemed to care much about statistics...

> +	vb->stats[idx].val = cpu_to_virtio64(vb->vdev, val);
>  }
> 
>  #define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT)
> 

With these changes merged in:

Acked-by: Cornelia Huck <cornelia.huck@...ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/