[PATCH 0/3] *** jailhouse cell config generator ***

[Xuguo Wang]

*** jailhouse cell config generator ***

Xuguo Wang (3):
tools: Adjust the hierarchy of the MemRegion and PortRegion.
tools: Redefine the parse ioports function.
tools: Modify the root-cell-config template.

tools/jailhouse-config-create | 304 +++++++++++++++++++++++++++++++++++++-----
tools/root-cell-config.c.tmpl | 21 ++-
2 files changed, 277 insertions(+), 48 deletions(-)

--
2.5.0

[Xuguo Wang]

Abstract the common property of the MemRegion and PortRegion
to the super class IORegion, named IOMemRegion and IOPortRegion
for parsing the /proc/ioports, the same action of the MemRegion-
Tree.

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

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index 4e61abc..9c6cf57 100755
--- a/tools/jailhouse-config-create
+++ b/tools/jailhouse-config-create
@@ -368,18 +368,31 @@ class PCIPCIBridge(PCIDevice):
return (secondbus, subordinate)


-class MemRegion:
- def __init__(self, start, stop, typestr, comments=None):
+# super class of IOMemRegion and IOPortRegion
+class IORegion:
+ def __init__(self, start, stop, comments=None):
self.start = start
self.stop = stop
- self.typestr = typestr
+
if comments is None:
self.comments = []
else:
self.comments = comments

def __str__(self):
- return 'MemRegion: %08x-%08x : %s' % \
+ return 'IORegion: %08x-%08x : %s' % \
+ (self.start, self.stop, self.comments)
+
+
+# IOMemRegion includes the one of the /proc/iomem entry.
+# IOMemRegion has his special priority typestr.
+class IOMemRegion(IORegion):
+ def __init__(self, start, stop, typestr, comments=None):
+ self.typestr = typestr
+ IORegion.__init__(self, start, stop, comments)
+
+ def __str__(self):
+ return 'IOMemRegion: %08x-%08x : %s' % \
(self.start, self.stop, self.typestr)

def size(self):
@@ -400,6 +413,18 @@ class MemRegion:
return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE'


+# IOPortRegion includes one of the /proc/ioports entry.
+# IOPortRegion has his special priority value.
+class IOPortRegion(IORegion):
+ def __init__(self, start, stop, value, comments=None):
+ self.value = value
+ IORegion.__init__(self, start, stop, comments)
+
+ def size(self):
+ # plus 1 more for len since the self.start start from 0
+ return int(self.stop - self.start) + 1
+
+
class IOAPIC:
def __init__(self, id, address, gsi_base, iommu=0, bdf=0):
self.id = id
@@ -416,7 +441,8 @@ class IOAPIC:
return (self.iommu << 16) | self.bdf


-class IOMemRegionTree:
+# super class of IOMemRegionTree and IOPortRegionTree
+class IORegionTree:
def __init__(self, region, level):
self.region = region
self.level = level
@@ -425,15 +451,24 @@ class IOMemRegionTree:

def __str__(self):
s = ''
- if (self.region):
+ if (self):
s = (' ' * (self.level - 1)) + str(self.region)
if self.parent and self.parent.region:
- s += ' --> ' + self.parent.region.typestr
+ if hasattr(self.parent.region, 'typestr'):
+ s += ' --> ' + self.parent.region.typestr
+ else:
+ s += ' --> ' + self.parent.region.comments
s += '\n'
for c in self.children:
s += str(c)
return s

+
+# parse the IOMemRegions to the tree structure.
+class IOMemRegionTree(IORegionTree):
+ 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,23 +492,24 @@ class IOMemRegionTree:

# before Kernel if any
if (r.start < kernel_start):
- before_kernel = MemRegion(r.start, kernel_start - 1, s)
+ # only the IOMemRegion should split the kernel
+ 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):
+ def parse_ioregion_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])
+ return level, IOMemRegion(int(region[0], 16), int(region[1], 16), a[1])

@staticmethod
def parse_iomem_file():
@@ -482,7 +518,7 @@ class IOMemRegionTree:
lastlevel = 0
lastnode = root
for line in f:
- (level, r) = IOMemRegionTree.parse_iomem_line(line)
+ (level, r) = IOMemRegionTree.parse_ioregion_line(line)
t = IOMemRegionTree(r, level)
if (t.level > lastlevel):
t.parent = lastnode
@@ -558,6 +594,97 @@ class IOMemRegionTree:
return regions


