* [Qemu-devel] [PATCH RFC] e1000: defer packets until BM enabled
@ 2014-12-01 16:50 Michael S. Tsirkin
2014-12-01 18:01 ` Gabriel Somlo
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2014-12-01 16:50 UTC (permalink / raw)
To: qemu-devel, akong
Cc: Gabriel Somlo, Gerd Hoffmann, Stefan Hajnoczi, Alexander Graf
Some guests seem to set BM for e1000 after
enabling RX.
If packets arrive in the window, device is wedged.
Probably works by luck on real hardware, work around
this by making can_receive depend on BM.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/net/e1000.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index e33a4da..34625ac 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -923,7 +923,9 @@ e1000_can_receive(NetClientState *nc)
E1000State *s = qemu_get_nic_opaque(nc);
return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
- (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
+ (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
+ (s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
+ e1000_has_rxbufs(s, 1);
}
static uint64_t rx_desc_base(E1000State *s)
@@ -1529,6 +1531,20 @@ static NetClientInfo net_e1000_info = {
.link_status_changed = e1000_set_link_status,
};
+static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
+ uint32_t val, int len)
+{
+ E1000State *d = E1000(dev);
+
+ pci_default_write_config(pci_dev, address, val, len);
+
+ if (range_covers_byte(address, len, PCI_COMMAND) &&
+ (pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
+}
+
+
static int pci_e1000_init(PCIDevice *pci_dev)
{
DeviceState *dev = DEVICE(pci_dev);
@@ -1539,6 +1555,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
int i;
uint8_t *macaddr;
+ pci_dev->config_write = e1000_write_config;
+
pci_conf = pci_dev->config;
/* TODO: RST# value should be 0, PCI spec 6.2.4 */
--
MST
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] e1000: defer packets until BM enabled
2014-12-01 16:50 [Qemu-devel] [PATCH RFC] e1000: defer packets until BM enabled Michael S. Tsirkin
@ 2014-12-01 18:01 ` Gabriel Somlo
2014-12-01 18:05 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Somlo @ 2014-12-01 18:01 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel@nongnu.org, akong@redhat.com
Cc: Gerd Hoffmann, Stefan Hajnoczi, Alexander Graf
Hi Michael,
I had to make some small changes to get this patch to build successfully,
see inline below:
On Monday, December 01, 2014 11:50am, Michael S. Tsirkin [mst@redhat.com] wrote:
>
> Some guests seem to set BM for e1000 after
> enabling RX.
> If packets arrive in the window, device is wedged.
> Probably works by luck on real hardware, work around
> this by making can_receive depend on BM.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/net/e1000.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index e33a4da..34625ac 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -923,7 +923,9 @@ e1000_can_receive(NetClientState *nc)
> E1000State *s = qemu_get_nic_opaque(nc);
>
> return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
> - (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
> + (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
> + (s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
> + e1000_has_rxbufs(s, 1);
> }
>
> static uint64_t rx_desc_base(E1000State *s)
> @@ -1529,6 +1531,20 @@ static NetClientInfo net_e1000_info = {
> .link_status_changed = e1000_set_link_status,
> };
>
> +static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
> + uint32_t val, int len)
> +{
> + E1000State *d = E1000(dev);
s/dev/pci_dev/
> +
> + pci_default_write_config(pci_dev, address, val, len);
> +
> + if (range_covers_byte(address, len, PCI_COMMAND) &&
requires #include "qemu/range.h" at the top of e1000.c
> + (pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> + qemu_flush_queued_packets(qemu_get_queue(s->nic));
s/s->nic/d->nic/
> + }
> +}
> +
> +
> static int pci_e1000_init(PCIDevice *pci_dev)
> {
> DeviceState *dev = DEVICE(pci_dev);
> @@ -1539,6 +1555,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
> int i;
> uint8_t *macaddr;
>
> + pci_dev->config_write = e1000_write_config;
> +
> pci_conf = pci_dev->config;
>
> /* TODO: RST# value should be 0, PCI spec 6.2.4 */
> --
With this, I can confirm everything still works fine on both Mavericks and
F21-beta-live. So:
Tested-by: Gabriel Somlo <somlo@cmu.edu>
Regards,
--Gabriel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] e1000: defer packets until BM enabled
2014-12-01 18:01 ` Gabriel Somlo
@ 2014-12-01 18:05 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2014-12-01 18:05 UTC (permalink / raw)
To: Gabriel Somlo
Cc: Alexander Graf, akong@redhat.com, qemu-devel@nongnu.org,
Stefan Hajnoczi, Gerd Hoffmann
On Mon, Dec 01, 2014 at 06:01:18PM +0000, Gabriel Somlo wrote:
> Hi Michael,
>
> I had to make some small changes to get this patch to build successfully,
> see inline below:
Ouch, looks like I sent out a stale version:
git commit
build+edit
git format-patch (without git commit)
Happens to me now and again :(
Sorry.
> On Monday, December 01, 2014 11:50am, Michael S. Tsirkin [mst@redhat.com] wrote:
> >
> > Some guests seem to set BM for e1000 after
> > enabling RX.
> > If packets arrive in the window, device is wedged.
> > Probably works by luck on real hardware, work around
> > this by making can_receive depend on BM.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > hw/net/e1000.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> > index e33a4da..34625ac 100644
> > --- a/hw/net/e1000.c
> > +++ b/hw/net/e1000.c
> > @@ -923,7 +923,9 @@ e1000_can_receive(NetClientState *nc)
> > E1000State *s = qemu_get_nic_opaque(nc);
> >
> > return (s->mac_reg[STATUS] & E1000_STATUS_LU) &&
> > - (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
> > + (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
> > + (s->parent_obj.config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
> > + e1000_has_rxbufs(s, 1);
> > }
> >
> > static uint64_t rx_desc_base(E1000State *s)
> > @@ -1529,6 +1531,20 @@ static NetClientInfo net_e1000_info = {
> > .link_status_changed = e1000_set_link_status,
> > };
> >
> > +static void e1000_write_config(PCIDevice *pci_dev, uint32_t address,
> > + uint32_t val, int len)
> > +{
> > + E1000State *d = E1000(dev);
>
> s/dev/pci_dev/
>
> > +
> > + pci_default_write_config(pci_dev, address, val, len);
> > +
> > + if (range_covers_byte(address, len, PCI_COMMAND) &&
>
> requires #include "qemu/range.h" at the top of e1000.c
>
> > + (pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> > + qemu_flush_queued_packets(qemu_get_queue(s->nic));
>
> s/s->nic/d->nic/
>
> > + }
> > +}
> > +
> > +
> > static int pci_e1000_init(PCIDevice *pci_dev)
> > {
> > DeviceState *dev = DEVICE(pci_dev);
> > @@ -1539,6 +1555,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
> > int i;
> > uint8_t *macaddr;
> >
> > + pci_dev->config_write = e1000_write_config;
> > +
> > pci_conf = pci_dev->config;
> >
> > /* TODO: RST# value should be 0, PCI spec 6.2.4 */
> > --
>
> With this, I can confirm everything still works fine on both Mavericks and
> F21-beta-live. So:
>
> Tested-by: Gabriel Somlo <somlo@cmu.edu>
>
> Regards,
> --Gabriel
Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-01 18:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-01 16:50 [Qemu-devel] [PATCH RFC] e1000: defer packets until BM enabled Michael S. Tsirkin
2014-12-01 18:01 ` Gabriel Somlo
2014-12-01 18:05 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).