Re: [Qemu-devel] [PATCH 4/7] e1000e: Fix PBACLR implementation

[Jason Wang]

On 2016年09月22日 17:01, Dmitry Fleytman wrote:
> > On 22 Sep 2016, at 09:40 AM, Jason Wang <address@hidden> wrote:
> >
> >
> >
> > On 2016年09月15日 14:14, Dmitry Fleytman wrote:
> > > This patch fixes incorrect check for
> > > interrypt type being used.
> > >
> > > PBSCLR register is valid for MSI-X only.
> > >
> > > See spec. 10.2.3.13 MSI—X PBA Clear
> > >
> > > Signed-off-by: Dmitry Fleytman <address@hidden>
> > > ---
> > >   hw/net/e1000e_core.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> > > index 22765cb..c38ed10 100644
> > > --- a/hw/net/e1000e_core.c
> > > +++ b/hw/net/e1000e_core.c
> > > @@ -2347,7 +2347,7 @@ e1000e_set_pbaclr(E1000ECore *core, int index, uint32_t 
> > > val)
> > >         core->mac[PBACLR] = val & E1000_PBACLR_VALID_MASK;
> > >   -    if (msix_enabled(core->owner)) {
> > > +    if (!msix_enabled(core->owner)) {
> > >           return;
> > >       }
> > >   
> > Spec also said "writing 0b has no effect". So we'd better implement this 
> > behavior too?
>
> Hi Jason,
>
> Not sure I understand you correctly.
>
> With current implementation, writing 0b does nothing
> except that it changes value of PBACLR being read.
>
> I just verified that physical device behaves exactly like this.
>
> Is this what you meant?
>
> ~Dmitry

Yes, then it looks fine to me.

Thanks