[PATCH V10 0/3] *** pio_bitmap generator ***

22 views
Skip to first unread message

Xuguo Wang

unread,
Aug 11, 2016, 4:10:10 AM8/11/16
to jailho...@googlegroups.com, jan.k...@web.de, valentine...@gmail.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com
***
V10:
Refactor the region alignment descriptor human
readable to make the function parse_ioports more
easy to maintain.
***

Xuguo Wang (3):
tools: Add the hierarchy of the IOPortRegion.
tools: Refactor the parse ioports function.
tools: Modify the template of the root cell config.

tools/jailhouse-config-create | 559 ++++++++++++++++++++++++++++++++++++++----
tools/root-cell-config.c.tmpl | 21 +-
2 files changed, 526 insertions(+), 54 deletions(-)

--
2.1.4

Xuguo Wang

unread,
Aug 11, 2016, 4:10:13 AM8/11/16
to jailho...@googlegroups.com, jan.k...@web.de, valentine...@gmail.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com
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.

V10:
- refactor the variable of alignment.
ALIGNED_H means head aligned
ALIGNED_T means tail aligned
ALL_ALIGNED = ALIGNED_H | ALIGNED_T means means head and tail
all aligned
ALL_NOT_ALIGNED = ALIGNED_H & ALIGNED_T means head and tail
are all not aligned

v9:
- add class constants describe the alignment category
ALIGNED_H means head aligned
ALIGNED_T means tail aligned
ALIGNED_H | ALIGNED_T means head and tail all aligned
-ALIGNED_H | -ALIGNED_T means head and tail are all not aligned

v8:
- remove the pep8 warning.

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 <hudd...@gmail.com>
---
tools/jailhouse-config-create | 389 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 357 insertions(+), 32 deletions(-)

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index f0d65ed..5cb28a7 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.
+
+ - args :
+ :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.
+
+ - args :
+ :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,72 @@ 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: ALIGNED_H region head aligned
+ :param arg2: ALIGNED_T region tail aligned
+ - args:
+ :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)
+ """
+
+ ALIGNED_T = 1
+ ALIGNED_H = 2
+ ALL_ALIGNED = ALIGNED_H | ALIGNED_T
+ ALL_NOT_ALIGNED = ALIGNED_H & ALIGNED_T
+
+ 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 head and tail are both not aligned
+ 1 means head not aligned but the tail is
+ 2 means tail not aligned but the head is
+ 3 means both are aligned
+ - example :
+ alignment = {$IOPortRegion}.alignment()
+ """
+ # region head bit and tail bit are both aligned
+ t = self.ALL_ALIGNED
+ # region head bit and tail bit are both not aligned
+ if self.start % 8 != 0 \
+ and (self.stop % 8 + 1) % 8 != 0:
+ t = self.ALL_NOT_ALIGNED
+ # region head bit is not aligned but tail do
+ elif self.start % 8 != 0 \
+ and (self.stop % 8 + 1) % 8 == 0:
+ t = self.ALIGNED_T
+ # region head bit is aligned but tail bit not
+ elif self.start % 8 == 0 \
+ and (self.stop % 8 + 1) % 8 != 0:
+ t = self.ALIGNED_H
+
+ return t
+
+
class IOAPIC:
def __init__(self, id, address, gsi_base, iommu=0, bdf=0):
self.id = id
@@ -417,7 +526,26 @@ class IOAPIC:
return (self.iommu << 16) | self.bdf


