[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/5] Add support for receiving via receive buffe

From: Reimar Döffinger
Subject: Re: [Qemu-devel] [PATCH 3/5] Add support for receiving via receive buffers. While the Intel documentation claims this is unsupported, the OS X drivers use it, causing an assertion failure since rx buffer size is 0.
Date: Wed, 12 Aug 2009 02:35:53 +0200
User-agent: Mutt/1.5.20 (2009-06-14)

On Wed, Aug 12, 2009 at 03:04:17AM +0400, malc wrote:
> On Tue, 11 Aug 2009, Reimar D?ffinger wrote:
> > This adds support for some kind of receive buffers "flexible
> > mode". The Intel documentation as I read it claims that no such mode exist
> > for receive, but the fact that those (working with real hardware I
> > expect) drivers use it contradicts that...
> > This _definitely_ is necessary to support these AppleIntel8255x drivers.
> > 
> > Signed-off-by: Reimar D?ffinger <address@hidden>
> > ---
> >  hw/eepro100.c |   27 ++++++++++++++++++++++++---
> >  1 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/eepro100.c b/hw/eepro100.c
> > index f619d36..5620bc7 100644
> > --- a/hw/eepro100.c
> > +++ b/hw/eepro100.c
> > @@ -153,6 +153,14 @@ typedef struct {
> >      char packet[MAX_ETH_FRAME_SIZE + 4];
> >  } eepro100_rx_t;
> >  
> > +/* Receive buffer descriptor. */
> > +typedef struct {
> > +    uint32_t count;
> > +    uint32_t link;
> > +    uint32_t buffer;
> > +    uint32_t size;
> > +} eepro100_rbd_t;
> _t suffix aside (the whole file is quite tainted in this regard) and from
> skimming over the rest of the file it appears that this structure is
> shared with guest, even though it's neatly/(and for some definition of 
> natural)naturally aligned, i believe this is dodgy. Then again maybe just
> like _t the existing code already expects things to be rosy in this
> regard.

Well, I had mostly the same thoughts while working on this, but I
thought it preferable to keep with the current "style" even if it is
Discussing the best way to do it and then fixing all the code at a later
point seemed like a better idea (well, if the goal is to get this
applied without having to fix all those little issues with the current
code first).
Particularly in this case it is simple and still readable to just use
lduw_phys I think if you prefer...
I attached a proof-of concept patch that converts parts of it, but doing
the same for the tx_t and statistics_t structs would get a bit messy
without changing the overall code a bit.
Also I expect it to be full of stuff that can be discussed for weeks,
i.e. typical bikeshed material...
And maybe this is the kind of thing Stefan Weil might want to take care

P.S.: And I'd appreciate it if you wouldn't CC me, I am subscribed to the
list. Thanks.

Attachment: avoid_structs.diff
Description: Text document

reply via email to

[Prev in Thread] Current Thread [Next in Thread]