On Mon, May 24, 2010 at 01:01:34PM +0300, Michael S. Tsirkin wrote:
On Mon, May 24, 2010 at 12:55:30PM +0300, Gleb Natapov
On Mon, May 24, 2010 at 12:39:41PM +0300, Michael
S. Tsirkin wrote:
On Mon, May 24, 2010 at 12:35:21PM +0300, Gleb
On Thu, May 20, 2010 at 04:36:32PM +0300, Michael
S. Tsirkin wrote:
> diff --git a/src/virtio-ring.h b/src/virtio-ring.h
> index 3fb86fe..8b546f4 100644
> --- a/src/virtio-ring.h
> +++ b/src/virtio-ring.h
> @@ -9,8 +9,9 @@
> #define virt_to_phys(v) (unsigned long)(v)
> #define phys_to_virt(p) (void*)(p)
> -#define wmb() barrier()
> -#define mb() barrier()
> +/* Compiler barrier is enough as an x86 CPU does not reorder reads or writes */
> +#define smp_rmb() barrier()
> +#define smp_wmb() barrier()
I thought you were going to use real memory barriers (although we
concluded compiler barrier should be enough, but it's better to be safe
On x86, ops of the same kind are ordered by CPU, so all we need to do is
1. avoid depending on order between reads and writes
2. prevent the compiler from reordering memory accesses
So on Linux smp_rmb() and smp_wmb() are just compiler barriers too? (look
like they are).
On x86, yes.
Of course x86. We are talking about x86 BIOS code here ;)
In that case
lets use barrier() directly no need to
obfuscate the code.
I think it's better to use smp_rmb/smp_wmb because this code
tends to get copied around, it might end up on non-x86 platforms.
For these, barrier() would be wrong.
So they will have a bug. It will teach them to not blindly copy code.
Another reason is that it makes it easier to audit
it with linux virtio guest.
If you know that on x86 smp_(w|r)mb() are just barrier() it's still
easy. To be honest though this is the reason I left
virt_to_phys()/phys_to_virt(), so Kevin this is your call.