+# parse the IOPortRegion to the tree structure.
+class IOPortRegionTree(IORegionTree):
+ def __init__(self, region, level):
+ IORegionTree.__init__(self, region, level)
+
+ @staticmethod
+ def parse_ioregion_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, IOPortRegion(
+ int(region[0], 16), int(region[1], 16), 0, a[1])
+
+ @staticmethod
+ def parse_ioport_file():
+ root = IOPortRegionTree(None, 0)
+ f = input_open('/proc/ioports')
+ lastlevel = 0
+ lastnode = root
+ for line in f:
+ (level, r) = IOPortRegionTree.parse_ioregion_line(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
+ @staticmethod
+ def parse_ioport_tree(tree):
+ regions = []
+
+ for tree in tree.children:
+ r = tree.region
+ s = r.value
+ c = r.comments
+
+ # set keyboard and PCI conf1 disable, others enable
+ if (c is not None and (c == 'keyboard' or c == 'PCI conf1')):
+ r.value = IOPortRegionTree.parse_ioport_value(
+ r.start, r.stop + 1)
+
+ # if the tree continues recurse further down ...
+ if (len(tree.children) > 0):
+ regions.extend(IOPortRegionTree.parse_ioport_tree(tree))
+ continue
+
+ # add all remaining leaves
+ regions.append(r)
+
+ return regions
+
+ @staticmethod
+ def parse_ioport_value(start, stop):
+ size = stop - start
+ value = -1
+ if size > 8:
+ value = -1
+ elif size == 8:
+ value = 0xff
+ elif size == 7:
+ value = 0x7f
+ elif size == 6:
+ value = 0x3f
+ elif size == 5:
+ value = 0x1f
+ elif size == 4:
+ value = 0x0f
+ elif size == 3:
+ value = 0x07
+ elif size == 2:
+ value = 0x03
+ else:
+ value = 0x01
+
+ return value
+
+
class IOMMUConfig(object):
def __init__(self, props):
self.base_addr = props['base_addr']
@@ -577,7 +704,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 = []
@@ -588,12 +715,14 @@ 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 +796,14 @@ 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 +958,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 +1135,9 @@ 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 '
@@ -1135,9 +1269,10 @@ 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]

Transferring the /proc/ioports of the target system into
.pio_bitmap of the configuration that "jailhouse config
create" generates. Since the old configuration is generated
by the manual and actually everyone's computer setting is
different.

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

tools/jailhouse-config-create | 117 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 108 insertions(+), 9 deletions(-)

diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
index 9c6cf57..50ffd68 100755
--- a/tools/jailhouse-config-create
+++ b/tools/jailhouse-config-create
@@ -1158,15 +1158,111 @@ def parse_ivrs(pcidevices, ioapics):
return units, regions


