Hey Andrej,

this feature was already proposed and discussed before, but never
resulted in patches getting merged.

https://groups.google.com/d/topic/jailhouse-dev/BSfMKio91BQ/discussion

I did not look into it yet. But you might want to reread that thread,
making sure your proposal covers what was discussed back than.

The main issue really is that a lot of device drivers do not register
themselfs as port-users, so we can not detect them.
But those exotic ports are probably blocked in the default config so
there is no new problem. A new problem would be if the generated configs
shrink the default set, revoking access that was granted before.

But i agree that it is a good idea to improve the generated config to
reach a working out-of-the-box state.

Henning

Am Fri, 21 Jun 2019 00:06:14 +0200
schrieb Andrej Utz <[email protected]>:

> 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.
> 
> 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/20190628102929.6e801007%40md1za8fc.ad001.siemens.net.
For more options, visit https://groups.google.com/d/optout.

Reply via email to