On Fri, Aug 06, 2021 at 12:06:04PM +0200, Patrick Wildt wrote:
> On Fri, Aug 06, 2021 at 11:05:53AM +0200, Patrick Wildt wrote:
> > Am Thu, Aug 05, 2021 at 02:33:01PM +0200 schrieb Jan Klemkow:
> > > Hi,
> > >
> > > The following diff removes useless code from the driver. As discussed
> > > here [1] and committed there [2], the hypervisor doesn't do anything
> > > with the data structures. We even just set NULL to the pointer since
> > > the initial commit of vmx(4). So, I guess it better to remove all of
> > > these. The variables are bzero'd in vmxnet3_dma_allocmem() anyway.
> > >
> > > OK?
> >
> > My main concern was if the structs are getting zeroed correctly, but
> > they do, so that's fine.
> >
> > That said, it looks like Linux sets the pointer to ~0ULL, not 0. Should
> > we follow Linux' pattern there and do that as well?
> >
>
> Thinking about it a little more, I think we should do that as well. And
> maybe explicitly set driver_data_len to 0 even though it's already zero.
> Basically for readability.
OK?
Index: dev/pci/if_vmx.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
retrieving revision 1.66
diff -u -p -r1.66 if_vmx.c
--- dev/pci/if_vmx.c 23 Jul 2021 00:29:14 -0000 1.66
+++ dev/pci/if_vmx.c 6 Aug 2021 12:28:51 -0000
@@ -157,7 +157,6 @@ struct vmxnet3_softc {
#define WRITE_BAR1(sc, reg, val) \
bus_space_write_4((sc)->sc_iot1, (sc)->sc_ioh1, reg, val)
#define WRITE_CMD(sc, cmd) WRITE_BAR1(sc, VMXNET3_BAR1_CMD, cmd)
-#define vtophys(va) 0 /* XXX ok? */
int vmxnet3_match(struct device *, void *, void *);
void vmxnet3_attach(struct device *, struct device *, void *);
@@ -468,8 +467,8 @@ vmxnet3_dma_init(struct vmxnet3_softc *s
ds->vmxnet3_revision = 1;
ds->upt_version = 1;
ds->upt_features = UPT1_F_CSUM | UPT1_F_VLAN;
- ds->driver_data = vtophys(sc);
- ds->driver_data_len = sizeof(struct vmxnet3_softc);
+ ds->driver_data = ~0ULL;
+ ds->driver_data_len = 0;
ds->queue_shared = qs_pa;
ds->queue_shared_len = qs_len;
ds->mtu = VMXNET3_MAX_MTU;
@@ -546,8 +545,8 @@ vmxnet3_alloc_txring(struct vmxnet3_soft
ts->cmd_ring_len = NTXDESC;
ts->comp_ring = comp_pa;
ts->comp_ring_len = NTXCOMPDESC;
- ts->driver_data = vtophys(tq);
- ts->driver_data_len = sizeof *tq;
+ ts->driver_data = ~0ULL;
+ ts->driver_data_len = 0;
ts->intr_idx = intr;
ts->stopped = 1;
ts->error = 0;
@@ -598,8 +597,8 @@ vmxnet3_alloc_rxring(struct vmxnet3_soft
rs->cmd_ring_len[1] = NRXDESC;
rs->comp_ring = comp_pa;
rs->comp_ring_len = NRXCOMPDESC;
- rs->driver_data = vtophys(rq);
- rs->driver_data_len = sizeof *rq;
+ rs->driver_data = ~0ULL;
+ rs->driver_data_len = 0;
rs->intr_idx = intr;
rs->stopped = 1;
rs->error = 0;