From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34316) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vjrs1-0004eh-L7 for qemu-devel@nongnu.org; Fri, 22 Nov 2013 09:38:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vjrrt-0003LE-UE for qemu-devel@nongnu.org; Fri, 22 Nov 2013 09:38:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10495) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vjrrt-0003L5-LG for qemu-devel@nongnu.org; Fri, 22 Nov 2013 09:38:01 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAMEc05v000439 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 22 Nov 2013 09:38:00 -0500 Message-ID: <528F6C46.8070405@redhat.com> Date: Fri, 22 Nov 2013 09:37:58 -0500 From: Vlad Yasevich MIME-Version: 1.0 References: <1385064260-20962-1-git-send-email-vyasevic@redhat.com> <1385064260-20962-2-git-send-email-vyasevic@redhat.com> <528F2836.8010800@redhat.com> In-Reply-To: <528F2836.8010800@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch Reply-To: vyasevic@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: alex.williamson@redhat.com, akong@redhat.com, stefanha@redhat.com, mst@redhat.com On 11/22/2013 04:47 AM, Jason Wang wrote: > On 11/22/2013 04:04 AM, Vlad Yasevich wrote: >> e1000 provides a E1000_RAH_AV bit on every complete write >> to the Receive Address Register. We can use this bit >> 2 ways: >> 1) To trigger HMP notifications. When the bit is set the >> mac address is fully set and we can update the HMP. >> >> 2) We can turn off he bit on the write to low order bits of >> the Receive Address Register, so that we would not try >> to match received traffic to this address when it is >> not completely set. >> >> Signed-off-by: Vlad Yasevich >> --- >> hw/net/e1000.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/hw/net/e1000.c b/hw/net/e1000.c >> index ae63591..82978ea 100644 >> --- a/hw/net/e1000.c >> +++ b/hw/net/e1000.c >> @@ -1106,10 +1106,19 @@ mac_writereg(E1000State *s, int index, uint32_t val) >> >> s->mac_reg[index] = val; >> >> - if (index == RA || index == RA + 1) { >> + switch (index) { >> + case RA: >> + /* Mask off AV bit on the write of the low dword. The write of >> + * the high dword will set the bit. This way a half-written >> + * mac address will not be used to filter on rx. >> + */ >> + s->mac_reg[RA+1] &= ~E1000_RAH_AV; > > If a stupid driver write high dword first, it won't receive any packets. I need to ping Intel guys again. I asked them what happens when only the low register is set, but haven't heard back. >> + break; >> + case (RA + 1): >> macaddr[0] = cpu_to_le32(s->mac_reg[RA]); >> macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]); >> qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t *)macaddr); > > Guest may invalid the mac address by clearing the AV bit through writing > to high dword. So this may notify a wrong mac address. In this case, testing for the AV bit would solve this issue. > > Generally, we could teset the AV bit before notification, and try to do > the this on both high and low dword. This obeys specs and > receive_filter() above. This will not really help since the AV bit would already be set from the prior mac address. So, if a stupid driver writes just the low word, the AV bit would already be set. > > If we don't want half-written status, driver should clear AV bit before > each writing of new mac address. But looks like linux and freebsd does > not do this. But the window is really small and harmless. We can emulate this. Thanks for the idea. -vlad >> + break; >> } >> } >> >