changeset 138:6527e078252c

Wip
author Louis Opter <kalessin@kalessin.fr>
date Sun, 19 Jan 2014 13:45:30 -0800
parents 8bbdb488f6fe
children ab3a2aa22f95
files rathaxes_sample_e1000_rewrite_device_dependent_code.patch
diffstat 1 files changed, 178 insertions(+), 89 deletions(-) [+]
line wrap: on
line diff
--- 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