OK, accepted. 2016-07-20 17:16 GMT+08:00 Henning Schild <[email protected]>: > Good to see that work on these patches is still ongoing! The result > looks good, i did not test it yet. But the code still needs some work. > > For now just one comment after looking at the patches. > > Am Wed, 20 Jul 2016 16:29:20 +0800 > schrieb Xuguo Wang <[email protected]>: > >> Abstract the common property of the ioports from the >> /proc/ioports as the super class named IOPortRegion, >> using the region of ioports from the IOPortRegionTree, >> this tree includes all of the ioport regions we could >> use this regions generate the pio_bitmap. >> >> v7: >> - modify the python docstrings style. >> - remove the cosmetic fixes. >> >> v6: >> - add all the commit message and the changes info >> - modify error comments >> - modify self.comments to self.comments = comments or [] to >> void merging conflict >> - refactor error message more verbose >> - modify f.read() to the f.read().splitlines() to >> void trailing '\n' >> - remove pep8 warning >> - refactor parse_ioport_tree to non-static method >> - add the docstring to new introduced classes and functions >> >> Signed-off-by: Xuguo Wang <[email protected]> >> --- >> tools/jailhouse-config-create | 373 >> ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 344 >> insertions(+), 29 deletions(-) >> >> diff --git a/tools/jailhouse-config-create >> b/tools/jailhouse-config-create index f0d65ed..348de60 100755 >> --- a/tools/jailhouse-config-create >> +++ b/tools/jailhouse-config-create >> @@ -372,19 +372,62 @@ class PCIPCIBridge(PCIDevice): >> return (secondbus, subordinate) >> >> >> -class MemRegion: >> - def __init__(self, start, stop, typestr, comments=None): >> +class IORegion: >> + """ >> + The super class of the IOPortRegion and IOMemRegion. >> + >> + The IORegion means a region of a /proc/io{mem,ports} entry, >> + which starts from param start, ends of the param stop. >> + >> + - attributes : >> + :param arg1: start start address >> + :param arg2: stop stop address >> + :param arg3: typestr region type description >> + - type : >> + :type arg1: long >> + :type arg2: long >> + :type arg3: str >> + - example : >> + /proc/ioports: >> + region = IORegion(0x0, 0x1f, "dma1") >> + /proc/iomem: >> + region = IORegion(0x0000, 0xfff, "reserved") >> + """ >> + >> + def __init__(self, start, stop, typestr): >> self.start = start >> self.stop = stop >> self.typestr = typestr >> + >> + >> +class IOMemRegion(IORegion): >> + """ >> + Each IOMemRegion represents one of the /proc/iomem entries. >> + It extends IORegion by the comments property. >> + >> + - attributes : >> + :param arg1: start region start address >> + :param arg2: stop region stop address >> + :param arg3: typestr region type description >> + :param arg4: comments region specified description >> + - type : >> + :type arg1: long >> + :type arg2: long >> + :type arg3: str >> + :type arg4: str >> + - example : >> + memregion = IOMemRegion(0x0000, 0xfff, "reserved", NULL) >> + """ >> + >> + def __init__(self, start, stop, typestr, comments=None): >> self.comments = comments or [] >> + IORegion.__init__(self, start, stop, typestr) >> >> def __str__(self): >> - return 'MemRegion: %08x-%08x : %s' % \ >> + return 'IOMemRegion: %08x-%08x : %s' % \ >> (self.start, self.stop, self.typestr) >> >> def size(self): >> - # round up to full PAGE_SIZE >> return int((self.stop - self.start + 0xfff) / 0x1000) * >> 0x1000 >> def flagstr(self, p=''): >> @@ -401,6 +444,64 @@ class MemRegion: >> return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE' >> >> >> +class IOPortRegion(IORegion): >> + """ >> + Each IOPortRegion represents one of the /proc/ioports entries. >> + The value == 0 means permit, 1 means intercept. >> + >> + - attributes : >> + :param arg1: start region start address >> + :param arg2: stop region stop address >> + :param arg3: typestr region type description >> + :param arg4: value region value description >> + - type : >> + :type arg1: long >> + :type arg2: long >> + :type arg3: str >> + :type arg4: int >> + - example : >> + portRegion = IOPortRegion(0x0, 0x1f, "dma1", -0x1) >> + """ >> + >> + def __init__(self, start, stop, typestr, value=0): >> + self.value = value >> + IORegion.__init__(self, start, stop, typestr) >> + >> + def size(self): >> + return self.stop - self.start + 1 >> + >> + def alignment(self): >> + """ >> + Return the region alignment. >> + >> + - return type : >> + int: the return value type. >> + - return value : >> + 0 means both are aligned >> + 1 means both head and tail are both not aligned >> + 2 means head not aligned but the tail is >> + 3 means tail not aligned but the head is > > These values should be class constants and i suggest using a bitmask. > i.e. (IOPortRegion.ALIGNED_T | IOPortRegion.ALIGNED_H) == 0x3 > so your 0 would be 3, your 1 would be 0 ... > > That should allow you to simplify the code dealing with that. And > making it much more readable! > >> + - example : >> + alignment = {$IOPortRegion}.alignment() >> + """ >> + # region head bit and tail bit are both aligned >> + t = 0 >> + # region head bit and tail bit are both not aligned >> + if self.start % 8 != 0 \ >> + and (self.stop % 8 + 1) % 8 != 0: >> + t = 1 >> + # region head bit is not aligned but tail do >> + elif self.start % 8 != 0 \ >> + and (self.stop % 8 + 1) % 8 == 0: >> + t = 2 >> + # region head bit is aligned but tail bit not >> + elif self.start % 8 == 0 \ >> + and (self.stop % 8 + 1) % 8 != 0: >> + t = 3 >> + >> + return t >> + >> + >> class IOAPIC: >> def __init__(self, id, address, gsi_base, iommu=0, bdf=0): >> self.id = id >> @@ -417,7 +518,26 @@ class IOAPIC: >> return (self.iommu << 16) | self.bdf >> >> >> -class IOMemRegionTree: >> +class IORegionTree: >> + """ >> + Super class of IOMemRegionTree and IOPortRegionTree. >> + >> + - attributes : >> + :param arg1: region region of a /proc/io{mem, ports} >> entry >> + :param arg2: level the level of region >> + - type : >> + :type arg1: IO{Port, Mem}Region >> + :type arg2: int >> + - example : >> + level, r = IOMemRegionTree.parse_line(IOMemRegion, line) >> + or >> + level, r = IOPortRegionTree.parse_line(IOPortRegion, line) >> + """ >> + >> + # This regex matches entries in /proc/ioports or /proc/iomem, >> like >> + # ' 0020-0021 : pic1' >> + IOEntry_Regex = re.compile('( *)([0-9a-f]*)-([0-9a-f]*) : >> (\w?.*)') + >> def __init__(self, region, level): >> self.region = region >> self.level = level >> @@ -435,6 +555,63 @@ class IOMemRegionTree: >> s += str(c) >> return s >> >> + @staticmethod >> + def parse_line(RegionClass, line): >> + """ >> + Parse an entry of /proc/iomem or /proc/ioports to an object >> + of region, this region type is specified by the regionclass. >> + >> + - parameters : >> + :param arg1: regionclass region type >> + :param arg2: line an entry of /proc/iomem or >> + /proc/ioports content >> + - type : >> + :type arg1: IO{Port, Mem}Region >> + :type arg2: str >> + - return type : >> + list: a list node contains tree node level and the >> region of entry >> + - return value : >> + list = [level, region] >> + - example : >> + level, r = IOMemRegionTree.parse_line(IOMemRegion, line) >> + or >> + level, r = IOPortRegionTree.parse_line(IOPortRegion, >> line) >> + """ >> + match = IORegionTree.IOEntry_Regex.match(line) >> + if not match: >> + raise ValueError('invalid entry: %s' % line) >> + >> + # blank level >> + level = int(match.group(1).count(' ') / 2) + 1 >> + # start address >> + start = match.group(2) >> + # end address >> + stop = match.group(3) >> + # comments >> + typestr = match.group(4) >> + return level, RegionClass(int(start, 16), int(stop, 16), >> typestr) + >> + >> +class IOMemRegionTree(IORegionTree): >> + """ >> + This class parses the /proc/iomem file to the tree structure and >> parse the >> + tree structure to a list contains all regions. >> + >> + - attributes : >> + :param arg1: region region of a /proc/iomem entry >> + :param arg2: level the level of region >> + - type : >> + :type arg1: IOMemRegion >> + :type arg2: int >> + - example : >> + level, r = IOMemRegionTree.parse_line(IOMemRegion, line) >> + or >> + level, r = IOPortRegionTree.parse_line(IOPortRegion, line) >> + """ >> + >> + def __init__(self, region, level): >> + IORegionTree.__init__(self, region, level) >> + >> def regions_split_by_kernel(self): >> kernel = [x for x in self.children if >> x.region.typestr.startswith('Kernel ')] >> @@ -458,32 +635,34 @@ class IOMemRegionTree: >> >> # before Kernel if any >> if (r.start < kernel_start): >> - before_kernel = MemRegion(r.start, kernel_start - 1, s) >> + before_kernel = IOMemRegion(r.start, kernel_start - 1, s) >> >> - kernel_region = MemRegion(kernel_start, kernel_stop, >> "Kernel") >> + kernel_region = IOMemRegion(kernel_start, kernel_stop, >> "Kernel") >> # after Kernel if any >> if (r.stop > kernel_stop): >> - after_kernel = MemRegion(kernel_stop + 1, r.stop, s) >> + after_kernel = IOMemRegion(kernel_stop + 1, r.stop, s) >> >> return [before_kernel, kernel_region, after_kernel] >> >> @staticmethod >> - def parse_iomem_line(line): >> - a = line.split(':', 1) >> - level = int(a[0].count(' ') / 2) + 1 >> - region = a[0].split('-', 1) >> - a[1] = a[1].strip() >> - return level, MemRegion(int(region[0], 16), int(region[1], >> 16), a[1]) - >> - @staticmethod >> def parse_iomem_file(): >> + """ >> + This function parses the file of /proc/iomem to a tree >> structure. + >> + - return type : >> + IOMemRegionTree: /proc/iomem all entries' tree root node. >> + - return value : >> + Return the tree root node. >> + - example : >> + regions = >> IOMemRegionTree.parse_iomem_file().parse_iomem_tree() >> + """ >> root = IOMemRegionTree(None, 0) >> f = input_open('/proc/iomem') >> lastlevel = 0 >> lastnode = root >> - for line in f: >> - (level, r) = IOMemRegionTree.parse_iomem_line(line) >> + for line in f.read().splitlines(): >> + level, r = IOMemRegionTree.parse_line(IOMemRegion, line) >> t = IOMemRegionTree(r, level) >> if (t.level > lastlevel): >> t.parent = lastnode >> @@ -521,8 +700,22 @@ class IOMemRegionTree: >> return regions >> >> # recurse down the tree >> - @staticmethod >> def parse_iomem_tree(tree): >> + """ >> + This function parses the IOMemRegionTree tree nodes to >> regions, >> + and return the list contains all regions. >> + >> + - parameters : >> + :param arg1: tree IOMemRegionTree tree root >> node >> + - type : >> + :type arg1: IOMemRegionTree >> + - return type : >> + list: contains all /proc/iomem tree nodes. >> + - return value : >> + Return the list regions of /proc/iomem. >> + - example : >> + regions = >> IOMemRegionTree.parse_iomem_file().parse_iomem_tree() >> + """ >> regions = [] >> >> for tree in tree.children: >> @@ -559,6 +752,128 @@ class IOMemRegionTree: >> return regions >> >> >> +class IOPortRegionTree(IORegionTree): >> + """ >> + This class parses the /proc/ioports file to the tree structure >> and parse >> + the tree structure to a list contains all regions. >> + >> + - attributes: >> + :param arg1: region region of a /proc/ioports entry >> + :param arg2: level the level of region tree node >> + - type : >> + :type arg1: IOPortRegion >> + :type arg2: int >> + - example : >> + level, r = IOMemRegionTree.parse_line(IOMemRegion, line) >> + or >> + level, r = IOPortRegionTree.parse_line(IOPortRegion, line) >> + """ >> + >> + def __init__(self, region, level): >> + IORegionTree.__init__(self, region, level) >> + >> + @staticmethod >> + def parse_ioport_file(): >> + """ >> + This function parses the file of /proc/ioports to a tree >> structure. + >> + - return type : >> + IOPortRegionTree: contains /proc/ioports tree structure. >> + - return value : >> + Return the tree root node. >> + - example : >> + regions = >> IOPortRegionTree.parse_ioport_file().parse_ioport_tree() >> + """ >> + root = IOPortRegionTree(None, 0) >> + f = input_open('/proc/ioports') >> + lastlevel = 0 >> + lastnode = root >> + for line in f.read().splitlines(): >> + level, r = IOPortRegionTree.parse_line(IOPortRegion, >> line) >> + t = IOPortRegionTree(r, level) >> + if t.level > lastlevel: >> + t.parent = lastnode >> + if t.level == lastlevel: >> + t.parent = lastnode.parent >> + if t.level < lastlevel: >> + p = lastnode.parent >> + while(t.level < p.level): >> + p = p.parent >> + t.parent = p.parent >> + >> + t.parent.children.append(t) >> + lastnode = t >> + lastlevel = t.level >> + f.close() >> + >> + return root >> + >> + # recurse down the tree >> + def parse_ioport_tree(tree): >> + """ >> + This function parses the IOPortRegionTree tree nodes to >> regions, >> + and return the list contains all regions. >> + >> + - parameters : >> + :param arg1: tree IOPortRegionTree tree root >> node >> + - type : >> + :type arg1: IOPortRegionTree >> + - return type : >> + list: contains all /proc/ioports tree nodes. >> + - return value : >> + Return the list regions of /proc/ioports. >> + - example : >> + regions = >> IOPortRegionTree.parse_ioport_file().parse_ioport_tree() >> + """ >> + regions = [] >> + >> + for tree in tree.children: >> + r = tree.region >> + c = r.typestr >> + >> + # intercept keyboard and PCI conf1 >> + if c is not None and (c == 'keyboard' or c == 'PCI >> conf1'): >> + r.value = IOPortRegionTree.get_first_ioport_byte( >> + r.start, r.stop) >> + >> + # if the tree continues recurse further down ... >> + if len(tree.children) > 0: >> + regions.extend(tree.parse_ioport_tree()) >> + continue >> + >> + # add all remaining leaves >> + regions.append(r) >> + >> + return regions >> + >> + @staticmethod >> + def get_first_ioport_byte(start_permitted, last_permitted): >> + """ >> + Get a byte of ioport region mask from start_permitted, end >> + at the last_permitted bit. >> + >> + - parameters : >> + :param arg1: start_permitted first permitted bit >> + :param arg2: last_permitted last permitted bit >> + - type : >> + :type arg1: int >> + :type arg2: int >> + - return type : >> + int: >> + - return value : >> + a bitwise mask start from start_permitted to >> last_permitted >> + - example : >> + value_head = IOPortRegionTree.get_first_ioport_byte( >> + region.start, region.stop) >> + """ >> + byte_head = start_permitted % 8 >> + byte_tail = last_permitted % 8 >> + byte = (1 << byte_head) - 1 >> + byte |= (~((1 << (byte_tail + 1)) - 1) & 0xff) >> + byte &= 0xff >> + return int(byte) >> + >> + >> class IOMMUConfig(object): >> def __init__(self, props): >> self.base_addr = props['base_addr'] >> @@ -578,7 +893,7 @@ def parse_iomem(pcidevices): >> regions = IOMemRegionTree.parse_iomem_tree( >> IOMemRegionTree.parse_iomem_file()) >> >> - rom_region = MemRegion(0xc0000, 0xdffff, 'ROMs') >> + rom_region = IOMemRegion(0xc0000, 0xdffff, 'ROMs') >> add_rom_region = False >> >> ret = [] >> @@ -589,11 +904,11 @@ def parse_iomem(pcidevices): >> for d in pcidevices: >> if d.msix_address >= r.start and d.msix_address <= >> r.stop: if d.msix_address > r.start: >> - head_r = MemRegion(r.start, d.msix_address - 1, >> + head_r = IOMemRegion(r.start, d.msix_address - 1, >> r.typestr, r.comments) >> ret.append(head_r) >> if d.msix_address + d.msix_region_size < r.stop: >> - tail_r = MemRegion(d.msix_address + >> d.msix_region_size, >> + tail_r = IOMemRegion(d.msix_address + >> d.msix_region_size, r.stop, r.typestr, r.comments) >> ret.append(tail_r) >> append_r = False >> @@ -668,10 +983,10 @@ def alloc_mem(regions, size): >> r.stop + 1 >= mem[0] + mem[1] >> ): >> if r.start < mem[0]: >> - head_r = MemRegion(r.start, mem[0] - 1, r.typestr, >> r.comments) >> + head_r = IOMemRegion(r.start, mem[0] - 1, r.typestr, >> r.comments) regions.insert(regions.index(r), head_r) >> if r.stop + 1 > mem[0] + mem[1]: >> - tail_r = MemRegion(mem[0] + mem[1], r.stop, >> r.typestr, >> + tail_r = IOMemRegion(mem[0] + mem[1], r.stop, >> r.typestr, r.comments) >> regions.insert(regions.index(r), tail_r) >> regions.remove(r) >> @@ -827,7 +1142,7 @@ def parse_dmar(pcidevices, ioapics, >> dmar_regions): comments.append('DMAR parser could not decode device >> path') offset += scope_len >> >> - reg = MemRegion(base, limit, 'ACPI DMAR RMRR', comments) >> + reg = IOMemRegion(base, limit, 'ACPI DMAR RMRR', >> comments) regions.append(reg) >> >> f.seek(struct_len - offset, os.SEEK_CUR) >> @@ -1004,7 +1319,7 @@ def parse_ivrs(pcidevices, ioapics): >> 'regions. The memory at 0x%x will be mapped >> accessible ' 'to all devices.' % mem_addr) >> >> - regions.append(MemRegion(mem_addr, mem_len, 'ACPI IVRS', >> comment)) >> + regions.append(IOMemRegion(mem_addr, mem_len, 'ACPI >> IVRS', comment)) elif type == 0x40: >> raise RuntimeError( >> 'You board uses IVRS Rev. 2 feature Jailhouse >> doesn\'t ' @@ -1136,9 +1451,9 @@ elif (total > ourmem[1]): >> >> hvmem[0] = ourmem[0] >> >> -inmatereg = MemRegion(ourmem[0] + hvmem[1], >> - ourmem[0] + hvmem[1] + inmatemem - 1, >> - 'JAILHOUSE Inmate Memory') >> +inmatereg = IOMemRegion(ourmem[0] + hvmem[1], >> + ourmem[0] + hvmem[1] + inmatemem - 1, >> + 'JAILHOUSE Inmate Memory') >> regions.append(inmatereg) >> >> cpucount = count_cpus() >
-- 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]. For more options, visit https://groups.google.com/d/optout.
