2016-07-19 21:11 GMT+08:00 Ralf Ramsauer <[email protected]>:
>
>
> On 07/06/2016 06:23 AM, Xuguo Wang wrote:
>> 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.
>>
>> v6:
>> - add all the commit messages and the changes info
>> - modify error comments
>> - modify self.comments to self.comments = comments or [] to avoid merging
>> conflict
>> - refactor error message more verbose
>> - modify f.read() to the f.read().splitlines() to void trailing '\n'
>> - remove pep8 warnings
>> - 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 | 421
>> +++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 374 insertions(+), 47 deletions(-)
>>
>> diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
>> index 4e61abc..dfe98db 100755
>> --- a/tools/jailhouse-config-create
>> +++ b/tools/jailhouse-config-create
>> @@ -368,22 +368,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 **:
> Is this standard docstring style or do you want to highlight something?
Sorry, this docstring style from a python document, it is not a standard
style, is there standard docstring style?
>> + :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
>> - if comments is None:
>> - self.comments = []
>> - else:
>> - self.comments = comments
>> +
>> +
>> +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=''):
>> @@ -400,6 +440,68 @@ 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.
>> +
>> + - ** parameters **:
>> + no.
>> + - ** type **:
>> + no.
> I'd remove those lines, parameters and type.
Since the docstring style file has this infomation, I will remove these lines.
>> + - ** 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
>> + - ** 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
>> @@ -416,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
>> @@ -434,6 +555,65 @@ 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.
>> +
>> + The parse regex is "( *)([0-9a-f]*)-([0-9a-f]*) : (\w?.*)".
> Sure, it is :-)
> I'd drop this line as well.
OK, accepted.
>> +
>> + - ** 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 ')]
>> @@ -457,38 +637,44 @@ 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.
>> +
>> + - ** parameters **:
>> + no.
>> + - ** type **:
>> + no.
> drop those lines
OK, accepted.
>> + - ** 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):
>> + if t.level > lastlevel:
>> t.parent = lastnode
>> - if (t.level == lastlevel):
>> + if t.level == lastlevel:
>> t.parent = lastnode.parent
>> - if (t.level < lastlevel):
>> + if t.level < lastlevel:
> Your patch still introduces non-functional cosmetic changes...
These above lines are original code, so I am afraid the author has
his own meanings. I will refactor these lines.
>> p = lastnode.parent
>> while(t.level < p.level):
>> p = p.parent
>> @@ -510,18 +696,32 @@ class IOMemRegionTree:
>> r = tree.region
>> s = r.typestr
>>
>> - if (s.find('HPET') >= 0):
>> + if s.find('HPET') >= 0:
>> regions.append(r)
>>
>> # if the tree continues recurse further down ...
>> - if (len(tree.children) > 0):
>> + if len(tree.children) > 0:
>> regions.extend(IOMemRegionTree.find_hpet_regions(tree))
>>
>> 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:
>> @@ -543,13 +743,13 @@ class IOMemRegionTree:
>> continue
>>
>> # generally blacklisted, unless we find an HPET behind it
>> - if (s == 'reserved'):
>> + if s == 'reserved':
>> regions.extend(IOMemRegionTree.find_hpet_regions(tree))
>> continue
>>
>> # if the tree continues recurse further down ...
>> - if (len(tree.children) > 0):
>> - regions.extend(IOMemRegionTree.parse_iomem_tree(tree))
>> + if len(tree.children) > 0:
>> + regions.extend(tree.parse_iomem_tree())
>> continue
>>
>> # add all remaining leaves
>> @@ -558,6 +758,132 @@ 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.
>> +
>> + - ** parameters **:
>> + no.
>> + - ** type **:
>> + no.
>> + - ** 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']
>> @@ -574,10 +900,9 @@ class IOMMUConfig(object):
>>
>>
>> def parse_iomem(pcidevices):
>> - regions = IOMemRegionTree.parse_iomem_tree(
>> - IOMemRegionTree.parse_iomem_file())
>> + regions = IOMemRegionTree.parse_iomem_file().parse_iomem_tree()
>>
>> - rom_region = MemRegion(0xc0000, 0xdffff, 'ROMs')
>> + rom_region = IOMemRegion(0xc0000, 0xdffff, 'ROMs')
>> add_rom_region = False
>>
>> ret = []
>> @@ -588,12 +913,12 @@ 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,
>> - r.typestr, r.comments)
>> + 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,
>> - r.stop, r.typestr, r.comments)
>> + tail_r = IOMemRegion(d.msix_address +
>> d.msix_region_size,
>> + r.stop, r.typestr, r.comments)
>> ret.append(tail_r)
>> append_r = False
>> break
>> @@ -667,11 +992,12 @@ 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,
>> - r.comments)
>> + tail_r = IOMemRegion(mem[0] + mem[1], r.stop,
>> + r.typestr, r.comments)
>> regions.insert(regions.index(r), tail_r)
>> regions.remove(r)
>> return mem
>> @@ -826,7 +1152,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)
>> @@ -1003,7 +1329,8 @@ 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 '
>> @@ -1064,7 +1391,7 @@ def get_cpu_vendor():
>> if cpuvendor is not None:
>> return cpuvendor
>> with input_open('/proc/cpuinfo', 'r') as f:
>> - for line in f:
>> + for line in f.read().splitlines():
>> if not line.strip():
>> continue
>> key, value = line.split(':')
>> @@ -1135,9 +1462,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()
>>
>
> --
> Ralf Ramsauer
> PGP: 0x8F10049B
--
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.
