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 <vyase...@redhat.com> >> --- >> 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; >> } >> } >> >