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.
