Mercurial > archived > louis > epitech > mq > rathaxes
view rathaxes_rewrite_create_and_destroy_device_in_the_e1000_sample.patch @ 97:ffdb018893e2
Finish the patch on the create/destroy_device rewrite in the e1000 sample and fix Builtin::symbol
| author | Louis Opter <louis@lse.epita.fr> |
|---|---|
| date | Thu, 22 Nov 2012 15:37:46 -0800 |
| parents | 3e715b3e0ecd |
| children | e2c237d6c37b |
line wrap: on
line source
# HG changeset patch # Parent d65cd0228e5c25692e6f37d0524e61ae26102a33 rathaxes: rehaul the device initialization/destruction in the e1000 sample - Add several chunks on the e1000::Context; these chunks perform low-level, device-dependant operations and should be move in the front-end at some point. They weave into pointcuts defined by the Ethernet subsystem; - The e1000::{create,destroy}_device sequence templates are entirely gone; they are replaced by the aforementioned chunks and more generic code in the PCI and Ethernet subsystems; - The PCI::{probe,remove} chunks/pointcuts have been rehauled and are centered on the new PCI::Device type template. diff --git a/rathaxes/samples/e1000/e1000.blt b/rathaxes/samples/e1000/e1000.blt --- a/rathaxes/samples/e1000/e1000.blt +++ b/rathaxes/samples/e1000/e1000.blt @@ -1,4 +1,4 @@ -with e1000, Ethernet, Socket, PCI, LKM, Log +with e1000, Ethernet, Socket, PCI, LKM, Log, Builtin { template type e1000::RxDescriptor() { @@ -268,6 +268,22 @@ template type e1000::Context() { + chunk ::decl() + { + struct rtx_e1000_ctx + { + int bars; + unsigned char /* __iomem */ *ioaddr; + unsigned int irq; + + /* we can't use the Rathaxes type here (#8) */ + //${e1000::RxRing} rx_ring; + //${e1000::TxRing} tx_ring; + struct rtx_e1000_rx_ring rx_ring; + struct rtx_e1000_tx_ring tx_ring; + }; + } + chunk Ethernet::SubContext() { /* @@ -278,28 +294,58 @@ ${e1000::Context} hw_ctx; } - chunk ::decl() + chunk Ethernet::adapter_init_context(Ethernet::Device rtx_ether_ctx, + Builtin::number bars, + Builtin::symbol ioaddr, + Builtin::number irq) { - /* - * Yes, this typedef looks ugly but read the remark about - * Ethernet::Device in ethernet.blt. - */ - struct rtx_e1000_ctx { - int bars; - unsigned char /* __iomem */ *ioaddr; - int irq; - - /* we can't use the Rathaxes type here (#8) */ - //${e1000::RxRing} rx_ring; - //${e1000::TxRing} tx_ring; - struct rtx_e1000_rx_ring rx_ring; - struct rtx_e1000_tx_ring tx_ring; - }; + struct rtx_e1000_ctx *hw_ctx = &${rtx_ether_ctx}->hw_ctx; + hw_ctx->bars = ${bars}; + hw_ctx->ioaddr = ${ioaddr}; + hw_ctx->irq = ${irq}; + } } - chunk ::init() + chunk Ethernet::adapter_reset(Ethernet::Device rtx_ether_ctx) { + { + struct rtx_e1000_ctx *hw_ctx = &${rtx_ether_ctx}->hw_ctx; + rtx_e1000_register_write32(hw_ctx, E1000_CTRL, E1000_CMD_RST); + udelay(10); + } + } + + chunk Ethernet::adapter_load_mac_address(Ethernet::Device rtx_ether_ctx) + { + { + struct rtx_e1000_ctx *hw_ctx = &${rtx_ether_ctx}->hw_ctx; + /* Shamelessly borrowed from Minix */ + for (int i = 0; i < 3; ++i) + { + rtx_e1000_register_write32(hw_ctx, E1000_EEPROM_READ, (i << 8) | 1); + int value; + do + value = rtx_e1000_register_read32(hw_ctx, E1000_EEPROM_READ); + while ((value & (1 << 4)) == 0); + value >>= 16; + /* + * XXX: The net_dev manipulations here should take place in the + * ethernet templates: + * TODO: recevoir un Builtin::symbol avec le tableau ici pour + * pouvoir les remonter directement. + */ + ${rtx_ether_ctx}->net_dev->dev_addr[i * 2] = value & 0xff; + ${rtx_ether_ctx}->net_dev->dev_addr[i * 2 + 1] = (value >> 8) & 0xff; + } + + /* TODO: deplacer ça dans Ethernet (see above point): */ + memcpy(${rtx_ether_ctx}->net_dev->perm_addr, + ${rtx_ether_ctx}->net_dev->dev_addr, + ${rtx_ether_ctx}->net_dev->addr_len); + + ${Log::info("e1000::create: mac address loaded from the EEPROM")}; + } } map @@ -477,91 +523,6 @@ } } - template sequence e1000::create_device() - { - chunk Ethernet::create_device(PCI::AbstractDevice pdev, Ethernet::Device rtx_ether_ctx) - { - /* - * PCI init stuff: - * - * Some of that code should certainly be moved in the PCI/Ethernet - * blts, also at some point maybe we could do that completely - * automatically in the PCI/Ethernet blts. - */ - - /* - * We could have used an init function here but since we can't init - * all the fields at once (see, ioaddr) and cannot call a C - * function within a placeholder (${}), it wasn't really worth it. - */ - ${rtx_ether_ctx}->hw_ctx.bars = pci_select_bars(${pdev}, IORESOURCE_MEM); - ${rtx_ether_ctx}->hw_ctx.irq = ${pdev}->irq; - - if (pci_enable_device_mem(${pdev})) - ${Log::info("e1000::create: pci_enable_device_mem failed")}; - if (pci_request_selected_regions(${pdev}, ${rtx_ether_ctx}->hw_ctx.bars, ${config.name})) - ${Log::info("e1000::create: pci_request_selected_regions failed")}; - if (${config.set_master}) - pci_set_master(${pdev}); - - /* 0 here is for BAR_0: */ - ${rtx_ether_ctx}->hw_ctx.ioaddr = pci_ioremap_bar(${pdev}, 0); - if (!${rtx_ether_ctx}->hw_ctx.ioaddr) - ${Log::info("e1000::create: pci_ioremap_bar failed")}; - - /* - * The really device specific algorithm starts here (so it should - * certainly be written in the frontend): - */ - - /* Reset the card */ - rtx_e1000_register_write32(&${rtx_ether_ctx}->hw_ctx, E1000_CTRL, E1000_CMD_RST); - udelay(10); - - /* Now we can load its mac address (thanks minix code) */ - for (int i = 0; i < 3; ++i) - { - rtx_e1000_register_write32(&${rtx_ether_ctx}->hw_ctx, E1000_EEPROM_READ, (i << 8) | 1); - - int value; - do - value = rtx_e1000_register_read32(&${rtx_ether_ctx}->hw_ctx, E1000_EEPROM_READ); - while ((value & (1 << 4)) == 0); - value >>= 16; - - ${rtx_ether_ctx}->net_dev->dev_addr[i * 2] = value & 0xff; - ${rtx_ether_ctx}->net_dev->dev_addr[i * 2 + 1] = (value >> 8) & 0xff; - } - - memcpy(${rtx_ether_ctx}->net_dev->perm_addr, - ${rtx_ether_ctx}->net_dev->dev_addr, - ${rtx_ether_ctx}->net_dev->addr_len); - - ${Log::info("e1000::create: mac address loaded from the EEPROM")}; - } - - chunk ::CALL() - { - } - } - - template sequence e1000::destroy_device() - { - chunk Ethernet::destroy_device(PCI::AbstractDevice pdev, Ethernet::Device rtx_ether_ctx) - { - /* - * Here, we should have some checks to avoid to free resources that - * haven't been allocated. (e.g: in case of previous errors). - */ - iounmap(${rtx_ether_ctx}->hw_ctx.ioaddr); - pci_release_selected_regions(${pdev}, ${rtx_ether_ctx}->hw_ctx.bars); - } - - chunk ::CALL() - { - } - } - /* TODO: make that a method of e1000::Context */ template sequence e1000::print_status(Ethernet::Device ctx) { diff --git a/rathaxes/samples/e1000/e1000.rti b/rathaxes/samples/e1000/e1000.rti --- a/rathaxes/samples/e1000/e1000.rti +++ b/rathaxes/samples/e1000/e1000.rti @@ -1,8 +1,5 @@ interface e1000 : Socket, Ethernet, PCI, LKM, Builtin { - /* XXX: This should be in pci.rti, also maybe we need a bool type */ - required variable Builtin::symbol set_master; - required variable Builtin::number rx_ring_size; required variable Builtin::number tx_ring_size; required variable Builtin::number rx_buffer_len; @@ -10,9 +7,18 @@ provided type Context { + method decl(); + chunk Ethernet::SubContext(); - method decl(); - method init(); + + /* Callbacks/Hooks which should probably be in the front-end: */ + chunk Ethernet::adapter_init_context(Ethernet::Device, + Builtin::number, + Builtin::symbol, + Builtin::number); +// chunk Ethernet::adapter_init_context(Ethernet::Device); + chunk Ethernet::adapter_reset(Ethernet::Device); + chunk Ethernet::adapter_load_mac_address(Ethernet::Device); } provided type RxDescriptor @@ -76,18 +82,6 @@ chunk ::decl(); } - provided sequence create_device() - { - provided chunk Ethernet::create_device(PCI::AbstractDevice, Ethernet::Device); - provided chunk ::CALL(); - } - - provided sequence destroy_device() - { - provided chunk Ethernet::destroy_device(PCI::AbstractDevice, Ethernet::Device); - provided chunk ::CALL(); - } - /* * This should take an e1000::Context as the first argument but this was * not working as wished. diff --git a/rathaxes/samples/e1000/ethernet.blt b/rathaxes/samples/e1000/ethernet.blt --- a/rathaxes/samples/e1000/ethernet.blt +++ b/rathaxes/samples/e1000/ethernet.blt @@ -113,6 +113,8 @@ map { + pci_device: ${self}->pci_dev; + net_device: ${self}->net_dev; } } @@ -210,48 +212,72 @@ }; } - /* For now the type is not handled, so we just omit it (see #17) */ - chunk PCI::pci_probe_hook(PCI::AbstractDevice pdev) + /* + * NOTE: for now, the error handling is leaking from PCI::probe(), but + * it's better than doing it at all. + * + * XXX: the chunk argument isn't correctly expanded by the + * compiler I have to use the same name as the actual C + * variable: + */ + chunk PCI::pci_probe_hook(PCI::Device rtx_pci_dev) { - /* - * This typedef is needed to workaround a bug in CNorm __std__ - * dialect. - */ ${Ethernet::Device} rtx_ether_ctx; - struct net_device *net_dev; - int error; + ${Ethernet::AbstractDevice} net_dev; - /* Initialize the net_device structure */ net_dev = alloc_etherdev(sizeof(*rtx_ether_ctx)); - if (net_dev == 0) + if (!net_dev) { - ${Log::info("cannot allocate memory")}; - /* - * Again, the error should be "raised" in the parent context. - * - * Here we know that we can return ENOMEM because *we* wrote - * the parent context. - */ - return -ENOMEM; + ${Log::info("cannot allocate the ethernet device context")}; + error = -ENOMEM; + goto fail; } - SET_NETDEV_DEV(net_dev, &${pdev}->dev); + SET_NETDEV_DEV(net_dev, ${rtx_pci_dev.device}); strlcpy(net_dev->name, ${config.ifname}, sizeof(net_dev->name)); - net_dev->irq = ${pdev}->irq; + net_dev->irq = ${rtx_pci_dev.irq}; net_dev->netdev_ops = &rtx_ether_ops; error = register_netdev(net_dev); if (error) { ${Log::info("cannot register the driver in the net subsystem")}; - return error; + goto fail; } /* Initialize our context held by the net_device structure */ - ${rtx_ether_ctx.init(local.net_dev, pdev)}; + /* + * XXX: the cast is here because the compiler resolve the + * type of rtx_pci_dev.pci_device to the type of + * rtx_pci_dev instead of the type of rtx_pci_dev.pci_device. + * + * Also, I'm getting placeholder in the generated code if + * I don't open a scope here. + */ + { + struct pci_dev *pci_device = ${rtx_pci_dev.pci_device}; + ${cast local.pci_device as PCI::AbstractDevice}; + ${local.rtx_ether_ctx.init(local.net_dev, local.pci_device)}; + } - pci_set_drvdata(${pdev}, net_dev); + /* Register ourselves in the parent context: */ + /* ${rtx_pci_dev.set_context(local.rtx_ether_ctx)}; */ + ${rtx_pci_dev}->context = rtx_ether_ctx; - ${pointcut Ethernet::create_device(pdev, local.rtx_ether_ctx)}; + /* + * XXX: the asssignments/casts are here to circumvent + * typing issues in the compiler (see previous XXX). + */ + int bars = ${rtx_pci_dev.bars}; + unsigned char /* __iomem */ *ioaddr = ${rtx_pci_dev.ioaddr}; + int irq = ${rtx_pci_dev.irq}; + ${cast local.bars as Builtin::number}; + ${cast local.irq as Builtin::number}; + ${pointcut Ethernet::adapter_init_context(local.rtx_ether_ctx, + local.bars, + local.ioaddr, + local.irq)}; + ${pointcut Ethernet::adapter_reset(local.rtx_ether_ctx)}; + ${pointcut Ethernet::adapter_load_mac_address(local.rtx_ether_ctx)}; } /* This chunk should be removed (see #26) */ @@ -262,16 +288,18 @@ template sequence Ethernet::exit() { - chunk PCI::pci_remove_hook(PCI::AbstractDevice pdev) + /* + * XXX: We have to use the same name as the C variable for + * the chunk argument... + */ + chunk PCI::pci_remove_hook(PCI::Device rtx_pci_dev) { - /* workaround for cnorm unstrict */ - struct net_device *net_dev = pci_get_drvdata(${pdev}); - ${Ethernet::Device} rtx_ether_ctx = netdev_priv(net_dev); + ${Ethernet::Device} rtx_ether_ctx = ${rtx_pci_dev.context}; - ${pointcut Ethernet::destroy_device(pdev, local.rtx_ether_ctx)}; + BUG_ON(!rtx_ether_ctx); - unregister_netdev(net_dev); - free_netdev(net_dev); + unregister_netdev(${local.rtx_ether_ctx.net_device}); + free_netdev(${local.rtx_ether_ctx.net_device}); } /* This chunk should be removed (see #26) */ diff --git a/rathaxes/samples/e1000/ethernet.rti b/rathaxes/samples/e1000/ethernet.rti --- a/rathaxes/samples/e1000/ethernet.rti +++ b/rathaxes/samples/e1000/ethernet.rti @@ -1,4 +1,4 @@ -interface Ethernet : Socket, PCI, LKM +interface Ethernet : Socket, PCI, LKM, Builtin { required variable Builtin::string ifname; @@ -23,6 +23,9 @@ method decl(); method init(Ethernet::AbstractDevice, PCI::AbstractDevice); pointcut Ethernet::SubContext(); + + attribute PCI::AbstractDevice pci_device; + attribute Ethernet::AbstractDevice net_device; } required sequence open(Ethernet::Device) @@ -52,15 +55,18 @@ provided sequence init() { provided chunk LKM::data(); - provided chunk PCI::pci_probe_hook(PCI::AbstractDevice); + provided chunk PCI::pci_probe_hook(PCI::Device); - provided pointcut Ethernet::create_device(PCI::AbstractDevice, Ethernet::Device); + provided pointcut Ethernet::adapter_init_context(Ethernet::Device, + Builtin::number, + Builtin::symbol, + Builtin::number); + provided pointcut Ethernet::adapter_reset(Ethernet::Device); + provided pointcut Ethernet::adapter_load_mac_address(Ethernet::Device); } provided sequence exit() { - provided chunk PCI::pci_remove_hook(PCI::AbstractDevice); - - provided pointcut Ethernet::destroy_device(PCI::AbstractDevice, Ethernet::Device); + provided chunk PCI::pci_remove_hook(PCI::Device); } } diff --git a/rathaxes/samples/e1000/lkm.rtx b/rathaxes/samples/e1000/lkm.rtx --- a/rathaxes/samples/e1000/lkm.rtx +++ b/rathaxes/samples/e1000/lkm.rtx @@ -74,7 +74,6 @@ */ PCI::vendor_id = 0x8086; PCI::product_id = 0x100e; - PCI::set_master = true; Ethernet::ifname = "rtx%d"; diff --git a/rathaxes/samples/e1000/pci.blt b/rathaxes/samples/e1000/pci.blt --- a/rathaxes/samples/e1000/pci.blt +++ b/rathaxes/samples/e1000/pci.blt @@ -12,12 +12,102 @@ struct pci_dev; } - chunk ::init(PCI::AbstractDevice) + chunk ::init(PCI::AbstractDevice) { } + chunk set_context(Builtin::symbol ctx) + { + pci_set_drvdata(${self}, ${ctx}); + } + map { + context: pci_get_drvdata(${self}); + } + } + + template type PCI::Device() + { + chunk ::decl() + { + struct rtx_pci_dev + { + struct pci_dev *pdev; + int bars; + /* It could be an array at some point: */ + unsigned char /* __iomem */ *ioaddr; + void *context; + }; + } + + chunk ::init(PCI::AbstractDevice pdev) + { + ${self}->pdev = ${pdev}; + ${self}->bars = pci_select_bars(${pdev}, IORESOURCE_MEM); + ${self}->ioaddr = NULL; + ${self}->context = NULL; + } + + chunk LKM::prototypes() + { + static int rtx_pci_device_enable(struct rtx_pci_dev *); + static void rtx_pci_device_disable(struct rtx_pci_dev *); + } + + chunk LKM::code() + { + static int rtx_pci_device_enable(struct rtx_pci_dev *self) + { + int error; + error = pci_enable_device(self->pdev); + if (error) + return error; + error = pci_request_selected_regions(self->pdev, self->bars, ${config.name}); + if (error) + return error; + pci_set_master(self->pdev); + return 0; + } + + static void rtx_pci_device_disable(struct rtx_pci_dev *self) + { + if (self->ioaddr) + iounmap(self->ioaddr); + pci_release_selected_regions(self->pdev, self->bars); + pci_disable_device(self->pdev); + } + } + + chunk enable() + { + rtx_pci_device_enable(${self}); + } + + chunk disable() + { + rtx_pci_device_disable(${self}); + } + + chunk select_ioaddr(Builtin::number bar) + { + ${self}->ioaddr = pci_ioremap_bar(${self}->pdev, ${bar}); + } + + chunk set_context(Builtin::symbol ctx) + { + ${self}->context = ctx; + } + + map + { + context: ${self}->context; + device: &${self}->pdev->dev; + pci_device: ${self}->pdev; + irq: ${self}->pdev->irq; + bars: ${self}->bars; + ioaddr: ${self}->ioaddr; + BAR_0: 0; } } @@ -34,24 +124,67 @@ static int /* __devinit */ rtx_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id) { - int err; + ${cast local.pdev as PCI::AbstractDevice}; + int error; + ${PCI::Device} *rtx_pci_dev; - err = pci_enable_device(pdev); - if (err < 0) + rtx_pci_dev = kmalloc(sizeof(*rtx_pci_dev), GFP_KERNEL); + if (!rtx_pci_dev) + { + ${Log::info("cannot allocate the pci device context")}; + error = -ENOMEM; goto fail; + } - /* Use local. to reference a local C variable: */ - ${cast local.pdev as PCI::AbstractDevice}; - ${pointcut PCI::pci_probe_hook(local.pdev)}; + /* + * XXX: I'm getting placeholder in the generated code if I don't + * open a scope here: + */ + { + ${local.rtx_pci_dev.init(local.pdev)}; + } + + /* ${local.pdev.set_context(local.rtx_pci_dev)}; */ + pci_set_drvdata(pdev, rtx_pci_dev); + + /* ${local.rtx_pci_dev.enable()}; */ + error = rtx_pci_device_enable(rtx_pci_dev); + if (error) + { + ${Log::info("rtx_pci_device_enable failed")}; + goto fail; + } + + /* ${local.rtx_pci_dev.select_ioaddr(local.rtx_pci_dev.BAR_0)}; */ + rtx_pci_dev->ioaddr = pci_ioremap_bar( + rtx_pci_dev->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? */ + goto fail; + } + + /* + * XXX: We have to cast here because the compiler is + * confused by the fact that rtx_pci_dev is a + * pointer. + */ + ${cast local.rtx_pci_dev as PCI::Device}; + ${pointcut PCI::pci_probe_hook(local.rtx_pci_dev)}; return 0; - fail: - return err; + fail: + /* ${local.pdev.set_context(NULL)}; */ + pci_set_drvdata(pdev, NULL); + kfree(rtx_pci_dev); + return error; } } - /* This chunk should be remove (see #26) */ + /* This chunk should be removed (see #26) */ chunk ::CALL() { } @@ -69,9 +202,16 @@ static void rtx_pci_remove(struct pci_dev *pdev) { ${cast local.pdev as PCI::AbstractDevice}; - ${pointcut PCI::pci_remove_hook(local.pdev)}; + ${PCI::Device} *rtx_pci_dev = ${local.pdev.context}; - pci_disable_device(pdev); + BUG_ON(!rtx_pci_dev); + + /* XXX: compiler confused by the pointer type. */ + ${cast local.rtx_pci_dev as PCI::Device}; + ${pointcut PCI::pci_remove_hook(local.rtx_pci_dev)}; + + /* ${local.rtx_pci_dev.disable()}; */ + rtx_pci_device_disable(rtx_pci_dev); } } diff --git a/rathaxes/samples/e1000/pci.rti b/rathaxes/samples/e1000/pci.rti --- a/rathaxes/samples/e1000/pci.rti +++ b/rathaxes/samples/e1000/pci.rti @@ -1,14 +1,40 @@ -interface PCI : LKM +interface PCI : LKM, Builtin, Device { + required variable Builtin::string LKM::name; + required variable Builtin::number PCI::vendor_id; + required variable Builtin::number PCI::product_id; + provided type PCI::AbstractDevice { chunk LKM::includes(); + method decl(); method init(PCI::AbstractDevice); + method set_context(Builtin::symbol); + + attribute Builtin::symbol context; } - required variable Builtin::number PCI::vendor_id; - required variable Builtin::number PCI::product_id; + provided type PCI::Device + { + chunk LKM::prototypes(); + chunk LKM::code(); + + method decl(); + method init(PCI::AbstractDevice); + method enable(); + method disable(); + method select_ioaddr(Builtin::number); + method set_context(Builtin::symbol); + + attribute Builtin::symbol context; + attribute Device::AbstractDevice device; + attribute PCI::AbstractDevice pci_device; + attribute Builtin::symbol ioaddr; + attribute Builtin::number BAR_0; + attribute Builtin::number irq; + attribute Builtin::number bars; + } provided sequence register() { @@ -28,7 +54,7 @@ provided chunk LKM::prototypes(); provided chunk LKM::code(); - provided pointcut PCI::pci_probe_hook(PCI::AbstractDevice); + provided pointcut PCI::pci_probe_hook(PCI::Device); } provided sequence remove() @@ -36,6 +62,6 @@ provided chunk LKM::prototypes(); provided chunk LKM::code(); - provided pointcut PCI::pci_remove_hook(PCI::AbstractDevice); + provided pointcut PCI::pci_remove_hook(PCI::Device); } }