Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch
On 11/22/2013 10:37 PM, Vlad Yasevich wrote:
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.
They probably don't have the willing to share the internals of card. I
hacked the e1000 driver to check the subtle and undocumented behaviour
in the past. Maybe we can do the same.
Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch
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.
+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.
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.
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.
+break;
}
}
Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch
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;
}
}
Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch
On 11/21/2013 01:04 PM, 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
s/he/the/
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(-)
-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;
Does real hardware also auto-clear this bit when writing the low word
(thus forcing all drivers to write high word last to make a change take
effect)? Or are we risking the case of a driver that writes high word
first including the bit, and where real hardware just glitches over the
temporary half-written address where our emulation locks the user out
entirely? (Asked by someone that has not read the datasheet, so take
with a grain of salt)
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch
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;
+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);
+break;
}
}
--
1.8.4.2
