Re: [PATCH] e1000: set RX descriptor status in a separate operation

[Jason Wang]

On Mon, Jul 4, 2022 at 5:05 PM Ding Hui <dinghui@sangfor.com.cn> wrote:
>
> On 2022/7/4 15:10, Jason Wang wrote:
> >
> > 在 2022/6/29 17:40, Ding Hui 写道:
> >> @@ -1013,6 +1013,9 @@ e1000_receive_iov(NetClientState *nc, const
> >> struct iovec *iov, int iovcnt)
> >>               DBGOUT(RX, "Null RX descriptor!!\n");
> >>           }
> >>           pci_dma_write(d, base, &desc, sizeof(desc));
> >> +        desc.status |= (vlan_status | E1000_RXD_STAT_DD);
> >> +        pci_dma_write(d, base + offsetof(struct e1000_rx_desc, status),
> >> +                      &desc.status, sizeof(desc.status));
> >
> >
> > Good catch, but to be more safe, I wonder if we can simply use
> > stx_le_pci_dma() here?
> >
>
> Do you mean stb_le_pci_dma(d, base + offsetof(struct e1000_rx_desc,
> status), desc.status, MEMTXATTRS_UNSPECIFIED)?
>
> I checked both pci_dma_write() and stb_le_pci_dma(), there is no
> difference between them,

I think the difference is that the stx_xxx() can guarantee the atomicy
when it is allowed by the arch.

> but I'm not sure whether it is proper to mixed
> use address based api and value based api, besides that it's OK to me.
>
> Thanks for reply.

I apply this patch as is.

Thanks

>
> --
> Thanks,
> - Ding Hui
>
>