Hi Ralf, > > > >On 02/12/2021 16:48, Stephane Viau wrote: >> Pick a cell and convert it to show the influence of these helper files. >> >> Signed-off-by: Stephane Viau <[email protected]> >> >> --- >> >> v2 -> v3: >> - Get rid of the *_NUM macros in config files and have them computed >> instead (suggested by Ralf) >> >> v2: >> - Convert an existing cell to see the impact of the helper files >> (suggested by Jan) >> >> Signed-off-by: Stephane Viau <[email protected]> >> --- >> configs/arm64/imx8mp-inmate-demo.c | 161 ++++++++--------------------- >> 1 file changed, 45 insertions(+), 116 deletions(-) > >nice diffstats.
I thought so too :-)... especially when we multiply the number of cells (per use-case, per device ...) > >> >> diff --git a/configs/arm64/imx8mp-inmate-demo.c >> b/configs/arm64/imx8mp-inmate-demo.c >> index 127392df..169177ad 100644 >> --- a/configs/arm64/imx8mp-inmate-demo.c >> +++ b/configs/arm64/imx8mp-inmate-demo.c >> @@ -1,126 +1,55 @@ >> /* >> - * iMX8MM target - inmate-demo >> + * iMX8MP target - inmate-demo >> * >> - * Copyright 2020 NXP >> + * Copyright 2020-2021 NXP >> * >> * Authors: >> * Peng Fan <[email protected]> >> + * Stephane Viau <[email protected]> >> * >> * This work is licensed under the terms of the GNU GPL, version 2. See >> * the COPYING file in the top-level directory. >> */ >> >> -#include <jailhouse/types.h> >> -#include <jailhouse/cell-config.h> >> - >> -struct { >> - struct jailhouse_cell_desc cell; >> - __u64 cpus[1]; >> - struct jailhouse_memory mem_regions[8]; >> - struct jailhouse_irqchip irqchips[1]; >> - struct jailhouse_pci_device pci_devices[1]; >> -} __attribute__((packed)) config = { >> - .cell = { >> - .signature = JAILHOUSE_CELL_DESC_SIGNATURE, >> - .revision = JAILHOUSE_CONFIG_REVISION, >> - .name = "inmate-demo", >> - .flags = JAILHOUSE_CELL_PASSIVE_COMMREG, >> - >> - .cpu_set_size = sizeof(config.cpus), >> - .num_memory_regions = ARRAY_SIZE(config.mem_regions), >> - .num_irqchips = ARRAY_SIZE(config.irqchips), >> - .num_pci_devices = ARRAY_SIZE(config.pci_devices), >> - /* IVSHMEM_IRQ - 32 */ >> - .vpci_irq_base = 76, /* Not include 32 base */ >> - >> - .console = { >> - .address = 0x30890000, >> - .type = JAILHOUSE_CON_TYPE_IMX, >> - .flags = JAILHOUSE_CON_ACCESS_MMIO | >> - JAILHOUSE_CON_REGDIST_4, >> - }, >> - }, >> - >> - .cpus = { >> - 0x8, >> - }, >> - >> - .mem_regions = { >> - /* IVSHMEM shared memory regions (demo) */ >> - { >> - .phys_start = 0xfd900000, >> - .virt_start = 0xfd900000, >> - .size = 0x1000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - { >> - .phys_start = 0xfd901000, >> - .virt_start = 0xfd901000, >> - .size = 0x9000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | >> - JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - { >> - .phys_start = 0xfd90a000, >> - .virt_start = 0xfd90a000, >> - .size = 0x2000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - { >> - .phys_start = 0xfd90c000, >> - .virt_start = 0xfd90c000, >> - .size = 0x2000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | >> - JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - { >> - .phys_start = 0xfd90e000, >> - .virt_start = 0xfd90e000, >> - .size = 0x2000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - /* UART2 */ { >> - .phys_start = 0x30890000, >> - .virt_start = 0x30890000, >> - .size = 0x1000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | >> - JAILHOUSE_MEM_IO | JAILHOUSE_MEM_ROOTSHARED, >> - }, >> - /* RAM: start from the bottom of inmate memory */ { >> - .phys_start = 0xc0000000, >> - .virt_start = 0, >> - .size = 0x00010000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | >> - JAILHOUSE_MEM_EXECUTE | JAILHOUSE_MEM_LOADABLE, >> - }, >> - /* communication region */ { >> - .virt_start = 0x80000000, >> - .size = 0x00001000, >> - .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | >> - JAILHOUSE_MEM_COMM_REGION, >> - }, >> - }, >> - >> - .irqchips = { >> - /* GIC */ { >> - .address = 0x38800000, >> - .pin_base = 96, >> - .pin_bitmap = { >> - 0x1 << (76 + 32 - 96) /* SPI 76 */ >> - }, >> - }, >> - }, >> - >> - .pci_devices = { >> - { >> - .type = JAILHOUSE_PCI_TYPE_IVSHMEM, >> - .domain = 2, >> - .bdf = 0 << 3, >> - .bar_mask = JAILHOUSE_IVSHMEM_BAR_MASK_INTX, >> - .shmem_regions_start = 0, >> - .shmem_dev_id = 1, >> - .shmem_peers = 1, >> - .shmem_protocol = JAILHOUSE_SHMEM_PROTO_UNDEFINED, >> - }, >> - }, >> -}; >> +#include "cell-helper.h" >> + >> +/* Name, cores, entry point */ >> +#define CONFIG_INMATE_NAME "inmate-demo" >> +#define CONFIG_INMATE_CORE_BITMAP (0b1000) >> +#define CONFIG_INMATE_BASE (0x00000000) >> + >> +/* Memory & peripherals */ >> +#define CONFIG_INMATE_REGIONS \ >> + MEM_REGION_RWXL(0xc0000000, CONFIG_INMATE_BASE, MB(16)), /* RAM */ \ >> + \ >> + MEM_REGION_ROS( 0xfd900000, 0xfd900000, KB(4)), /* IVSHMEM */ \ >> + MEM_REGION_RWS( 0xfd901000, 0xfd901000, KB(36)), /* IVSHMEM */ \ >> + MEM_REGION_ROS( 0xfd90a000, 0xfd90a000, KB(8)), /* IVSHMEM */ \ >> + MEM_REGION_RWS( 0xfd90c000, 0xfd90c000, KB(8)), /* IVSHMEM */ \ >> + MEM_REGION_ROS( 0xfd90e000, 0xfd90e000, KB(8)), /* IVSHMEM */ \ > >superfluous whitespaces. > >> + \ >> + MMIO_REGION_RW( 0x30890000, 0x30890000, KB(4)), /* UART2 */ \ > >No need for the backslash. > >> + >> +/* GIC */ >> +#define CONFIG_INMATE_IRQCHIPS_ADDR (0x30890000) >> +#define CONFIG_INMATE_IRQCHIPS_BASE (32) >> +#define CONFIG_INMATE_IRQCHIPS_BITMAP \ >> + /* interrupts 32..63 */ \ >> + 0, \ >> + /* interrupts 64..95 */ \ >> + 0, \ >> + /* interrupts 96..127 */ \ >> + 1 << (76 + 32 - 96), /* SPI */ \ >> + /* interrupts 128..159 */ \ >> + 0 >> + >> +#define CONFIG_INMATE_VPCI_IRQ_BASE (76) /* IVSHMEM_IRQ */ >> + >> +#define CONFIG_INMATE_PCI_DEVICES \ >> + PCI_DEVICE_IVSHMEM(2, 0, 0, 1, 1) >> + >> +#define CONFIG_INMATE_CONSOLE \ >> + CONSOLE(0x30890000, JAILHOUSE_CON_TYPE_IMX, \ >> + JAILHOUSE_CON_ACCESS_MMIO | JAILHOUSE_CON_REGDIST_4) >> + >> +#include "cell-create.h" > >In my opinion, it's good to have your patches and helper macros to >achieve more condensed configs. And they don't have any impact on >existing configurations. The open question is, if there are any other >plans on modifying the configuration format. We had a lots of discussion >on that topic before. > >So before posting a new series, I'd say to just wait for further >comments; no need to hurry. Alright, sounds good. Let's see how this goes after more discussions. Thanks for your feedback! BR, Stephane. > >Thanks! > Ralf > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/PAXPR04MB88630D511CA5956C1BE79DA9A7709%40PAXPR04MB8863.eurprd04.prod.outlook.com.