+# parse the ioports by the /proc/ioports
def 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
+ pm_regions = []
+ regions = IOPortRegionTree.parse_ioport_tree(
+ IOPortRegionTree.parse_ioport_file())
+
+ for region in regions:
+ # parse the ACPI PM_TMR
+ if region.comments == 'ACPI PM_TMR':
+ pm_timer_base = region.start
+ continue
+
+ # padding the address
+ # [0x20 ... 0x21][padding][0x40 ... 43]
+ if len(pm_regions) and pm_regions[-1].stop + 1 < region.start:
+ value = IOPortRegionTree.parse_ioport_value(
+ pm_regions[-1].stop, region.start - 1)
+ pm_regions.append(IOPortRegion(
+ pm_regions[-1].stop + 1, region.start - 1, value))
+
+ # align with the 8 bytes, without the first region
+ if region.start != 0:
+ # formatted address
+ # [0x20 ... 0x27][0x28 ... 0x2f]
+ # append this region
+ if region.start == pm_regions[-1].stop + 1 and \
+ region.start % 8 == 0 and region.size() % 8 == 0:
+ pm_regions.append(IOPortRegion(
+ region.start, region.stop, region.value, region.comments))
+ continue
+
+ # size < 8
+ # [0x20 ... 0x21] --> [0x20 ... 0x27]
+ # then append this region
+ if region.start == pm_regions[-1].stop + 1 and \
+ region.start % 8 == 0 and region.size() < 8:
+ value = 0xff
+ region.value = value & ~IOPortRegionTree.parse_ioport_value(
+ 0, region.size())
+ region.stop = region.start + 7
+ pm_regions.append(IOPortRegion(
+ region.start, region.stop, region.value, region.comments))
+ continue
+
+ # size > 8 but the size of region size is not devided by 8
+ # [0x20 ... 0x36] --> [0x20 ... 0x37]
+ # then append this region
+ if region.start == pm_regions[-1].stop + 1 and \
+ region.start % 8 == 0 and region.size() > 8 and \
+ region.size() % 8 != 0:
+ shift = 8 - (region.size() % 8)
+ region.stop += shift
+ region.value = 0xff & ~IOPortRegionTree.parse_ioport_value(
+ 0, 8 - shift)
+ pm_regions.append(IOPortRegion(
+ region.start, region.stop, region.value, region.comments))
+ continue
+
+ # start address is not divided by 8 and size < 8
+ # [0x21 ... 0x26] --> [0x20 ... 0x27]
+ # then append this region
+ if region.start == pm_regions[-1].stop + 1 and \
+ region.start % 8 != 0 and region.size() < 8:
+ shift = (region.start % 8)
+ value = IOPortRegionTree.parse_ioport_value(0, shift)
+ region.start -= shift
+ if region.size() % 8 != 0:
+ region.stop += (region.stop % 8)
+ pm_regions[-1].stop -= shift
+ region.value = value
+ pm_regions.append(IOPortRegion(
+ region.start, region.stop, region.value, region.comments))
+ continue
+
+ # this region.stop < the last append region stop
+ # [0x20 ... 0x27][0x24 ... 0x27] --> [0x20 ... 0x27]
+ # then append this region
+ if region.stop <= pm_regions[-1].stop:
+ shift = pm_regions[-1].stop - region.stop + 1
+ region.value = region.value << shift
+ pm_regions[-1].value &= ~region.value
+ continue
+
+ # this region.start < the last append region stop
+ # but region stop > last region stop
+ # [0x20 ... 0x27][0x26 ... 0x3f] --> [0x20 ... 0x27][0x28 ... 0x3f]
+ # then append this region
+ if region.start < pm_regions[-1].stop and \
+ region.stop > pm_regions[-1].stop:
+ region.start = pm_regions[-1].stop + 1
+ continue
+
+ pm_regions.append(region)
+
+ # if the last address is not 0xffff
+ # padding the last address
+ if pm_regions[-1].stop != 0xffff:
+ start = pm_regions[-1].stop + 1
+ stop = 0xffff
+ value = IOPortRegionTree.parse_ioport_value(start, stop)
+ comments = []
+ pm_regions.append(IOPortRegion(start, stop, value, comments))
+
+ return pm_timer_base, pm_regions


class MMConfig:
@@ -1277,7 +1373,8 @@ regions.append(inmatereg)

cpucount = count_cpus()

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


f = open(options.file, 'w')
@@ -1295,7 +1392,9 @@ kwargs = {
'irqchips': ioapics,
'pm_timer_base': pm_timer_base,
'mmconfig': mmconfig,
- 'iommu_units': iommu_units
+ 'iommu_units': iommu_units,
+ 'pm_size': pm_size,
+ 'pm_regions': pm_regions
}

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

[Xuguo Wang]

Since the paramenters transfered to the root-cell-config.c-tmpl
are chaged, so we have to modify these paramenters, these para-
menters are used to generate the .pio_bitmap configuration.

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 5ee3767..4c40b5b 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_size}];
struct jailhouse_pci_device pci_devices[${len(pcidevices)}];
struct jailhouse_pci_capability pci_caps[${len(pcicaps)}];
} __attribute__((packed)) config = {
@@ -132,18 +132,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/7] = -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_regions:
+ % if k.comments != []:
+ [${ hex(k.start)}/8 ... ${hex(k.stop)}/8] = ${hex(k.value)}, /*${k.comments}*/
+ % else:
+ [${ hex(k.start)}/8 ... ${hex(k.stop)}/8] = ${hex(k.value)},
+ % endif
+ % endfor
},