-class IOMemRegionTree:
+class IORegionTree:
+ """
+ Super class of IOMemRegionTree and IOPortRegionTree.
+
+ - args :
+ :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 +563,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.
+
+ - args :
+ :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 +643,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 +708,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.
+
+ - args :
+ :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 +760,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.
+
+ - args:
+ :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.
+
+ - args :
+ :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.
+
+ - args :
+ :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 +901,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,12 +912,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
@@ -668,11 +991,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
@@ -827,7 +1151,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 +1328,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 '
@@ -1136,9 +1461,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()
--
2.1.4

Xuguo Wang

unread,
Aug 11, 2016, 4:10:16 AM8/11/16
to jailho...@googlegroups.com, jan.k...@web.de, valentine...@gmail.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com
Refactoring the algorithm to the generate the .pio_bitmap using
the ioports entries and new parameters for the
root-cell-config.c-tmpl.

V10:
- refactor the way how to process alignement region.

v9:
- modify the parse_ioport alignment bitmask.

v8:
- remove the pep8 warning.

v7:
- fix the merge conflicts.

v6:
- refactor the static function of parse_ioport_tree to non-static
- refactor the function of parse_ioport
- add the docstrings to the function of parse_ioport

Signed-off-by: Xuguo Wang <hudd...@gmail.com>
---
tools/jailhouse-config-create | 170 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 161 insertions(+), 9 deletions(-)

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index 5cb28a7..e4b1565 100755
--- a/tools/jailhouse-config-create
+++ b/tools/jailhouse-config-create
@@ -1351,14 +1351,163 @@ def parse_ivrs(pcidevices, ioapics):


def parse_ioports():
+ """
+ This function parses the file content of /proc/ioports to the data
+ formatted of pio_bitmap need.
+
+ - return type :
+ long: base address of pm_timer_base
+ list: contains all /proc/ioports entries
+ - return value :
+ pm_timer_base: the base address of pm_timer_base
+ bitmap_regions: pio_bitmap list
+ - example :
+ (pm_timer_base, pm_bitmap_regions) = parse_ioports()
+ """
pm_timer_base = None
- f = input_open('/proc/ioports')
- for line in f:
- if line.endswith('ACPI PM_TMR\n'):
- pm_timer_base = int(line.split('-')[0], 16)
- break
- f.close()
- return pm_timer_base
+ bitmap_regions = []
+ regions = IOPortRegionTree.parse_ioport_file().parse_ioport_tree()
+
+ for region in regions:
+ # parse the ACPI PM_TMR
+ if region.typestr == 'ACPI PM_TMR':
+ pm_timer_base = region.start
+ continue
+
+ # region PCI conf1 must intercept
+ if region.typestr == 'PCI conf1':
+ region.value = 0xff
+ if region.size() > 8:
+ region.value = -1
+ bitmap_regions.append(region)
+ continue
+
+ byte_size = region.size() / 8
+
+ alignment = region.alignment()
+
+ if alignment == IOPortRegion.ALL_ALIGNED:
+ region.value = 0
+ bitmap_regions.append(region)
+ continue
+
+ if not alignment & IOPortRegion.ALIGNED_H:
+ round_up = (region.start % 8)
+ if not alignment & IOPortRegion.ALIGNED_T:
+ round_down = 7 - (region.stop % 8)
+
+ if byte_size <= 1:
+ value_head = IOPortRegionTree.get_first_ioport_byte(
+ region.start, region.stop)
+ if alignment & IOPortRegion.ALIGNED_H:
+ region.value = value_head
+ else:
+ if not alignment & IOPortRegion.ALIGNED_H:
+ value_head = IOPortRegionTree.get_first_ioport_byte(
+ region.start, region.start + (7 - round_up))
+ value_tail = IOPortRegionTree.get_first_ioport_byte(
+ region.stop - (region.stop % 8), region.stop)
+
+ # adjust the region address to make the region aligned
+ if not alignment & IOPortRegion.ALIGNED_H:
+ region.start -= round_up
+ if not alignment & IOPortRegion.ALIGNED_T:
+ region.stop += round_down
+
+ # the region overlap with the previous region,
+ # so have to modify value and typestr
+ if not alignment & IOPortRegion.ALIGNED_H:
+ if region.start - round_up < bitmap_regions[-1].stop:
+ region.value = bitmap_regions[-1].value & value_head
+ region.typestr += ', ' + bitmap_regions[-1].typestr
+ bitmap_regions.pop()
+ else:
+ region.value = value_head
+
+ # append the region according to the region overlap with the
+ # previous region or not
+ if byte_size <= 1:
+ bitmap_regions.append(region)
+ elif byte_size == 2:
+ if not alignment & IOPortRegion.ALIGNED_H:
+ stop = region.start + (7 - round_up)
+ value = region.value
+ else:
+ stop = region.start + 7
+ value = 0
+
+ bitmap_regions.append(IOPortRegion(
+ region.start,
+ stop,
+ region.typestr,
+ value))
+ bitmap_regions.append(IOPortRegion(
+ region.stop - (region.stop % 8),
+ region.stop,
+ region.typestr,
+ value_tail))
+ else:
+ if not alignment & IOPortRegion.ALIGNED_H:
+ stop = region.start + (7 - round_up)
+ value = region.value
+ else:
+ stop = region.stop - (region.stop % 8) - 1
+ value = 0
+
+ bitmap_regions.append(IOPortRegion(
+ region.start,
+ stop,
+ region.typestr,
+ value))
+
+ if alignment == IOPortRegion.ALL_NOT_ALIGNED:
+ bitmap_regions.append(IOPortRegion(
+ region.start + (7 - round_up) + 1,
+ region.stop - (region.stop % 8) - 1,
+ region.typestr,
+ 0))
+
+ if not alignment & IOPortRegion.ALIGNED_T:
+ start = region.stop - (region.stop % 8)
+ value = value_tail
+ else:
+ start = region.start + (7 - round_up) + 1
+ value = 0
+
+ bitmap_regions.append(IOPortRegion(
+ start,
+ region.stop,
+ region.typestr,
+ value))
+
+ # replenishment the region hole
+ for region in bitmap_regions:
+ i = bitmap_regions.index(region)
+ if region.start != 0 \
+ and region.stop != 0xffff \
+ and region != bitmap_regions[-1]:
+ if region.stop + 1 != bitmap_regions[i + 1].start:
+ size = bitmap_regions[i + 1].start - region.stop - 1
+ value = 0xff
+ if size > 8:
+ value = -1
+ bitmap_regions.insert(i + 1,
+ IOPortRegion(
+ region.stop + 1,
+ bitmap_regions[i + 1].start - 1,
+ '',
+ value))
+
+ # pad with the 0xff if the last address is not 0xffff
+ if bitmap_regions[-1].stop != 0xffff:
+ start = bitmap_regions[-1].stop + 1
+ stop = 0xffff
+ value = 0xff
+ if stop - start - 1 > 8:
+ value = -1
+ bitmap_regions.append(IOPortRegion(start, stop, '', value))
+
+ return pm_timer_base, bitmap_regions


class MMConfig:
@@ -1468,7 +1617,8 @@ regions.append(inmatereg)

cpucount = count_cpus()

-pm_timer_base = parse_ioports()
+(pm_timer_base, pm_bitmap_regions) = parse_ioports()
+pm_bitmap_size = int(pm_bitmap_regions[-1].stop / 8 + 1)


f = open(options.file, 'w')
@@ -1486,7 +1636,9 @@ kwargs = {
'irqchips': ioapics,
'pm_timer_base': pm_timer_base,
'mmconfig': mmconfig,
- 'iommu_units': iommu_units
+ 'iommu_units': iommu_units,
+ 'pm_bitmap_size': pm_bitmap_size,
+ 'pm_bitmap_regions': pm_bitmap_regions
}

f.write(tmpl.render(**kwargs))
--
2.1.4

Xuguo Wang

unread,
Aug 11, 2016, 4:10:19 AM8/11/16
to jailho...@googlegroups.com, jan.k...@web.de, valentine...@gmail.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com
Since the parse_ioports function be refactored, the template
of the root cell must make a modification to meet the parame-
ters changes.

v7:
- fix the merge conflicts.

Signed-off-by: Xuguo Wang <hudd...@gmail.com>
---
tools/root-cell-config.c.tmpl | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index a864fb6..73127c4 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -49,7 +49,7 @@ struct {
__u64 cpus[${int((cpucount + 63) / 64)}];
struct jailhouse_memory mem_regions[${len(regions)}];
struct jailhouse_irqchip irqchips[${len(irqchips)}];
- __u8 pio_bitmap[0x2000];
+ __u8 pio_bitmap[${pm_bitmap_size}];
struct jailhouse_pci_device pci_devices[${len(pcidevices)}];
struct jailhouse_pci_capability pci_caps[${len(pcicaps)}];
} __attribute__((packed)) config = {
@@ -133,18 +133,13 @@ struct {
},

.pio_bitmap = {
- [ 0/8 ... 0x3f/8] = -1,
- [ 0x40/8 ... 0x47/8] = 0xf0, /* PIT */
- [ 0x48/8 ... 0x5f/8] = -1,
- [ 0x60/8 ... 0x67/8] = 0xec, /* HACK: NMI status/control */
- [ 0x68/8 ... 0x6f/8] = -1,
- [ 0x70/8 ... 0x77/8] = 0xfc, /* RTC */
- [ 0x78/8 ... 0x7f/8] = -1,
- [ 0x80/8 ... 0x87/8] = 0xfe, /* Linux: native_io_delay() */
- [ 0x88/8 ... 0x3af/8] = -1,
- [ 0x3b0/8 ... 0x3df/8] = 0x00, /* VGA */
- [ 0x3e0/8 ... 0xcff/8] = -1,
- [ 0xd00/8 ... 0xffff/8] = 0, /* HACK: PCI bus */
+ % for k in pm_bitmap_regions:
+ % if k.typestr != '':
+ [${' 0x%04x' % k.start}/8 ... ${'0x%04x' % k.stop}/8] = ${hex(k.value)}, /* ${k.typestr} */
+ % else:
+ [${' 0x%04x' % k.start}/8 ... ${'0x%04x' % k.stop}/8] = ${hex(k.value)},
+ % endif
+ % endfor
},

.pci_devices = {
--
2.1.4

Valentine Sinitsyn

unread,
Aug 13, 2016, 3:22:29 AM8/13/16
to Xuguo Wang, jailho...@googlegroups.com, jan.k...@web.de, ra...@ramses-pyramidenbau.de, henning...@siemens.com
Nit: Is this '- args :' yntax standard? I've never encountered it.
Docstring syntax is usually kept machine (e.g. Sphinx) readable, and
it's better not to introduce any constructs it doesn't understand.
Maybe just 0?

> +
> + 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 head and tail are both not aligned
> + 1 means head not aligned but the tail is
> + 2 means tail not aligned but the head is
> + 3 means both are aligned
:returns: or :return:, see
http://www.sphinx-doc.org/en/stable/domains.html#python-signatures
Valentine

charles king

unread,
Aug 17, 2016, 4:40:41 AM8/17/16
to Valentine Sinitsyn, jailho...@googlegroups.com, Jan Kiszka, Ralf Ramsauer, Henning Schild
Here the ALL_NOT_ALIGNED if assigned to 0, that looks like
so unexpected, look likes nothing connect to the ALIGNED_T
and ALIGNED_H, this is better, what do you think?

Xuguo Wang

unread,
Aug 17, 2016, 4:43:12 AM8/17/16
to valentine...@gmail.com, jailho...@googlegroups.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com, jan.k...@web.de
***
V11:
modify the docstrings to fit the PEP 257.

Xuguo Wang

unread,
Aug 17, 2016, 4:43:15 AM8/17/16
to valentine...@gmail.com, jailho...@googlegroups.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com, jan.k...@web.de
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.

V11:
- modify the docstring to fit the PEP 257.
tools/jailhouse-config-create | 321 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 289 insertions(+), 32 deletions(-)

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index f0d65ed..397501b 100755
--- a/tools/jailhouse-config-create
+++ b/tools/jailhouse-config-create
@@ -372,19 +372,46 @@ 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.
+
+ args:
+ :param long start: start address
+ :param long stop: stop address
+ :param str typestr: region type description
+ """
+
+ 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.
+
+ args:
+ :param long start: region start address
+ :param long stop: region stop address
+ :param str typestr: region type description
+ :param str comments: region specified description
+ """
+
+ 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 +428,65 @@ 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 byte ALIGNED_H: region head aligned
+ :param byte ALIGNED_T: region tail aligned
+ :param byte ALL_ALIGNED: region head and tail all aligned
+ :param byte ALL_NOT_ALIGNED: region head and tail all not aligned
+ args:
+ :param long start: region start address
+ :param long stop: region stop address
+ :param str typestr: region type description
+ :param str value: region value description
+ """
+
+ ALIGNED_T = 1
+ ALIGNED_H = 2
+ ALL_ALIGNED = ALIGNED_H | ALIGNED_T
+ ALL_NOT_ALIGNED = ALIGNED_H & ALIGNED_T
+
+ 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.
+
+ rtype:
+ int
+ returns:
+ 0 means both head and tail are both not aligned
+ 1 means head not aligned but the tail is
+ 2 means tail not aligned but the head is
+ 3 means both are aligned
+ """
+ # region head bit and tail bit are both aligned
+ t = self.ALL_ALIGNED
+ # region head bit and tail bit are both not aligned
+ if self.start % 8 != 0 \
+ and (self.stop % 8 + 1) % 8 != 0:
+ t = self.ALL_NOT_ALIGNED
+ # region head bit is not aligned but tail do
+ elif self.start % 8 != 0 \
+ and (self.stop % 8 + 1) % 8 == 0:
+ t = self.ALIGNED_T
+ # region head bit is aligned but tail bit not
+ elif self.start % 8 == 0 \
+ and (self.stop % 8 + 1) % 8 != 0:
+ t = self.ALIGNED_H
+
+ return t
+
+
class IOAPIC:
def __init__(self, id, address, gsi_base, iommu=0, bdf=0):
self.id = id
@@ -417,7 +503,20 @@ class IOAPIC:
return (self.iommu << 16) | self.bdf


-class IOMemRegionTree:
+class IORegionTree:
+ """
+ Super class of IOMemRegionTree and IOPortRegionTree.
+
+ args:
+ :param IO{Port, Mem}Region region: region of a \
+ /proc/io{mem, ports} entry
+ :param int level: the level of region
+ """
+
+ # 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 +534,49 @@ 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 IO{Port, Mem}Region: regionclass region type
+ :param str line: an entry of /proc/iomem or \
+ /proc/ioports content
+ rtype:
+ list
+ return:
+ list = [level, region]
+ """
+ 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.
+
+ args:
+ :param IOMemRegion region: region of a /proc/iomem entry
+ :param int level: the level of region
+ """
+
+ 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 +600,32 @@ 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.
+
+ rtype :
+ IOMemRegionTree
+ return:
+ Return the tree root node.
+ """
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 +663,18 @@ 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.
+
+ arg:
+ :param IOMemRegionTree tree: IOMemRegionTree tree root node
+ rtype:
+ list
+ return:
+ Return the list regions of /proc/iomem
+ """
regions = []

for tree in tree.children:
@@ -559,6 +711,109 @@ 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.
+
+ args:
+ :param IOPortRegion region: region of a /proc/ioports entry
+ :param int level: the level of region tree node
+ """
+
+ 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.
+
+ rtype:
+ IOPortRegionTree
+ return:
+ Return the tree root node.
+ """
+ arg:
+ :param IOPortRegionTree tree: IOPortRegionTree tree root node
+ rtype:
+ list
+ return:
+ Return the list regions of /proc/ioports.
+ """
+ args:
+ :param int start_permitted: first permitted bit
+ :param int last_permitted: last permitted bit
+ rtype:
+ int
+ return:
+ a bitwise mask start from start_permitted to last_permitted
+ """
+ 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 +833,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,12 +844,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
@@ -668,11 +923,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
@@ -827,7 +1083,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 +1260,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 '
@@ -1136,9 +1393,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()
--
2.5.0

Xuguo Wang

unread,
Aug 17, 2016, 4:43:19 AM8/17/16
to valentine...@gmail.com, jailho...@googlegroups.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com, jan.k...@web.de
Refactoring the algorithm to the generate the .pio_bitmap using
the ioports entries and new parameters for the
root-cell-config.c-tmpl.

V11:
- modify the docstring to fit the PEP 257

V10:
- refactor the way how to process alignement region.

v9:
- modify the parse_ioport alignment bitmask.

v8:
- remove the pep8 warning.

v7:
- fix the merge conflicts.

v6:
- refactor the static function of parse_ioport_tree to non-static
- refactor the function of parse_ioport
- add the docstrings to the function of parse_ioport

Signed-off-by: Xuguo Wang <hudd...@gmail.com>
---
tools/jailhouse-config-create | 168 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 159 insertions(+), 9 deletions(-)

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index 397501b..2c48e06 100755
--- a/tools/jailhouse-config-create
+++ b/tools/jailhouse-config-create
@@ -1283,14 +1283,161 @@ def parse_ivrs(pcidevices, ioapics):


def parse_ioports():
+ """
+ This function parses the file content of /proc/ioports to the data
+ formatted of pio_bitmap need.
+
+ rtype:
+ long
+ list
+ return:
+ pm_timer_base: the base address of pm_timer_base
+ bitmap_regions: pio_bitmap list
+ """
pm_timer_base = None
- f = input_open('/proc/ioports')
- for line in f:
- if line.endswith('ACPI PM_TMR\n'):
- pm_timer_base = int(line.split('-')[0], 16)
- break
- f.close()
- return pm_timer_base
+ bitmap_regions = []
+ regions = IOPortRegionTree.parse_ioport_file().parse_ioport_tree()
+
+ for region in regions:
+ # parse the ACPI PM_TMR
+ if region.typestr == 'ACPI PM_TMR':
+ pm_timer_base = region.start
+ continue
+
+ # region PCI conf1 must intercept
+ if region.typestr == 'PCI conf1':
+ region.value = 0xff
+ if region.size() > 8:
+ region.value = -1
+ bitmap_regions.append(region)
+ continue
+
+ byte_size = region.size() / 8
+
+ alignment = region.alignment()
+
+ if alignment == IOPortRegion.ALL_ALIGNED:
+ region.value = 0
+ bitmap_regions.append(region)
+ continue
+
+ if not alignment & IOPortRegion.ALIGNED_H:
+ round_up = (region.start % 8)
+ if not alignment & IOPortRegion.ALIGNED_T:
+ round_down = 7 - (region.stop % 8)
+
+ if byte_size <= 1:
+ value_head = IOPortRegionTree.get_first_ioport_byte(
+ region.start, region.stop)
+ if alignment & IOPortRegion.ALIGNED_H:
+ region.value = value_head
+ else:
+ if not alignment & IOPortRegion.ALIGNED_H:
+ value_head = IOPortRegionTree.get_first_ioport_byte(
@@ -1400,7 +1547,8 @@ regions.append(inmatereg)

cpucount = count_cpus()

-pm_timer_base = parse_ioports()
+(pm_timer_base, pm_bitmap_regions) = parse_ioports()
+pm_bitmap_size = int(pm_bitmap_regions[-1].stop / 8 + 1)


f = open(options.file, 'w')
@@ -1418,7 +1566,9 @@ kwargs = {
'irqchips': ioapics,
'pm_timer_base': pm_timer_base,
'mmconfig': mmconfig,
- 'iommu_units': iommu_units
+ 'iommu_units': iommu_units,
+ 'pm_bitmap_size': pm_bitmap_size,
+ 'pm_bitmap_regions': pm_bitmap_regions
}

f.write(tmpl.render(**kwargs))
--
2.5.0

Xuguo Wang

unread,
Aug 17, 2016, 4:43:22 AM8/17/16
to valentine...@gmail.com, jailho...@googlegroups.com, ra...@ramses-pyramidenbau.de, henning...@siemens.com, jan.k...@web.de
Since the parse_ioports function be refactored, the template
of the root cell must make a modification to meet the parame-
ters changes.

v7:
- fix the merge conflicts.

Signed-off-by: Xuguo Wang <hudd...@gmail.com>
---
2.5.0

Valentine Sinitsyn

unread,
Aug 17, 2016, 4:48:53 AM8/17/16
to charles king, jailho...@googlegroups.com, Jan Kiszka, Ralf Ramsauer, Henning Schild
On 17.08.2016 13:40, charles king wrote:
> 2016-08-13 15:22 GMT+08:00 Valentine Sinitsyn <valentine...@gmail.com>:
>> On 12.08.2016 01:10, 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.
...
I feel that plain 0 is more straightforward, yet is a minor remark. You
don't have to fix it if you feel another way.

Valentine

charles king

unread,
Aug 17, 2016, 4:55:15 AM8/17/16
to Valentine Sinitsyn, jailho...@googlegroups.com, Jan Kiszka, Ralf Ramsauer, Henning Schild
Yes, indeed, Thank you for you reply.

best regards
from Xuguo Wang

Henning Schild

unread,
Aug 17, 2016, 5:00:37 AM8/17/16
to charles king, Valentine Sinitsyn, jailho...@googlegroups.com
Am Wed, 17 Aug 2016 16:40:39 +0800
schrieb charles king <hudd...@gmail.com>:
The point is you do not even need these names. That 0 means no
alignment is not surprising at all. And 1 & 2 is 0.
def alignment(self):
t = self.ALIGNED_T | self.ALIGNED_H
if (self.start % 8):
t -= self.ALIGNED_H
if ((self.stop + 1) % 8):
t -= self.ALIGNED_T
return t

that should be axactly the same with two simple conditions and 4
possible return values that do not need names to be readable, not
tested ...

charles king

unread,
Aug 17, 2016, 5:23:57 AM8/17/16
to Henning Schild, Valentine Sinitsyn, jailho...@googlegroups.com
Yes, I tested this right now, it is the same results, But I think the
upper is more
readable, and the two process are both need two judgments, so the efficiency are
the same. So what do you think we have to use the another one?

charles king

unread,
Sep 2, 2016, 9:02:28 PM9/2/16
to Henning Schild, Valentine Sinitsyn, jailho...@googlegroups.com
Hi,
All, How about this patch? Accept or reject? If reject I could do modify
again, if not, please tell me. Thanks all.

best regards
from Xuguo Wang

Valentine Sinitsyn

unread,
Sep 3, 2016, 7:41:06 AM9/3/16
to charles king, Henning Schild, jailho...@googlegroups.com
Hi all,

On 03.09.2016 06:02, charles king wrote:
> Hi,
> All, How about this patch? Accept or reject? If reject I could do modify
> again, if not, please tell me. Thanks all.
Unfortunately, I'll be unable to review this patch set in the near future.

Ralf and Henning did a great job spotting smelly parts, and I'm sure
their valuable feedback will be enough to put your changes into the
mergeable state.

My apologizes.

Best,
Valentine

Henning Schild

unread,
Sep 6, 2016, 8:44:25 AM9/6/16
to charles king, Valentine Sinitsyn, jailho...@googlegroups.com
Hey Xuguo Wang,

i am also busy with other projects at the moment. I know it took a long
time already and many iterations, but i am afraid we are still not very
close to accept.

This code is important since in can improve security a lot. Especially
by getting rid of "HACK: PCI bus". But it also has the potential to
generate a config that will not work and get new users frustrated. For
starting off with jailhouse the HACKs are ok, but for security
hardening a review of the pio_bitmap is required. And a tool to help
with that will be very useful!

If you look back at the reviews you got, i am sure you failed to
address some points. And that made the review process slow and
maybe even frustrating on both sides with a lot of iterations.

Keep sending your patches as they improve, maybe next month we can join
efforts and see if we can speed up the process by working together.

Henning


Am Sat, 3 Sep 2016 09:02:26 +0800

charles king

unread,
Sep 6, 2016, 9:36:09 AM9/6/16
to Henning Schild, Valentine Sinitsyn, jailho...@googlegroups.com
Hi,
Henning, I already have made tests in some platforms with the
generator config , not so much platforms, since
I have not so many, it works fine from now. I do not understand the
potential not work what this means? could you
tell me clearly?
I check the mail since the day of 2016-2-1, some points I have not
answer, but I already modify , and
give the remarks in the cover-letter, What you said is right, that
confused and frustrating the iteration, but recently, all
the points I have already addressed. I will remember what you told,
address to every point.
I want to improve the patches, could you give me some advises?
OK, before we get together work, I will keep refactoring the
patches, hope speed up the process.

best regards
from Xuguo Wang

Reply all
Reply to author
Forward
0 new messages