Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
Re: bce(4): use km_alloc
To:
David Hill <dhill@mindcry.org>
Cc:
tech@openbsd.org
Date:
Wed, 29 Jan 2025 16:36:50 +0100

Download raw body.

Thread
On 15/11/24(Fri) 16:00, David Hill wrote:
> 
> 
> On 11/14/24 14:58, Mark Kettenis wrote:
> > > Date: Thu, 14 Nov 2024 14:46:21 +0000
> > > From: David Hill <dhill@mindcry.org>
> > > 
> > > Hello -
> > > 
> > > uvm_km_kmemalloc_pla has 3 users: bce(4), envy(4), and malloc.
> > > 
> > > I have attempted a conversion to km_alloc for bce(4), but I lack the
> > > hardware to test.  If anyone has bce(4), could you please test?
> > 
> > This should really use bus_dmamem_alloc_range(9).  I don't think we
> > had that interface back when the driver was changed to use
> > uvm_km_kmemalloc_pla().
> > 
> 
> My attempt at the conversion:

Anyone with bce(4) could try this?

> Index: dev/pci/if_bce.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_bce.c,v
> diff -u -p -r1.57 if_bce.c
> --- dev/pci/if_bce.c	31 Aug 2024 16:23:09 -0000	1.57
> +++ dev/pci/if_bce.c	15 Nov 2024 15:58:18 -0000
> @@ -188,8 +188,8 @@ bce_attach(struct device *parent, struct
>  	pci_intr_handle_t ih;
>  	const char *intrstr = NULL;
>  	caddr_t kva;
> -	bus_dma_segment_t seg;
> -	int rseg;
> +	bus_dma_segment_t seg, dseg;
> +	int rseg, drseg;
>  	struct ifnet *ifp;
>  	pcireg_t memtype;
>  	bus_addr_t memaddr;
> @@ -249,10 +249,19 @@ bce_attach(struct device *parent, struct
>  	bce_reset(sc);
> 
>  	/* Create the data DMA region and maps. */
> -	if ((sc->bce_data = (caddr_t)uvm_km_kmemalloc_pla(kernel_map,
> -	    uvm.kernel_object, (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES, 0,
> -	    UVM_KMF_NOWAIT, 0, (paddr_t)(0x40000000 - 1), 0, 0, 1)) == NULL) {
> -		printf(": unable to alloc space for ring");
> +	if ((error = bus_dmamem_alloc_range(sc->bce_dmatag,
> +	    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES, 0, 0, &dseg, 1, &drseg,
> +	    BUS_DMA_NOWAIT, (bus_addr_t)0,
> +	    (bus_addr_t)(0x40000000 - 1))) != 0) {
> +		printf(": unable to alloc space for data, error = %d", error);
> +		return;
> +	}
> +
> +	if ((error = bus_dmamem_map(sc->bce_dmatag, &dseg, drseg,
> +	    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES, &sc->bce_data,
> +	    BUS_DMA_NOWAIT))) {
> +		printf(": unable to map data, error = %d\n", error);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -261,8 +270,7 @@ bce_attach(struct device *parent, struct
>  	    1, BCE_NRXDESC * MCLBYTES, 0, BUS_DMA_NOWAIT | BUS_DMA_ALLOCNOW,
>  	    &sc->bce_rxdata_map))) {
>  		printf(": unable to create ring DMA map, error = %d\n", error);
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -270,9 +278,8 @@ bce_attach(struct device *parent, struct
>  	if (bus_dmamap_load(sc->bce_dmatag, sc->bce_rxdata_map, sc->bce_data,
>  	    BCE_NRXDESC * MCLBYTES, NULL, BUS_DMA_READ | BUS_DMA_NOWAIT)) {
>  		printf(": unable to load rx ring DMA map\n");
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_rxdata_map);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -314,10 +321,9 @@ bce_attach(struct device *parent, struct
>  	    (bus_addr_t)0, (bus_addr_t)0x3fffffff))) {
>  		printf(": unable to alloc space for ring descriptors, "
>  		    "error = %d\n", error);
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_rxdata_map);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_txdata_map);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -325,11 +331,10 @@ bce_attach(struct device *parent, struct
>  	if ((error = bus_dmamem_map(sc->bce_dmatag, &seg, rseg,
>  	    2 * PAGE_SIZE, &kva, BUS_DMA_NOWAIT))) {
>  		printf(": unable to map DMA buffers, error = %d\n", error);
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_rxdata_map);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_txdata_map);
>  		bus_dmamem_free(sc->bce_dmatag, &seg, rseg);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -337,11 +342,10 @@ bce_attach(struct device *parent, struct
>  	if ((error = bus_dmamap_create(sc->bce_dmatag, 2 * PAGE_SIZE, 1,
>  	    2 * PAGE_SIZE, 0, BUS_DMA_NOWAIT, &sc->bce_ring_map))) {
>  		printf(": unable to create ring DMA map, error = %d\n", error);
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_rxdata_map);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_txdata_map);
>  		bus_dmamem_free(sc->bce_dmatag, &seg, rseg);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
> @@ -349,12 +353,11 @@ bce_attach(struct device *parent, struct
>  	if (bus_dmamap_load(sc->bce_dmatag, sc->bce_ring_map, kva,
>  	    2 * PAGE_SIZE, NULL, BUS_DMA_NOWAIT)) {
>  		printf(": unable to load ring DMA map\n");
> -		uvm_km_free(kernel_map, (vaddr_t)sc->bce_data,
> -		    (BCE_NTXDESC + BCE_NRXDESC) * MCLBYTES);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_rxdata_map);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_txdata_map);
>  		bus_dmamap_destroy(sc->bce_dmatag, sc->bce_ring_map);
>  		bus_dmamem_free(sc->bce_dmatag, &seg, rseg);
> +		bus_dmamem_free(sc->bce_dmatag, &dseg, drseg);
>  		return;
>  	}
> 
>