lkml.org 
[lkml]   [2012]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] virtio-spec: split virtio-net device status filed into ro and rw byte
    On Mon, Mar 19, 2012 at 11:09:24AM +0800, Jason Wang wrote:
    > On 03/18/2012 08:22 PM, Michael S. Tsirkin wrote:
    > >On Fri, Mar 16, 2012 at 11:20:26PM +0800, Jason Wang wrote:
    > >>This patch splits the device status field of virtio-net into ro and rw
    > >>byte. This would simplify the implementation of both host and guest
    > >>and make the layout more clean. As VIRTIO_NET_S_ANNOUNCE is a rw bit,
    > >>it was moved to bit 8 (0x100).
    > >>
    > >>btw. looks like there's no implementation that depends on
    > >>VIRTIO_NET_S_ANNOUNCE, so the move is safe.
    > >>
    > >>Signed-off-by: Jason Wang<jasowang@redhat.com>
    > >Hmm, I know I proposed this myself, and I thought it will
    > >prevent problems if we will add more rw bits,
    > >but I missed the following race:
    > >
    > >host writes VIRTIO_NET_S_ANNOUNCE = 1, interrupt
    > >
    > > guest reads VIRTIO_NET_S_ANNOUNCE = 1
    > >
    > >host writes VIRTIO_NET_S_SOME_NEW_FIELD = 1, interrupt
    > >
    > > guest writes VIRTIO_NET_S_ANNOUNCE = 0
    > > VIRTIO_NET_S_SOME_NEW_FIELD is overwritten
    > >
    > > guest reads VIRTIO_NET_S_SOME_NEW_FIELD = 0
    > >
    > >How about making the new bit write 1 to clear?
    > >If we do, we can keep it where it is currently ...
    > >
    >
    > Didn't follow, do you mean to make VIRITO_NET_S_ANNOUNCE bit clear
    > on read? Looks like this can prevent the race and keep what
    > currently we have.

    Clear on read is evil, it makes debugging harder
    since you can't check the state without destroying it.
    Write 1 to clear means writing 0 to a bit does not
    change it, writing 1 clears it. This makes it possible
    to flip individual bits in config space without
    making Schrödinger's cat experiments.

    > >
    > >>---
    > >> virtio-0.9.4.lyx | 20 +++++++++++++++++++-
    > >> 1 files changed, 19 insertions(+), 1 deletions(-)
    > >>
    > >>diff --git a/virtio-0.9.4.lyx b/virtio-0.9.4.lyx
    > >>index 6c7bab1..ef3951c 100644
    > >>--- a/virtio-0.9.4.lyx
    > >>+++ b/virtio-0.9.4.lyx
    > >>@@ -58,6 +58,7 @@
    > >> \html_be_strict false
    > >> \author -608949062 "Rusty Russell,,,"
    > >> \author 1531152142 "pbonzini"
    > >>+\author 2090695081 "Jason"
    > >> \end_header
    > >>
    > >> \begin_body
    > >>@@ -4012,8 +4013,19 @@ configuration
    > >> layout Two configuration fields are currently defined.
    > >> The mac address field always exists (though is only valid if VIRTIO_NET_F_MAC
    > >> is set), and the status field only exists if VIRTIO_NET_F_STATUS is set.
    > >>+
    > >>+\change_inserted 2090695081 1331907586
    > >>+ The low byte of status field is read-only, guest write to this byte would
    > >>+ be ignored.
    > >>+ Currently only one bit is defined for this byte: VIRTIO_NET_S_LINK_UP.
    > >>+ The high byte of status field is read-writable.
    > >>+ Currently only one bit is defined for this byte: VIRTIO_NET_S_ANNOUNCE.
    > >>+
    > >>+\change_deleted 2090695081 1331907489
    > >> Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
    > >> and VIRTIO_NET_S_ANNOUNCE.
    > >>+
    > >>+\change_unchanged
    > >>
    > >> \begin_inset listings
    > >> inline false
    > >>@@ -4026,7 +4038,13 @@ status open
    > >>
    > >> \begin_layout Plain Layout
    > >>
    > >>-#define VIRTIO_NET_S_ANNOUNCE 2
    > >>+#define VIRTIO_NET_S_ANNOUNCE
    > >>+\change_inserted 2090695081 1331907493
    > >>+0x100
    > >>+\change_deleted 2090695081 1331907491
    > >>+2
    > >>+\change_unchanged
    > >>+
    > >> \end_layout
    > >>
    > >> \begin_layout Plain Layout
    > >--
    > >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > >the body of a message to majordomo@vger.kernel.org
    > >More majordomo info at http://vger.kernel.org/majordomo-info.html
    > >Please read the FAQ at http://www.tux.org/lkml/
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-19 09:25    [W:0.053 / U:30.384 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site