Hi,
On 6/21/19 12:06 AM, Andrej Utz wrote:
> This replaces the old static port list with actual port regions from
> '/proc/ioports'. The static regions from said list are kept and override
> the data in case of region overlap to retain compability.
> The generated port list is virtually identicall to the old one but eases
> manual configuration.
just found a bug in this series. This series creates regions such as:
<snip>
[ 0x80/8 ... 0x87/8] = -1, /* 0080-0087 : dma page reg */
[ 0x88/8 ... 0x8f/8] = -1, /* 0088-008f : dma page reg */
[ 0xa0/8 ... 0xa7/8] = -1, /* 00a0-00a1 : pic2 */
<snip>
Now we have a hole between [0x90/8 ... 0x1f/8]. A hole means that this
area will be initialised with zero -> access is permitted.
Root of this bug: In addition known port regions, we must also respect
unknown port regions and deny access.
@Jan: This brings me to an idea. The TODO says that whitelist-based MSR
bitmaps are a v1.0 target. I think the PIO bitmap would also benefit if
it would be whitelist based. Do you agree?
E.g.:
.pio_bitmap = {
[ 0x3f8/8 ... 0x3ff/8 ] = -1,
},
would denote that only access to 3f8-3ff is allowed. All other ports are
denied. Much easier to write and understand.
Ralf
>
> Signed-off-by: Andrej Utz <[email protected]>
> ---
> pyjailhouse/sysfs_parser.py | 150 ++++++++++++++++++++++++++++++++++
> tools/jailhouse-config-create | 26 ++----
> tools/root-cell-config.c.tmpl | 31 ++++---
> 3 files changed, 176 insertions(+), 31 deletions(-)
>
> diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py
> index d612c6d3..ce490236 100644
> --- a/pyjailhouse/sysfs_parser.py
> +++ b/pyjailhouse/sysfs_parser.py
> @@ -141,6 +141,52 @@ def parse_iomem(pcidevices):
>
> return ret, dmar_regions
>
> +def parse_ioports():
> + regions = IOMapTree.parse_ioports_tree(
> + IOMapTree.parse_iomap_file('/proc/ioports', PortRegion))
> +
> + tmp = [
> + # static regions
> + PortRegion(0x0, 0x3f, ''),
> + PortRegion(0x40, 0x43, 'PIT', allowed=True),
> + PortRegion(0x60, 0x61, 'NMI', allowed=True, comments=["HACK!"]), #
> NMI status/control
> + PortRegion(0x64, 0x64, 'NMI', allowed=True, comments=["HACK!"]), #
> ditto
> + PortRegion(0x70, 0x71, 'RTC', allowed=True),
> + PortRegion(0x3b0, 0x3df, 'VGA', allowed=True),
> + PortRegion(0xd00, 0xffff, 'PCI bus', allowed=True),
> + ]
> +
> + pm_timer_base = None
> + for r in regions:
> + if r.typestr == 'ACPI PM_TMR':
> + pm_timer_base = r.start
> +
> + tmp.append(r)
> +
> + tmp.sort(key=lambda r: r.start)
> + ret = [ tmp[0] ]
> +
> + # adjust overlapping regions
> + for r in tmp[1:]:
> + prev = ret[-1]
> +
> + # combine multiple regions under the same bit mask
> + if prev.aligned_stop() >= r.aligned_start():
> + if r.stop > prev.stop:
> + n = prev
> + while n.neighbor != None:
> + n = n.neighbor
> + n.neighbor = r
> + continue
> +
> + # forbid access to unrecognized regions
> + if prev.aligned_stop() - r.aligned_start() > 0:
> + ret.append(
> + PortRegion(prev.aligned_stop() + 1, r.aligned_start() - 1,
> ''))
> +
> + ret.append(r)
> +
> + return (ret, pm_timer_base)
>
> def parse_pcidevices():
> int_src_cnt = 0
> @@ -772,6 +818,85 @@ class MemRegion:
> return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE'
>
>
> +class PortRegion:
> + def __init__(self, start, stop, typestr, comments=None, allowed=False):
> + self.start = start
> + self.stop = stop
> + self.typestr = typestr or ''
> + self.comments = comments or []
> + self.allowed = allowed
> + self.neighbor = None
> +
> + def __str__(self):
> + # as in MemRegion this method is purely for C comment generation
> +
> + # remove consecutive duplicates
> + neighbor = self.neighbor
> + stop = self.stop
> + ns = ''
> + while neighbor:
> + if self.typestr != neighbor.typestr \
> + or self.comments != neighbor.comments:
> + ns += ', ' + str(neighbor)
> + break
> +
> + stop = neighbor.stop
> + neighbor = neighbor.neighbor
> +
> + s = ''
> + # pretty print single ports
> + if self.start == stop:
> + s += '%5s' % ''
> + else:
> + s += '%04x-' % self.start
> +
> + s += '%04x' % stop
> +
> +
> + if self.typestr:
> + s += ' : ' + self.typestr
> +
> + if self.comments:
> + s += ' (' + ', '.join(c for c in self.comments) + ')'
> +
> + s += ns
> + return s
> +
> + # used in root-cell-config.c.tmpl
> + def is_combined(self):
> + neighbor = self.neighbor
> + while neighbor:
> + if self.typestr != neighbor.typestr:
> + return True
> +
> + neighbor = neighbor.neighbor
> +
> + return False
> +
> + def size(self):
> + return self.stop - self.start
> +
> + def aligned_start(self):
> + return self.start - self.start % 8
> +
> + def aligned_stop(self):
> + return self.stop + (7 - self.stop % 8)
> +
> + def bits(self):
> + # in this method: 0 = disallowed,
> + # in config: 0 = allowed
> + if self.allowed:
> + bits = ((1 << (self.size() + 1)) - 1) << \
> + (self.start - self.aligned_start())
> + else:
> + bits = 0
> +
> + if self.neighbor:
> + bits |= ~self.neighbor.bits()
> +
> + return ~bits & 0xFF
> +
> +
> class IOAPIC:
> def __init__(self, id, address, gsi_base, iommu=0, bdf=0):
> self.id = id
> @@ -935,6 +1060,31 @@ class IOMapTree:
>
> return regions, dmar_regions
>
> + # recurse down the tree
> + @staticmethod
> + def parse_ioports_tree(tree):
> + regions = []
> +
> + for tree in tree.children:
> + r = tree.region
> + s = r.typestr
> +
> + if len(tree.children) > 0:
> + regions.extend(IOMapTree.parse_ioports_tree(tree))
> + continue
> +
> + # split in byte sized regions
> + while r.size() > 8:
> + # byte-align r.stop
> + sub = PortRegion(r.start, r.start + 7 - r.start % 8,
> r.typestr)
> + regions.append(sub)
> + r.start = sub.stop + 1
> +
> + # add all remaining leaves
> + regions.append(r)
> +
> + return regions
> +
>
> class IOMMUConfig:
> def __init__(self, props):
> diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create
> index 55601a6d..d154ec44 100755
> --- a/tools/jailhouse-config-create
> +++ b/tools/jailhouse-config-create
> @@ -162,18 +162,6 @@ def count_cpus():
> count += 1
> return count
>
> -
> -def parse_ioports():
> - pm_timer_base = None
> - f = sysfs_parser.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
> -
> -
> class MMConfig:
> def __init__(self, base, end_bus):
> self.base = base
> @@ -269,7 +257,7 @@ product = [input_readline('/sys/class/dmi/id/sys_vendor',
> inmatemem = kmg_multiply_str(options.mem_inmates)
> hvmem = [0, kmg_multiply_str(options.mem_hv)]
>
> -(regions, dmar_regions) = sysfs_parser.parse_iomem(pcidevices)
> +(mem_regions, dmar_regions) = sysfs_parser.parse_iomem(pcidevices)
> ourmem = parse_kernel_cmdline()
> total = hvmem[1] + inmatemem
>
> @@ -283,11 +271,11 @@ if vendor == 'GenuineIntel':
> dmar_regions)
> else:
> (iommu_units, extra_memregs) = sysfs_parser.parse_ivrs(pcidevices,
> ioapics)
> -regions += extra_memregs
> +mem_regions += extra_memregs
>
> # kernel does not have memmap region, pick one
> if ourmem is None:
> - ourmem = alloc_mem(regions, total)
> + ourmem = alloc_mem(mem_regions, total)
> elif (total > ourmem[1]):
> raise RuntimeError('Your memmap reservation is too small you need >="' +
> hex(total) + '". Hint: your kernel cmd line needs '
> @@ -298,20 +286,20 @@ hvmem[0] = ourmem[0]
> inmatereg = sysfs_parser.MemRegion(ourmem[0] + hvmem[1],
> ourmem[0] + hvmem[1] + inmatemem - 1,
> 'JAILHOUSE Inmate Memory')
> -regions.append(inmatereg)
> +mem_regions.append(inmatereg)
>
> cpucount = count_cpus()
>
> -pm_timer_base = parse_ioports()
> +(port_regions, pm_timer_base) = sysfs_parser.parse_ioports()
>
> debug_console = DebugConsole(options.console)
>
> -
> f = open(options.file, 'w')
> tmpl = Template(filename=os.path.join(options.template_dir,
> 'root-cell-config.c.tmpl'))
> kwargs = {
> - 'regions': regions,
> + 'mem_regions': mem_regions,
> + 'port_regions': port_regions,
> 'ourmem': ourmem,
> 'argstr': ' '.join(sys.argv),
> 'hvmem': hvmem,
> diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
> index b6ac8637..9f65a72a 100644
> --- a/tools/root-cell-config.c.tmpl
> +++ b/tools/root-cell-config.c.tmpl
> @@ -45,7 +45,7 @@
> struct {
> struct jailhouse_system header;
> __u64 cpus[${int((cpucount + 63) / 64)}];
> - struct jailhouse_memory mem_regions[${len(regions)}];
> + struct jailhouse_memory mem_regions[${len(mem_regions)}];
> struct jailhouse_irqchip irqchips[${len(irqchips)}];
> __u8 pio_bitmap[0x2000];
> struct jailhouse_pci_device pci_devices[${len(pcidevices)}];
> @@ -125,7 +125,7 @@ struct {
> },
>
> .mem_regions = {
> - % for r in regions:
> + % for r in mem_regions:
> /* ${str(r)} */
> % for c in r.comments:
> /* ${c} */
> @@ -153,18 +153,25 @@ 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 ... 0x3af/8] = -1,
> - [ 0x3b0/8 ... 0x3df/8] = 0x00, /* VGA */
> - [ 0x3e0/8 ... 0xcff/8] = -1,
> - [ 0xd00/8 ... 0xffff/8] = 0, /* HACK: PCI bus */
> + % for r in port_regions:
> + % if r.is_combined():
> +
> + % for c in str(r).split(', '):
> + /* ${c} */
> + % endfor
> + % endif
> + [${
> + '%6s' % hex(r.aligned_start())}/8 ... ${
> + '%6s' % hex(r.aligned_stop())}/8] = ${
> + '%4s' % ('0' if r.bits() == 0 else \
> + '-1' if r.bits() == 0xff else \
> + hex(r.bits()))}, ${
> + ('/* %s */' % str(r)) if not r.is_combined() else '\n'
> + }
> + % endfor
> },
>
> +
> .pci_devices = {
> % for d in pcidevices:
> /* ${str(d)} */
>
--
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].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/e369a4a6-57cf-3fae-a68c-3351394da184%40oth-regensburg.de.
For more options, visit https://groups.google.com/d/optout.