.pci_devices = {
--
2.5.0

[Jan Kiszka]

Thanks for the update. I may not be able to look into this soon, though.

Jan

[charles king]

Hi, Jan. Take you time.

Thank you Jan.

best regards

from Xuguo Wang

[Jan Kiszka]

On 2016-04-02 04:57, Xuguo Wang wrote:

> Abstract the common property of the MemRegion and PortRegion
> to the super class IORegion, named IOMemRegion and IOPortRegion
> for parsing the /proc/ioports, the same action of the MemRegion-
> Tree.

The patch introduces a number of pep8 warnings - did you check this?

Let's rephrase a bit:

Each IOMemRegion holds one entry of /proc/iomem.
It extends IORegion by the typestr property.

> +class IOMemRegion(IORegion):
> + def __init__(self, start, stop, typestr, comments=None):
> + self.typestr = typestr
> + IORegion.__init__(self, start, stop, comments)
> +
> + def __str__(self):
> + return 'IOMemRegion: %08x-%08x : %s' % \
> (self.start, self.stop, self.typestr)
>
> def size(self):
> @@ -400,6 +413,18 @@ class MemRegion:
> return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE'
>
>
> +# IOPortRegion includes one of the /proc/ioports entry.
> +# IOPortRegion has his special priority value.

Analogously to above.

> +class IOPortRegion(IORegion):
> + def __init__(self, start, stop, value, comments=None):
> + self.value = value
> + IORegion.__init__(self, start, stop, comments)
> +
> + def size(self):
> + # plus 1 more for len since the self.start start from 0

I think the proper reason is the the range is from start to stop,
including. self.start may be > 0.

??

Most of this is identical to parse_iomem_file. I think you can
generalize this method and pass in the differences (input file and
target class) as parameters. parse_ioregion_line and parse_iomem_line
should become parse_line and just be differently implemented by both
classes.

> +
> + # recurse down the tree
> + @staticmethod
> + def parse_ioport_tree(tree):
> + regions = []
> +
> + for tree in tree.children:
> + r = tree.region
> + s = r.value
> + c = r.comments
> +
> + # set keyboard and PCI conf1 disable, others enable

Better say "intercept keyboard and PCI conf1".

> + if (c is not None and (c == 'keyboard' or c == 'PCI conf1')):
> + r.value = IOPortRegionTree.parse_ioport_value(
> + r.start, r.stop + 1)
> +
> + # if the tree continues recurse further down ...
> + if (len(tree.children) > 0):
> + regions.extend(IOPortRegionTree.parse_ioport_tree(tree))
> + continue
> +
> + # add all remaining leaves
> + regions.append(r)
> +
> + return regions
> +
> + @staticmethod
> + def parse_ioport_value(start, stop):
> + size = stop - start
> + value = -1

Unneeded initialization.

> + if size > 8:
> + value = -1
> + elif size == 8:
> + value = 0xff
> + elif size == 7:
> + value = 0x7f
> + elif size == 6:
> + value = 0x3f
> + elif size == 5:
> + value = 0x1f
> + elif size == 4:
> + value = 0x0f
> + elif size == 3:
> + value = 0x07
> + elif size == 2:
> + value = 0x03
> + else:
> + value = 0x01

This you can do in a single line, I'm sure ;).

Jan

[Jan Kiszka]

The subject should reflect better that this switches from a fixed
pio-bitmap template to one that is now generated per-target.

On 2016-04-02 13:57, Xuguo Wang wrote:
> Since the paramenters transfered to the root-cell-config.c-tmpl
> are chaged, so we have to modify these paramenters, these para-
> menters are used to generate the .pio_bitmap configuration.

Here are some typos in the description. I always have a spell-checker
enabled in my editors to reduce that risk.

Please preserve the original indentions and alignments, specifically two
tabs at the line start and " = " around the assignment. It should even
be possible to restore the alignment inside the [ ], Python is capable
enough.

Jan

[charles king]

Thank you Jan, very much.

For you are reading the patch carefully.

I will amend these, and I already use the pep8 check,

that is strange.

best regards

from Xuguo Wang