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. > > > bye, > > Jan > > > > [1]: https://www.lkml.org/lkml/2021/1/19/1225 > > [2]: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/vmxnet3/vmxnet3_drv.c?id=de1da8bcf40564a2adada2d5d5426e05355f66e8 > > > > 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 5 Aug 2021 11:12:26 -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,6 @@ 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->queue_shared = qs_pa; > > ds->queue_shared_len = qs_len; > > ds->mtu = VMXNET3_MAX_MTU; > > @@ -546,8 +543,6 @@ 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->intr_idx = intr; > > ts->stopped = 1; > > ts->error = 0; > > @@ -598,8 +593,6 @@ 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->intr_idx = intr; > > rs->stopped = 1; > > rs->error = 0; > > >
