# HG changeset patch # User Louis Opter # Date 1390167930 28800 # Node ID 6527e078252cf0ceb21afe38c4c25fb18a591d85 # Parent 8bbdb488f6fe811a21982ec079f3352c8ee065d4 Wip diff -r 8bbdb488f6fe -r 6527e078252c rathaxes_sample_e1000_rewrite_device_dependent_code.patch --- a/rathaxes_sample_e1000_rewrite_device_dependent_code.patch Sat Jan 18 17:52:33 2014 -0800 +++ b/rathaxes_sample_e1000_rewrite_device_dependent_code.patch Sun Jan 19 13:45:30 2014 -0800 @@ -1,12 +1,12 @@ # HG changeset patch -# Parent d71e19169c392a9509bf816bf2ca7858212df878 +# Parent 06f2b107580ea4f01d8e9333161d1e1b5f0de759 rathaxes: rewrite/refactor all the e1000 device dependent code diff --git a/notes.txt b/notes.txt new file mode 100644 --- /dev/null +++ b/notes.txt -@@ -0,0 +1,47 @@ +@@ -0,0 +1,52 @@ +Remarks for David & Lionel: + +- Too much changes to not start over; @@ -45,7 +45,12 @@ + Ethernet::init sequence, maybe the content of this sequence should be moved + under the Ethernet::Device type definition (maybe we could solve the + free_netdev call in case of failure at the same time); -+- PCI::Device.device should be PCI::Device.k_device. ++- Likewise sequences in the PCI subsystem could be moved to the PCI::Device ++ type definition; ++- PCI::Device.device should be PCI::Device.k_device; ++- The Ethernet's chunk for PCI::pci_remove_hook should probably call cleanup ++ functions from the Context and the Rings, unless the kernel alway calls ++ rtx_ethernet_close first automatically. + +Questions: + @@ -689,7 +694,7 @@ { E1000_TXD_DTYP_D = 0x00100000, /* Data Descriptor */ E1000_TXD_DTYP_C = 0x00000000, /* Context Descriptor */ -@@ -701,326 +140,701 @@ +@@ -701,326 +140,703 @@ } } @@ -823,7 +828,8 @@ + + static unsigned int rtx_e1000_reg_read32(${e1000::Context} self, ${e1000::Register} reg) + { -+ // XXX The compiler adds a & if I use ${local.self.io_addr}: ++ // XXX The compiler generates &self->io_addr instead of ++ // self->io_addr if I use ${local.self.io_addr}: + return ioread32(self->io_addr + reg); + } + @@ -999,9 +1005,9 @@ + err_alloc_tx_ring: + rtx_e1000_rx_ring_free_resources(&hw_ctx->rx_ring); + err_alloc_rx_ring: -+ // XXX Can't return here since we don't know the context. -+ // TODO: hardcode a goto -+ (void)1; ++ // XXX Can't return here since we don't know the context, ++ // anyway, hardcoded goto: ++ goto rtx_ethernet_open_fail; + } + } + @@ -1195,16 +1201,19 @@ + int i; + + // Allocate the descriptors -+ hw_ctx->rx_ring.base = ${DMA::alloc_coherent( ++ ${local.self.descs} = ${DMA::alloc_coherent( + local.self.hw_ctx.net_dev.device, local.self.size, + local.self.dma.dma_handle + )}; -+ if (!hw_ctx->rx_ring.base) { ++ ++ if (!${local.self.descs}) { + ${Log::info("adapter_init_rx: cannot allocate the descriptors for the rx ring")}; + goto err_rx_ring_alloc; + } + -+ ${Log::info("adapter_init_rx: rx descriptors allocated")}; ++ // XXX The generated code is completely scrambled if this is ++ // not scoped: ++ { ${Log::info("adapter_init_rx: rx descriptors allocated")}; } + + // Allocate the skbuffs, map them for DMA, and write their address + // in the corresponding descriptor. @@ -1365,13 +1374,12 @@ + + map + { -+ // TODO: fix cast to directly get the attributes from the Ring: -+ hw_ctx: ((int)(${self})->hw_ctx); -+ dma: ((int)(${self})->dma); -+ size: ((int)(${self})->size); -+ descs: ((int)(${self})->descs); -+ buffs: ((int)(${self})->buffs); -+ desc_size: sizeof(/* TODO: get the generated type... ${e1000::RxDescriptor} */int); ++ hw_ctx: ${self}->ring.hw_ctx; ++ dma: ${self}->ring.dma; ++ size: ${self}->ring.size; ++ descs: ${self}->ring.descs; ++ buffs: ${self}->ring.buffs; ++ desc_size: sizeof(rtxType_e1000_RxDescriptor); // XXX hardcoded } } @@ -1634,13 +1642,12 @@ - /* XXX: ${local.skb.unmap_to_and_free(local.dev)}; */ - rtx_socket_skbuff_unmap_and_free(&skb, ${devp.k_device}, DMA_TO_DEVICE); - return NETDEV_TX_OK; -+ // TODO: fix cast to directly get the attributes from the Ring: -+ hw_ctx: ((int)(${self})->hw_ctx); -+ dma: ((int)(${self})->dma); -+ size: ((int)(${self})->size); -+ descs: ((int)(${self})->descs); -+ buffs: ((int)(${self})->buffs); -+ desc_size: sizeof(/* TODO: get the generated type ${e1000::TxDescriptor} */int); ++ hw_ctx: ${self}->ring.hw_ctx; ++ dma: ${self}->ring.dma; ++ size: ${self}->ring.size; ++ descs: ${self}->ring.descs; ++ buffs: ${self}->ring.buffs; ++ desc_size: sizeof(rtxType_e1000_TxDescriptor); // XXX hardcoded } } } @@ -1965,7 +1972,7 @@ { chunk LKM::includes() { -@@ -157,100 +160,114 @@ +@@ -157,100 +160,115 @@ chunk LKM::prototypes() { @@ -1983,9 +1990,10 @@ - * "enclosed" type (e.g: local.var.enclosed) correctly. - */ ${Ethernet::AbstractDevice.ref} rtx_net_dev; - { /* XXX: I end up with a placeholder if I don't open a scope */ - ${local.rtx_net_dev.init(local.dev)}; - } +- { /* XXX: I end up with a placeholder if I don't open a scope */ +- ${local.rtx_net_dev.init(local.dev)}; +- } ++ ${local.rtx_net_dev.init(local.dev)}; ${Ethernet::Device.ref} rtx_ether_ctx = ${local.rtx_net_dev.rtx_ether_ctx}; - int error; @@ -2019,86 +2027,69 @@ - ${pointcut ::IMPLEMENTATION(local.rtx_ether_ctx)}; + + ${pointcut Ethernet::adapter_enable_interrupts(local.rtx_ether_ctx)}; - - return 0; - } - } ++ ++ return 0; ++ ++ rtx_ethernet_open_fail: ++ return -EIO; // XXX Probably not appropriate. ++ } ++ } + + /* XXX This chunk should be removed (see #26) */ + chunk ::CALL() + { + } - } - -- template sequence Ethernet::send(Ethernet::Device dev, Socket::AbstractSKBuff skb) ++ } ++ + template sequence Ethernet::send() - { - chunk LKM::prototypes() - { -- static int rtx_ethernet_xmit(struct sk_buff* skb, struct net_device *dev); ++ { ++ chunk LKM::prototypes() ++ { + static int rtx_ethernet_xmit(struct sk_buff *skb, struct net_device *dev); - } - - chunk LKM::code() - { -- static int rtx_ethernet_xmit(struct sk_buff* kernel_skb, struct net_device *net_dev) ++ } ++ ++ chunk LKM::code() ++ { + static int rtx_ethernet_xmit(struct sk_buff *k_skb, struct net_device *net_dev) - { - ${Ethernet::Device.ref} rtx_ether_ctx = netdev_priv(net_dev); -- ${Socket::AbstractSKBuff.ref} rtx_skb = (${Socket::AbstractSKBuff.ref}) kernel_skb; ++ { ++ ${Ethernet::Device.ref} rtx_ether_ctx = netdev_priv(net_dev); + ${Socket::SKBuff.ref} rtx_skb; - -- ${pointcut ::IMPLEMENTATION(local.rtx_ether_ctx, local.rtx_skb)}; ++ + ${cast local.k_skb as Socket::AbstractSKBuff.ref}; + ${local.rtx_skb.init(local.k_skb)}; + + ${Log::info("we have one packet to transmit!")}; + ${pointcut Ethernet::adapter_xmit(local.rtx_ether_ctx, local.rtx_skb)}; - } - } ++ } ++ } + + /* XXX This chunk should be removed (see #26) */ + chunk ::CALL() + { + } - } - -- template sequence Ethernet::close(Ethernet::Device dev) ++ } ++ + template sequence Ethernet::close() - { - chunk LKM::prototypes() - { -- static int rtx_ethernet_close(struct net_device *); ++ { ++ chunk LKM::prototypes() ++ { + static int rtx_ethernet_close(struct net_device *); - } - - chunk LKM::code() - { -- static int rtx_ethernet_close(struct net_device *dev) ++ } ++ ++ chunk LKM::code() ++ { + static int rtx_ethernet_close(struct net_device *net_dev) - { -- ${Ethernet::AbstractDevice.ref} rtx_net_dev; -- { /* XXX: I end up with a placeholder if I don't open a scope */ -- ${local.rtx_net_dev.init(local.dev)}; -- } ++ { + ${cast local.net_dev as Ethernet::AbstractDevice.ref}; + ${Ethernet::Device.ref} rtx_ether_ctx = ${local.net_dev.rtx_ether_ctx}; - -- ${Ethernet::Device.ref} rtx_ether_ctx = ${local.rtx_net_dev.rtx_ether_ctx}; ++ + ${pointcut Ethernet::adapter_disable_rx(local.rtx_ether_ctx)}; - -- /* TODO: change this pointcut into a pointcut/adapter/callback: */ -- { -- ${pointcut ::IMPLEMENTATION(local.rtx_ether_ctx)}; -- } ++ + netif_tx_disable(net_dev); + ${pointcut Ethernet::adapter_disable_tx(local.rtx_ether_ctx)}; - ++ + ${pointcut Ethernet::adapter_disable_interrupts(local.rtx_ether_ctx)}; - free_irq(${local.rtx_ether_ctx.irq}, dev); -- { -- ${Log::info("interrupt handler free'ed")}; -- } ++ free_irq(${local.rtx_ether_ctx.irq}, dev); + ${Log::info("interrupt handler free'ed")}; + + // TODO/XXX: There is definitely more stuff to do around here @@ -2110,19 +2101,68 @@ return 0; } } -+ +- } + +- template sequence Ethernet::send(Ethernet::Device dev, Socket::AbstractSKBuff skb) +- { +- chunk LKM::prototypes() + /* XXX This chunk should be removed (see #26) */ + chunk ::CALL() -+ { -+ } + { +- static int rtx_ethernet_xmit(struct sk_buff* skb, struct net_device *dev); +- } +- +- chunk LKM::code() +- { +- static int rtx_ethernet_xmit(struct sk_buff* kernel_skb, struct net_device *net_dev) +- { +- ${Ethernet::Device.ref} rtx_ether_ctx = netdev_priv(net_dev); +- ${Socket::AbstractSKBuff.ref} rtx_skb = (${Socket::AbstractSKBuff.ref}) kernel_skb; +- +- ${pointcut ::IMPLEMENTATION(local.rtx_ether_ctx, local.rtx_skb)}; +- } + } } +- template sequence Ethernet::close(Ethernet::Device dev) +- { +- chunk LKM::prototypes() +- { +- static int rtx_ethernet_close(struct net_device *); +- } +- +- chunk LKM::code() +- { +- static int rtx_ethernet_close(struct net_device *dev) +- { +- ${Ethernet::AbstractDevice.ref} rtx_net_dev; +- { /* XXX: I end up with a placeholder if I don't open a scope */ +- ${local.rtx_net_dev.init(local.dev)}; +- } +- +- ${Ethernet::Device.ref} rtx_ether_ctx = ${local.rtx_net_dev.rtx_ether_ctx}; +- +- /* TODO: change this pointcut into a pointcut/adapter/callback: */ +- { +- ${pointcut ::IMPLEMENTATION(local.rtx_ether_ctx)}; +- } +- +- free_irq(${local.rtx_ether_ctx.irq}, dev); +- { +- ${Log::info("interrupt handler free'ed")}; +- } +- +- return 0; +- } +- } +- } +- - template sequence Ethernet::interrupt_handler(Ethernet::Device dev) + template sequence Ethernet::interrupt_handler() { /* * We can't use the irqreturn_t type here because CNornm doesn't know -@@ -258,29 +275,34 @@ +@@ -258,29 +276,34 @@ */ chunk LKM::prototypes() { @@ -2161,7 +2201,7 @@ { .ndo_open = rtx_ethernet_open, .ndo_stop = rtx_ethernet_close, -@@ -288,74 +310,65 @@ +@@ -288,74 +311,65 @@ }; } @@ -2289,7 +2329,7 @@ chunk ::CALL() { } -@@ -369,15 +382,17 @@ +@@ -369,15 +383,17 @@ */ chunk PCI::pci_remove_hook(PCI::Device rtx_pci_dev) { @@ -2562,7 +2602,14 @@ ${self}->ioaddr = NULL; ${self}->context = NULL; } -@@ -98,7 +97,7 @@ +@@ -92,13 +91,12 @@ + + method select_ioaddr(Builtin::number bar) + { +- ${PCI::AbstractDevice.ref} select_ioaddr_pdev = ${self}->pdev; +- ${self}->ioaddr = pci_ioremap_bar(${local.select_ioaddr_pdev.k_pci_dev}, ${bar}); ++ ${self.ioaddr} = pci_ioremap_bar(${self.pci_device.k_pci_dev}, ${bar}); + } method set_rtx_drv_context(Builtin::symbol ctx) { @@ -2571,7 +2618,7 @@ } map -@@ -127,7 +126,7 @@ +@@ -127,7 +125,7 @@ const struct pci_device_id *pdev_id) { int error; @@ -2580,6 +2627,48 @@ ${PCI::AbstractDevice.ref} rtx_pdev = (${PCI::AbstractDevice.ref})pdev; rtx_pci_dev = kmalloc(sizeof(*rtx_pci_dev), GFP_KERNEL); +@@ -138,13 +136,7 @@ + goto fail; + } + +- /* +- * XXX: I'm getting placeholder in the generated code if I don't +- * open a scope here: +- */ +- { +- ${local.rtx_pci_dev.init(local.rtx_pdev)}; +- } ++ ${local.rtx_pci_dev.init(local.rtx_pdev)}; + + /* ${local.pdev.set_rtx_context(local.rtx_pci_dev)}; */ + pci_set_drvdata(pdev, rtx_pci_dev); +@@ -157,14 +149,14 @@ + goto fail; + } + +- /* ${local.rtx_pci_dev.select_ioaddr(local.rtx_pci_dev.BAR_0)}; */ +- rtx_pci_dev->ioaddr = pci_ioremap_bar( ++ // XXX #46: ${local.rtx_pci_dev.select_ioaddr(local.rtx_pci_dev.BAR_0)}; ++ ${local.rtx_pci_dev.ioaddr} = pci_ioremap_bar( + pdev, ${local.rtx_pci_dev.BAR_0} + ); + if (!${local.rtx_pci_dev.ioaddr}) + { + ${Log::info("can't map the device address space")}; +- error = 1; /* XXX anything more approriate? */ ++ error = -EIO; /* XXX anything more approriate? */ + goto fail; + } + +@@ -173,7 +165,7 @@ + return 0; + + fail: +- /* ${local.pdev.set_rtx_drv_context(NULL)}; */ ++ // ${local.pdev.set_rtx_drv_context(NULL)}; + pci_set_drvdata(pdev, NULL); + kfree(rtx_pci_dev); + return error; diff --git a/rathaxes/samples/e1000/socket.blt b/rathaxes/samples/e1000/socket.blt --- a/rathaxes/samples/e1000/socket.blt +++ b/rathaxes/samples/e1000/socket.blt