Re: [EXT] Jailhouse: unhandled APIC access when booting a Linux guest
Hi all, On 05.12.22 15:41, Ralf Ramsauer wrote: [Adding Andrej] Hi Karim, On 05/12/2022 13:30, Karim Manaouil wrote: Hi Ralf, I am trying to boot a Linux guest (based on configs/x86/linux-x86-demo.c). I tried to debug and solve this issue for a while but no success so far. The cell is created, and the guest starts booting, but then fails when initialising the APIC. A write to an APIC register is intercepted by Jailhouse and it decides that the guest is trying to set an invalid LVT delivery mode. I checked the x86 documentation, it seems that it should not be invalid, but I am not knowledgeable enough. Sound familiar… I remember that Andrej and I have hit this one on an AMD machine some years ago: https://groups.google.com/g/jailhouse-dev/c/1wRKIiGN0GA/m/_p_NSIBpDwAJ Andrej, do you know how we finally (quick?)-fixed that issue back then? Did we really have hardware misbehavior? It's been a while, but as far as I remember that was the case. Afaik it was a CPU bug. We had to apply the attached patch to the kernel to make it boot as guest. Thanks, Andrej Thanks, Ralf Here is, pasted below, the log output from Jailhouse and the booting guest kernel. PS: I am following the steps in Documentation/non-root-linux.txt and using the patched kernel in queues/jailhouse branch. Cheers Karim --- Jailhouse: Cell "linux-x86-demo" can be loaded Started cell "linux-x86-demo" CPU 2 received SIPI, vector 100 CPU 3 received SIPI, vector 100 FATAL: Setting invalid LVT delivery mode (reg 35, value 0700) FATAL: Unhandled APIC access, offset 848, is_write: 1 RIP: 0xaf84fb92 RSP: 0xb1003e80 FLAGS: 246 RAX: 0xaf84fb90 RBX: 0x0180 RCX: 0x RDX: 0x RSI: 0x0700 RDI: 0x0350 CS: 10 BASE: 0x AR-BYTES: 29b EFER.LMA 1 CR0: 0x80050033 CR3: 0x3ae0c000 CR4: 0x000406b0 EFER: 0x1d01 Parking CPU 2 (Cell: "linux-x86-demo") Cell "linux-x86-demo" can be loaded Closing cell "linux-x86-demo" Page pool usage after cell destruction: mem 2972/32211, remap 65703/131072 CPU 2 received SIPI, vector 96 CPU 3 received SIPI, vector 96 Linux demo guest (last few lines from kernel boot log on the serial console): init, 1176K bss, 73676K reserved, 0K cma-reserved) [ 2.960440] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1 [ 3.040332] Dynamic Preempt: voluntary [ 3.083151] rcu: Preemptible hierarchical RCU implementation. [ 3.151749] rcu: RCU event tracing is enabled. [ 3.205830] rcu: RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=2. [ 3.284872] Trampoline variant of Tasks RCU enabled. [ 3.345191] rcu: RCU calculated value of scheduler-enlistment delay is 100 jiffies. [ 3.436710] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=2 [ 3.518471] NR_IRQS: 4352, nr_irqs: 424, preallocated irqs: 0 [ 3.585483] rcu: srcu_init: Setting srcu_struct sizes based on contention. [ 3.667665] Console: colour dummy device 80x25 [ 3.720639] Enabling UART0 (port 0x3f8) [ 3.766423] printk: console [ttyS0] enabled [ 3.766423] printk: console [ttyS0] enabled [ 3.866333] printk: bootconsole [earlyser0] disabled [ 3.866333] printk: bootconsole [earlyser0] disabled [ 3.985019] APIC: Switch to symmetric I/O mode setup [ 4.046377] Switched APIC routing to physical flat. The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336. Is e buidheann carthannais a th’ ann an Oilthigh Dhùn Èideann, clàraichte an Alba, àireamh clàraidh SC005336. -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/b48f1252-a6b7-a183-7f37-080b95043ad6%40st.oth-regensburg.de. diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 0428ad289899..836436ce05ba 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1699,6 +1699,14 @@ static void setup_local_APIC(void) value |= SPURIOUS_APIC_VECTOR; apic_write(APIC_SPIV, value); + // HACK: some CPUs (e.g. the AMD Ryzen family) fail to reset LVT_LINT + // registers according to specification, so we help them do that + if (((value = apic_read(APIC_LVT0)) & APIC_LVT_MASKED) == 0) + apic_write(APIC_LVT0, value | APIC_LVT_MASKED); + + if (((value = apic_read(APIC_LVT1)) & APIC_LVT_MASKED) == 0) + apic_write(APIC_LVT1, value | APIC_LVT_MASKED); + perf_events_lapic_init(); /*
Re: [PATCH v3 0/7] Configuration parser streamlining
Hi Jan, On 27/08/2020 11:33, Jan Kiszka wrote: On 25.08.20 16:50, Andrej Utz wrote: Theses patches mostly improve non-functional aspects of the Jailhouse configuration parser. Logic for unpacking binary data is consolidated into CStruct class, which all parser classes are inherited from. To improve input flexibility, data slices are replaced with I/O streams (see Pythons 'io' package for how to use them). Finally, a CPU set check is added, which will list conflicting CPUs between cells and also usage of non-existing ones in the root-cell. Changes v2->v3: - fix config_parser usage in jailhouse-cell-linux - fix output style in "CPU checks" commit Changes v1->v2: - reference class variables via class object or class name - drop implicit PEP8 formatting - dropped unused patches Andrej Utz (7): pyjailhouse: config_parser: store binary format specification in struct.Struct pyjailhouse: config_parser: move parsing into class methods pyjailhouse: config_parser: consolidate binary parsing into CStruct class pyjailhouse: config_parser: use I/O stream instead slice of bytes pyjailhouse: config_parser: parse pin_bitman in Irqchip as list pyjailhouse: config_parser: consolidate header parsing tools: config-check: add CPU overlap and boundary check pyjailhouse/config_parser.py | 296 ++- tools/jailhouse-cell-linux | 2 +- tools/jailhouse-config-check | 33 +++- 3 files changed, 215 insertions(+), 116 deletions(-) -- 2.28.0 Still doesn't work - did you test jailhouse cell linux? I did and it works... with Python 3. Traceback (most recent call last): File "jailhouse/tools/jailhouse-cell-linux", line 723, in config = config_parser.parse(args.config, config_parser.CellConfig) File "jailhouse/tools/../pyjailhouse/config_parser.py", line 287, in parse return config_expect.parse(stream) File "jailhouse/tools/../pyjailhouse/config_parser.py", line 220, in parse cpu_set_num = int(self.cpu_set / cpu_fmt.size) TypeError: unsupported operand type(s) for /: 'set' and 'int' When using Python 2, the slots of the child classes are not updated with cell config data. I'll search for a workaround. By the way: how long will Jailhouse require Python 2 compatibility? Thanks, Andrej Utz Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/fe12ea0a-d279-1532-099b-e23f0ec822dc%40st.oth-regensburg.de.
[PATCH v3 5/7] pyjailhouse: config_parser: parse pin_bitman in Irqchip as list
Just like the array of 4 in the C struct. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 6ab769ff..a45aa7d7 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -155,20 +155,30 @@ class CacheRegion(CStruct): class Irqchip(CStruct): -__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +__slots__ = 'address', 'id', 'pin_base', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT = struct.Struct('QIIQQ') +_BIN_FMT = struct.Struct('QII') +_BIN_FMT_PIN_MAP = struct.Struct('4I') + +# constructed fields +__slots__ += 'pin_bitmap', def __init__(self): self.address = 0 self.id = 0 self.pin_base = 0 -self.pin_bitmap_lo = 0 -self.pin_bitmap_hi = 0 +self.pin_bitmap = [0,0,0,0] def is_standard(self): return self.address == 0xfec0 +@classmethod +def parse(cls, stream): +self = cls.parse_class(cls, stream) +pin_fmt = cls._BIN_FMT_PIN_MAP +self.pin_bitmap = pin_fmt.unpack_from(stream.read(pin_fmt.size)) +return self + class PIORegion(CStruct): __slots__ = 'base', 'length', -- 2.28.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200825145032.115837-6-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 4/7] pyjailhouse: config_parser: use I/O stream instead slice of bytes
This enables more flexibility in input types as long as they provide binary I/O capabilities. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 59 +++- tools/jailhouse-cell-linux | 2 +- tools/jailhouse-config-check | 4 +-- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 961d3062..6ab769ff 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -16,6 +16,7 @@ from __future__ import print_function import struct +import io from .extendedenum import ExtendedEnum @@ -39,29 +40,30 @@ class CStruct: return attrs @classmethod -def parse(cls, data): -obj, data = cls.parse_class(cls, data) -return obj, data +def parse(cls, stream): +obj = cls.parse_class(cls, stream) +return obj @staticmethod -def parse_class(cls, data): +def parse_class(cls, stream): +fmt = cls._BIN_FMT +data_tuple = fmt.unpack_from(stream.read(fmt.size)) obj = cls() slots = obj._slots() if len(slots) > 0: -data_tuple = cls._BIN_FMT.unpack_from(data) for assigment in zip(slots, data_tuple): setattr(obj, *assigment) -return obj, data[cls._BIN_FMT.size:] +return obj @staticmethod -def parse_array(cls, num, data): +def parse_array(cls, num, stream): array = [] for i in range(num): -obj, data = cls.parse(data) +obj = cls.parse(stream) array += [obj] -return array, data +return array def flag_str(enum_class, value, separator=' | '): @@ -197,28 +199,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, data, root_cell=False): +def parse(cls, stream, root_cell=False): try: if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from(data) +(signature, revision) = cls._BIN_FMT_HDR.unpack_from( +stream.read(cls._BIN_FMT_HDR.size)) if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -data = data[cls._BIN_FMT_HDR.size:] -self, data = cls.parse_class(cls, data) +self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') -data = data[self._cpu_sets:] # skip CPU set +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set -self.memory_regions, data = \ -cls.parse_array(MemRegion, self.memory_regions, data) -self.cache_regions, data = \ -cls.parse_array(CacheRegion, self.cache_regions, data) -self.irqchips, data = \ -cls.parse_array(Irqchip, self.irqchips, data) -self.pio_regions, data = \ -cls.parse_array(PIORegion, self.pio_regions, data) +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) return self except struct.error: @@ -239,21 +240,23 @@ class SystemConfig(CStruct): self.root_cell = CellConfig() @classmethod -def parse(cls, data): +def parse(cls, stream): try: hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = hdr_fmt.unpack_from(data) +(signature, revision) = \ +hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) if signature != b'JHSYST': raise RuntimeError('Not a root cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -self, data = cls.parse_class(cls, data[hdr_fmt.size:]) -self.hypervisor_memory, data = MemRegion.parse(data) +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell -self.root_cell = CellConfig.parse(data[offs:], root_cell=True) +offs += hdr_fmt.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream, True) return self except struct.error: ra
[PATCH v3 0/7] Configuration parser streamlining
Theses patches mostly improve non-functional aspects of the Jailhouse configuration parser. Logic for unpacking binary data is consolidated into CStruct class, which all parser classes are inherited from. To improve input flexibility, data slices are replaced with I/O streams (see Pythons 'io' package for how to use them). Finally, a CPU set check is added, which will list conflicting CPUs between cells and also usage of non-existing ones in the root-cell. Changes v2->v3: - fix config_parser usage in jailhouse-cell-linux - fix output style in "CPU checks" commit Changes v1->v2: - reference class variables via class object or class name - drop implicit PEP8 formatting - dropped unused patches Andrej Utz (7): pyjailhouse: config_parser: store binary format specification in struct.Struct pyjailhouse: config_parser: move parsing into class methods pyjailhouse: config_parser: consolidate binary parsing into CStruct class pyjailhouse: config_parser: use I/O stream instead slice of bytes pyjailhouse: config_parser: parse pin_bitman in Irqchip as list pyjailhouse: config_parser: consolidate header parsing tools: config-check: add CPU overlap and boundary check pyjailhouse/config_parser.py | 296 ++- tools/jailhouse-cell-linux | 2 +- tools/jailhouse-config-check | 33 +++- 3 files changed, 215 insertions(+), 116 deletions(-) -- 2.28.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200825145032.115837-1-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 1/7] pyjailhouse: config_parser: store binary format specification in struct.Struct
Improves its handling in the code and slightly increases the overall performance as well. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 50 +++- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 47350c6f..9b828f2a 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -52,15 +52,13 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: -_REGION_FORMAT = '' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('') def __init__(self, region_struct): (self.phys_start, self.virt_start, self.size, - self.flags) = \ -struct.unpack_from(MemRegion._REGION_FORMAT, region_struct) + self.flags) = MemRegion._BIN_FMT.unpack_from(region_struct) def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -106,37 +104,32 @@ class MemRegion: class CacheRegion: -_REGION_FORMAT = 'IIBxH' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('IIBxH') class Irqchip: -_IRQCHIP_FORMAT = 'QIIQQ' -SIZE = struct.calcsize(_IRQCHIP_FORMAT) +_BIN_FMT = struct.Struct('QIIQQ') def __init__(self, irqchip_struct): (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = \ -struct.unpack_from(self._IRQCHIP_FORMAT, irqchip_struct) + self.pin_bitmap_hi) = Irqchip._BIN_FMT.unpack_from(irqchip_struct) def is_standard(self): return self.address == 0xfec0 class PIORegion: -_REGION_FORMAT = 'HH' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('HH') def __init__(self, pio_struct): -(self.base, self.length) = struct.unpack_from(self._REGION_FORMAT, - pio_struct) +(self.base, self.length) = PIORegion._BIN_FMT.unpack_from(pio_struct) class CellConfig: -_HEADER_FORMAT = '=6sH32s4xIIQ8x32x' +_BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') def __init__(self, data, root_cell=False): self.data = data @@ -156,7 +149,7 @@ class CellConfig: self.num_stream_ids, self.vpci_irq_base, self.cpu_reset_address) = \ -struct.unpack_from(CellConfig._HEADER_FORMAT, self.data) +CellConfig._BIN_FMT.unpack_from(self.data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -164,55 +157,52 @@ class CellConfig: raise RuntimeError('Configuration file revision mismatch') self.name = str(name.decode().strip('\0')) -mem_region_offs = struct.calcsize(CellConfig._HEADER_FORMAT) + \ -self.cpu_set_size +mem_region_offs = CellConfig._BIN_FMT.size + self.cpu_set_size self.memory_regions = [] for n in range(self.num_memory_regions): self.memory_regions.append( MemRegion(self.data[mem_region_offs:])) -mem_region_offs += MemRegion.SIZE +mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ -self.num_cache_regions * CacheRegion.SIZE +self.num_cache_regions * CacheRegion._BIN_FMT.size self.irqchips = [] for n in range(self.num_irqchips): self.irqchips.append( Irqchip(self.data[irqchip_offs:])) -irqchip_offs += Irqchip.SIZE +irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs self.pio_regions = [] for n in range(self.num_pio_regions): self.pio_regions.append(PIORegion(self.data[pioregion_offs:])) -pioregion_offs += PIORegion.SIZE +pioregion_offs += PIORegion._BIN_FMT.size except struct.error: raise RuntimeError('Not a %scell configuration' % ('root ' if root_cell else '')) class SystemConfig: -_HEADER_FORMAT = '=6sH4x' +_BIN_FMT = struct.Struct('=6sH4x') # ...followed by MemRegion as hypervisor memory -_CONSOLE_AND_PLATFORM_FORMAT = '32x12x224x44x' +_BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') def __init__(self, data): self.data = data try: -(signature, - revision) = \ -struct.unpack_from(SystemConfig._HEADER_FORMAT, self.data) +(signature, revision) = SystemConfig._BIN_FMT.unpack_from(self.data) if signature != b'JHSYST': raise RuntimeError('Not a root cell con
[PATCH v3 3/7] pyjailhouse: config_parser: consolidate binary parsing into CStruct class
The class slots define component fields in a more grounded way. This greatly simplifies definition of parseable compoments. The first `__slots__` tuple in each class defines a constant list of fields and also the corresponding binary ones in the C struct. `_BIN_FIELD_NUM` ensures that subsequent slot additions are ignored by CStruct as they must be constructed by the owning class itself. For complex parsing the class method `parse` needs to be overridden, see `CellConfig`. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 169 +++ 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index a6780d61..961d3062 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -23,6 +23,47 @@ from .extendedenum import ExtendedEnum _CONFIG_REVISION = 13 +class CStruct: +def _slots(self): +attrs = [] +all_attr = [getattr(cls, '__slots__', ()) +for cls in type(self).__mro__] +for cls in all_attr: +num = getattr(self, '_BIN_FIELD_NUM', 0) +if len(cls) < num: +continue + +for i in range(num): +attrs += [cls[i]] + +return attrs + +@classmethod +def parse(cls, data): +obj, data = cls.parse_class(cls, data) +return obj, data + +@staticmethod +def parse_class(cls, data): +obj = cls() +slots = obj._slots() +if len(slots) > 0: +data_tuple = cls._BIN_FMT.unpack_from(data) +for assigment in zip(slots, data_tuple): +setattr(obj, *assigment) + +return obj, data[cls._BIN_FMT.size:] + +@staticmethod +def parse_array(cls, num, data): +array = [] +for i in range(num): +obj, data = cls.parse(data) +array += [obj] + +return array, data + + def flag_str(enum_class, value, separator=' | '): flags = [] while value: @@ -51,7 +92,9 @@ class JAILHOUSE_MEM(ExtendedEnum, int): } -class MemRegion: +class MemRegion(CStruct): +__slots__ = 'phys_start', 'virt_start', 'size', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('') def __init__(self): @@ -66,15 +109,6 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) -@classmethod -def parse(cls, data): -self = cls() -(self.phys_start, - self.virt_start, - self.size, - self.flags) = cls._BIN_FMT.unpack_from(data) -return self - def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -112,11 +146,15 @@ class MemRegion: self.virt_address_in_region(region.virt_start) -class CacheRegion: +class CacheRegion(CStruct): +__slots__ = 'start', 'size', 'type', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('IIBxH') -class Irqchip: +class Irqchip(CStruct): +__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('QIIQQ') def __init__(self): @@ -126,36 +164,29 @@ class Irqchip: self.pin_bitmap_lo = 0 self.pin_bitmap_hi = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.address, - self.id, - self.pin_base, - self.pin_bitmap_lo, - self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) -return self - def is_standard(self): return self.address == 0xfec0 -class PIORegion: +class PIORegion(CStruct): +__slots__ = 'base', 'length', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('HH') def __init__(self): self.base = 0 self.length = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.base, self.length) = cls._BIN_FMT.unpack_from(data) -return self - -class CellConfig: -_BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') +class CellConfig(CStruct): +# slots with a '_' prefix in name are private +__slots__ = 'name', '_flags', '_cpu_sets', \ +'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ +'_pci_devices', '_pci_caps', '_stream_ids', \ +'vpci_irq_base', 'cpu_reset_address', +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT = struct.Struct('=32s4xIIQ8x32x') def __init__(self): self.name = "" @@ -167,78 +198,62 @@ class CellConfig: @classmethod def parse(cls, data, root_cell=False): -self = cls() try: -(signature, - revision, -
[PATCH v3 6/7] pyjailhouse: config_parser: consolidate header parsing
This also enables probing for a configuration type. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 99 +++- tools/jailhouse-cell-linux | 2 +- tools/jailhouse-config-check | 5 +- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index a45aa7d7..2b47d9b6 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -197,8 +197,9 @@ class CellConfig(CStruct): '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT_HDR = struct.Struct('=6sH') _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" @@ -209,38 +210,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, stream, root_cell=False): -try: -if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from( -stream.read(cls._BIN_FMT_HDR.size)) -if signature != b'JHCELL': -raise RuntimeError('Not a cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set - -self.memory_regions = \ -cls.parse_array(MemRegion, self.memory_regions, stream) -self.cache_regions = \ -cls.parse_array(CacheRegion, self.cache_regions, stream) -self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) -self.pio_regions = \ -cls.parse_array(PIORegion, self.pio_regions, stream) - -return self -except struct.error: -raise RuntimeError('Not a %scell configuration' % - ('root ' if root_cell else '')) +def parse(cls, stream): +self = cls.parse_class(cls, stream) +self.name = self.name.decode().strip('\0') +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) + +return self class SystemConfig(CStruct): _BIN_FMT = struct.Struct('=4x') # ...followed by MemRegion as hypervisor memory _BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') +_BIN_SIGNATURE = b'JHSYST' # constructed fields __slots__ = 'hypervisor_memory', 'root_cell', @@ -251,22 +241,39 @@ class SystemConfig(CStruct): @classmethod def parse(cls, stream): -try: -hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = \ -hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) -if signature != b'JHSYST': -raise RuntimeError('Not a root cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.hypervisor_memory = MemRegion.parse(stream) - -offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += hdr_fmt.size # skip header inside rootcell -stream.seek(offs, io.SEEK_CUR) -self.root_cell = CellConfig.parse(stream, True) -return self -except struct.error: -raise RuntimeError('Not a root cell configuration') +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) + +offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size +offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream) +return self + + +def parse(stream, config_expect=None): +fmt = CellConfig._BIN_FMT_HDR + +try: +(signature, revision) = fmt.unpack_from(stream.read(fmt.size)) +except struct.error: +raise RuntimeError('Not a Jailhouse configuration') + +if config_expect == None: +# Try probing +if signature == CellConfig._BIN_SIGNATURE: +config_expect = CellConfig +elif signature == SystemConfig._BIN_SIGNATURE: +config_expect = SystemConfig +else: +raise RuntimeError('Not a Jailhouse con
[PATCH v3 2/7] pyjailhouse: config_parser: move parsing into class methods
... and use constructor for initialization only. This separation provides clarity on how to instantiate config components. This commit also serves as preparation for following one. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 93 +--- tools/jailhouse-cell-linux | 2 +- tools/jailhouse-config-check | 4 +- 3 files changed, 67 insertions(+), 32 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 9b828f2a..a6780d61 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -54,11 +54,11 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: _BIN_FMT = struct.Struct('') -def __init__(self, region_struct): -(self.phys_start, - self.virt_start, - self.size, - self.flags) = MemRegion._BIN_FMT.unpack_from(region_struct) +def __init__(self): +self.phys_start = 0 +self.virt_start = 0 +self.size = 0 +self.flags = 0 def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -66,6 +66,15 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) +@classmethod +def parse(cls, data): +self = cls() +(self.phys_start, + self.virt_start, + self.size, + self.flags) = cls._BIN_FMT.unpack_from(data) +return self + def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -110,12 +119,22 @@ class CacheRegion: class Irqchip: _BIN_FMT = struct.Struct('QIIQQ') -def __init__(self, irqchip_struct): +def __init__(self): +self.address = 0 +self.id = 0 +self.pin_base = 0 +self.pin_bitmap_lo = 0 +self.pin_bitmap_hi = 0 + +@classmethod +def parse(cls, data): +self = cls() (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = Irqchip._BIN_FMT.unpack_from(irqchip_struct) + self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) +return self def is_standard(self): return self.address == 0xfec0 @@ -124,16 +143,31 @@ class Irqchip: class PIORegion: _BIN_FMT = struct.Struct('HH') -def __init__(self, pio_struct): -(self.base, self.length) = PIORegion._BIN_FMT.unpack_from(pio_struct) +def __init__(self): +self.base = 0 +self.length = 0 + +@classmethod +def parse(cls, data): +self = cls() +(self.base, self.length) = cls._BIN_FMT.unpack_from(data) +return self class CellConfig: _BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') -def __init__(self, data, root_cell=False): -self.data = data - +def __init__(self): +self.name = "" +self.memory_regions = [] +self.irqchips = [] +self.pio_regions = [] +self.vpci_irq_base = 0 +self.cpu_reset_address = 0 + +@classmethod +def parse(cls, data, root_cell=False): +self = cls() try: (signature, revision, @@ -148,8 +182,7 @@ class CellConfig: self.num_pci_caps, self.num_stream_ids, self.vpci_irq_base, - self.cpu_reset_address) = \ -CellConfig._BIN_FMT.unpack_from(self.data) + self.cpu_reset_address) = cls._BIN_FMT.unpack_from(data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -157,53 +190,55 @@ class CellConfig: raise RuntimeError('Configuration file revision mismatch') self.name = str(name.decode().strip('\0')) -mem_region_offs = CellConfig._BIN_FMT.size + self.cpu_set_size -self.memory_regions = [] +mem_region_offs = cls._BIN_FMT.size + self.cpu_set_size for n in range(self.num_memory_regions): -self.memory_regions.append( -MemRegion(self.data[mem_region_offs:])) + self.memory_regions.append(MemRegion.parse(data[mem_region_offs:])) mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ self.num_cache_regions * CacheRegion._BIN_FMT.size -self.irqchips = [] for n in range(self.num_irqchips): -self.irqchips.append( -Irqchip(self.data[irqchip_offs:])) +self.irqchips.append(Irqchip.parse(data[irqchip_offs:])) irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs -self.pio_regions = []
[PATCH v3 7/7] tools: config-check: add CPU overlap and boundary check
Adds two checks for CPU specification: - overlap check detects two inmates using the same CPU(s) - boundary check detects CPU usage outside of what system config provides Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 14 -- tools/jailhouse-config-check | 28 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 2b47d9b6..2a0e6ec9 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -192,17 +192,19 @@ class PIORegion(CStruct): class CellConfig(CStruct): # slots with a '_' prefix in name are private -__slots__ = 'name', '_flags', '_cpu_sets', \ +__slots__ = 'name', '_flags', 'cpu_set', \ 'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') _BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT_CPU = struct.Struct('=Q') _BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" +self.cpu_set = set() self.memory_regions = [] self.irqchips = [] self.pio_regions = [] @@ -213,7 +215,15 @@ class CellConfig(CStruct): def parse(cls, stream): self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +cpu_fmt = cls._BIN_FMT_CPU +cpu_set_num = int(self.cpu_set / cpu_fmt.size) +self.cpu_set = set() +for set_idx in range(cpu_set_num): +cpu_bits = cpu_fmt.unpack_from(stream.read(cpu_fmt.size)) +for bit in range(cpu_fmt.size * 8): +if cpu_bits[0] & (1 << bit) > 0: +self.cpu_set.add(bit) self.memory_regions = \ cls.parse_array(MemRegion, self.memory_regions, stream) diff --git a/tools/jailhouse-config-check b/tools/jailhouse-config-check index 380f4a77..33d9110f 100755 --- a/tools/jailhouse-config-check +++ b/tools/jailhouse-config-check @@ -66,6 +66,7 @@ for cfg in args.cellcfgs: ret=0 +# Memory checks print("Overlapping memory regions inside cell:", end='') found=False for cell in cells: @@ -100,4 +101,31 @@ for cell in cells: ret=1 print("\n" if found else " None") +# CPU checks +print("Overlapping CPUs between cells:", end='') +found = False +for cell in non_root_cells: +cell_idx = cells.index(cell) +for cell2 in cells[cell_idx + 1:]: +overlap = cell.cpu_set & cell2.cpu_set +if overlap: +print("\n\nIn cell '%s' and '%s' following CPUs overlap: %s" % + (cell.name, cell2.name, str(overlap)), end='') +found = True +ret = 1 +print("\n" if found else " None") + +print("CPUs not in root cell CPU set:", end='') +found = False +for cell in non_root_cells: +diff = cell.cpu_set - root_cell.cpu_set +if diff: +print("\n\nIn cell '%s': %s" % (cell.name, str(diff)), end='') +found = True +ret = 1 +if found: +print("\nNote: root cell CPU set: %s\n" % str(root_cell.cpu_set)) +else: +print(" None") + exit(ret) -- 2.28.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200825145032.115837-8-andrej.utz%40st.oth-regensburg.de.
Re: [PATCH-set] pyjailhouse: config_parser - jailhouse cell linux throws an error
Hi all, On 25/08/2020 16:17, Jan Kiszka wrote: Hi Thorsten, please maintain CC lists on reply (AKA reply-to-all). On 25.08.20 14:48, contact@gmail.com wrote: nope, changed to: Traceback (most recent call last): File "tools/jailhouse-cell-linux", line 723, in config = config_parser.CellConfig.parse(args.config.read()) File "tools/../pyjailhouse/config_parser.py", line 214, in parse self = cls.parse_class(cls, stream) File "tools/../pyjailhouse/config_parser.py", line 50, in parse_class data_tuple = fmt.unpack_from(stream.read(fmt.size)) AttributeError: 'bytes' object has no attribute 'read' OK, I was only fixing one patch, but the series goes on, and at least two others ignored jailhouse-cell-linux. Somehow I missed to refactor this tool. Sorry about that. Andrej, I'm dropping your series from next again. Please fix and resend. Ack. Will send a v3 soon'ish. Thanks, Andrej Utz Thanks, Jan j.kiszka...@gmail.com schrieb am Dienstag, 25. August 2020 um 12:20:10 UTC+2: On 25.08.20 10:37, contact@gmail.com wrote: > At the current head of the next branch, jailhouse cell linux throws an > error: > > Traceback (most recent call last): > File "tools/jailhouse-cell-linux", line 723, in > config = config_parser.CellConfig(args.config.read()) > TypeError: __init__() takes 1 positional argument but 2 were given > > Thorsten > > PS sorry for probably replying to the wrong patch-thread > No problem - thanks for reporting! This comes from "pyjailhouse: config_parser: move parsing into class methods". Does this help? diff --git a/tools/jailhouse-cell-linux b/tools/jailhouse-cell-linux index 4178d4e0..aab82a5e 100755 --- a/tools/jailhouse-cell-linux +++ b/tools/jailhouse-cell-linux @@ -720,7 +720,7 @@ except IOError as e: arch = resolve_arch(args.arch) try: - config = config_parser.CellConfig(args.config.read()) + config = config_parser.CellConfig.parse(args.config.read()) except RuntimeError as e: print(str(e) + ": " + args.config.name <http://args.config.name>, file=sys.stderr) exit(1) Then I will fold it into Andrej's commit. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux -- 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 jailhouse-dev+unsubscr...@googlegroups.com <mailto:jailhouse-dev+unsubscr...@googlegroups.com>. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/bddbb89a-01c3-4d34-9821-405ec8dcffe5n%40googlegroups.com <https://groups.google.com/d/msgid/jailhouse-dev/bddbb89a-01c3-4d34-9821-405ec8dcffe5n%40googlegroups.com?utm_medium=email_source=footer>. -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/e3d8f4e7-54aa-0c4f-0f71-ec688b4269e7%40st.oth-regensburg.de.
[PATCH v2 4/7] pyjailhouse: config_parser: use I/O stream instead slice of bytes
This enables more flexibility in input types as long as they provide binary I/O capabilities. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 59 +++- tools/jailhouse-config-check | 4 +-- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 961d3062..6ab769ff 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -16,6 +16,7 @@ from __future__ import print_function import struct +import io from .extendedenum import ExtendedEnum @@ -39,29 +40,30 @@ class CStruct: return attrs @classmethod -def parse(cls, data): -obj, data = cls.parse_class(cls, data) -return obj, data +def parse(cls, stream): +obj = cls.parse_class(cls, stream) +return obj @staticmethod -def parse_class(cls, data): +def parse_class(cls, stream): +fmt = cls._BIN_FMT +data_tuple = fmt.unpack_from(stream.read(fmt.size)) obj = cls() slots = obj._slots() if len(slots) > 0: -data_tuple = cls._BIN_FMT.unpack_from(data) for assigment in zip(slots, data_tuple): setattr(obj, *assigment) -return obj, data[cls._BIN_FMT.size:] +return obj @staticmethod -def parse_array(cls, num, data): +def parse_array(cls, num, stream): array = [] for i in range(num): -obj, data = cls.parse(data) +obj = cls.parse(stream) array += [obj] -return array, data +return array def flag_str(enum_class, value, separator=' | '): @@ -197,28 +199,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, data, root_cell=False): +def parse(cls, stream, root_cell=False): try: if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from(data) +(signature, revision) = cls._BIN_FMT_HDR.unpack_from( +stream.read(cls._BIN_FMT_HDR.size)) if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -data = data[cls._BIN_FMT_HDR.size:] -self, data = cls.parse_class(cls, data) +self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') -data = data[self._cpu_sets:] # skip CPU set +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set -self.memory_regions, data = \ -cls.parse_array(MemRegion, self.memory_regions, data) -self.cache_regions, data = \ -cls.parse_array(CacheRegion, self.cache_regions, data) -self.irqchips, data = \ -cls.parse_array(Irqchip, self.irqchips, data) -self.pio_regions, data = \ -cls.parse_array(PIORegion, self.pio_regions, data) +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) return self except struct.error: @@ -239,21 +240,23 @@ class SystemConfig(CStruct): self.root_cell = CellConfig() @classmethod -def parse(cls, data): +def parse(cls, stream): try: hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = hdr_fmt.unpack_from(data) +(signature, revision) = \ +hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) if signature != b'JHSYST': raise RuntimeError('Not a root cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -self, data = cls.parse_class(cls, data[hdr_fmt.size:]) -self.hypervisor_memory, data = MemRegion.parse(data) +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell -self.root_cell = CellConfig.parse(data[offs:], root_cell=True) +offs += hdr_fmt.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream, True) return self except struct.error: raise RuntimeError('Not a root c
[PATCH v2 2/7] pyjailhouse: config_parser: move parsing into class methods
... and use constructor for initialization only. This separation provides clarity on how to instantiate config components. This commit also serves as preparation for following one. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 93 +--- tools/jailhouse-config-check | 4 +- 2 files changed, 66 insertions(+), 31 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 9b828f2a..a6780d61 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -54,11 +54,11 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: _BIN_FMT = struct.Struct('') -def __init__(self, region_struct): -(self.phys_start, - self.virt_start, - self.size, - self.flags) = MemRegion._BIN_FMT.unpack_from(region_struct) +def __init__(self): +self.phys_start = 0 +self.virt_start = 0 +self.size = 0 +self.flags = 0 def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -66,6 +66,15 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) +@classmethod +def parse(cls, data): +self = cls() +(self.phys_start, + self.virt_start, + self.size, + self.flags) = cls._BIN_FMT.unpack_from(data) +return self + def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -110,12 +119,22 @@ class CacheRegion: class Irqchip: _BIN_FMT = struct.Struct('QIIQQ') -def __init__(self, irqchip_struct): +def __init__(self): +self.address = 0 +self.id = 0 +self.pin_base = 0 +self.pin_bitmap_lo = 0 +self.pin_bitmap_hi = 0 + +@classmethod +def parse(cls, data): +self = cls() (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = Irqchip._BIN_FMT.unpack_from(irqchip_struct) + self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) +return self def is_standard(self): return self.address == 0xfec0 @@ -124,16 +143,31 @@ class Irqchip: class PIORegion: _BIN_FMT = struct.Struct('HH') -def __init__(self, pio_struct): -(self.base, self.length) = PIORegion._BIN_FMT.unpack_from(pio_struct) +def __init__(self): +self.base = 0 +self.length = 0 + +@classmethod +def parse(cls, data): +self = cls() +(self.base, self.length) = cls._BIN_FMT.unpack_from(data) +return self class CellConfig: _BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') -def __init__(self, data, root_cell=False): -self.data = data - +def __init__(self): +self.name = "" +self.memory_regions = [] +self.irqchips = [] +self.pio_regions = [] +self.vpci_irq_base = 0 +self.cpu_reset_address = 0 + +@classmethod +def parse(cls, data, root_cell=False): +self = cls() try: (signature, revision, @@ -148,8 +182,7 @@ class CellConfig: self.num_pci_caps, self.num_stream_ids, self.vpci_irq_base, - self.cpu_reset_address) = \ -CellConfig._BIN_FMT.unpack_from(self.data) + self.cpu_reset_address) = cls._BIN_FMT.unpack_from(data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -157,53 +190,55 @@ class CellConfig: raise RuntimeError('Configuration file revision mismatch') self.name = str(name.decode().strip('\0')) -mem_region_offs = CellConfig._BIN_FMT.size + self.cpu_set_size -self.memory_regions = [] +mem_region_offs = cls._BIN_FMT.size + self.cpu_set_size for n in range(self.num_memory_regions): -self.memory_regions.append( -MemRegion(self.data[mem_region_offs:])) + self.memory_regions.append(MemRegion.parse(data[mem_region_offs:])) mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ self.num_cache_regions * CacheRegion._BIN_FMT.size -self.irqchips = [] for n in range(self.num_irqchips): -self.irqchips.append( -Irqchip(self.data[irqchip_offs:])) +self.irqchips.append(Irqchip.parse(data[irqchip_offs:])) irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs -self.pio_regions = [] for n in range(self.num_pio_regions): -
[PATCH v2 5/7] pyjailhouse: config_parser: parse pin_bitman in Irqchip as list
Just like the array of 4 in the C struct. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 6ab769ff..a45aa7d7 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -155,20 +155,30 @@ class CacheRegion(CStruct): class Irqchip(CStruct): -__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +__slots__ = 'address', 'id', 'pin_base', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT = struct.Struct('QIIQQ') +_BIN_FMT = struct.Struct('QII') +_BIN_FMT_PIN_MAP = struct.Struct('4I') + +# constructed fields +__slots__ += 'pin_bitmap', def __init__(self): self.address = 0 self.id = 0 self.pin_base = 0 -self.pin_bitmap_lo = 0 -self.pin_bitmap_hi = 0 +self.pin_bitmap = [0,0,0,0] def is_standard(self): return self.address == 0xfec0 +@classmethod +def parse(cls, stream): +self = cls.parse_class(cls, stream) +pin_fmt = cls._BIN_FMT_PIN_MAP +self.pin_bitmap = pin_fmt.unpack_from(stream.read(pin_fmt.size)) +return self + class PIORegion(CStruct): __slots__ = 'base', 'length', -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200715212119.48052-6-andrej.utz%40st.oth-regensburg.de.
[PATCH v2 7/7] tools: config-check: add CPU overlap and boundary check
Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 14 -- tools/jailhouse-config-check | 34 +++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 2b47d9b6..2a0e6ec9 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -192,17 +192,19 @@ class PIORegion(CStruct): class CellConfig(CStruct): # slots with a '_' prefix in name are private -__slots__ = 'name', '_flags', '_cpu_sets', \ +__slots__ = 'name', '_flags', 'cpu_set', \ 'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') _BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT_CPU = struct.Struct('=Q') _BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" +self.cpu_set = set() self.memory_regions = [] self.irqchips = [] self.pio_regions = [] @@ -213,7 +215,15 @@ class CellConfig(CStruct): def parse(cls, stream): self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +cpu_fmt = cls._BIN_FMT_CPU +cpu_set_num = int(self.cpu_set / cpu_fmt.size) +self.cpu_set = set() +for set_idx in range(cpu_set_num): +cpu_bits = cpu_fmt.unpack_from(stream.read(cpu_fmt.size)) +for bit in range(cpu_fmt.size * 8): +if cpu_bits[0] & (1 << bit) > 0: +self.cpu_set.add(bit) self.memory_regions = \ cls.parse_array(MemRegion, self.memory_regions, stream) diff --git a/tools/jailhouse-config-check b/tools/jailhouse-config-check index 380f4a77..d7f405fd 100755 --- a/tools/jailhouse-config-check +++ b/tools/jailhouse-config-check @@ -66,6 +66,7 @@ for cfg in args.cellcfgs: ret=0 +# Memory checks print("Overlapping memory regions inside cell:", end='') found=False for cell in cells: @@ -79,10 +80,10 @@ for cell in cells: if (mem.virt_overlaps(mem2)): overlaps.append("virtually") if overlaps: -print("\n\nIn cell '%s', region %d" % (cell.name, idx)) +print("\nIn cell '%s', region %d" % (cell.name, idx)) print(str(mem)) -print(" and ".join(overlaps) + \ -" overlaps with region %d\n" % idx2 + str(mem2), end='') +print(" and ".join(overlaps) + + " overlaps with region %d\n" % idx2 + str(mem2), end='') found=True ret=1 print("\n" if found else " None") @@ -100,4 +101,31 @@ for cell in cells: ret=1 print("\n" if found else " None") +# CPU checks +print("Overlapping CPUs between cells:", end='') +found = False +for cell in non_root_cells: +cell_idx = cells.index(cell) +for cell2 in cells[cell_idx + 1:]: +overlap = cell.cpu_set & cell2.cpu_set +if overlap: +print("\nIn cell '%s' and '%s' following CPUs overlap: %s" % + (cell.name, cell2.name, str(overlap)), end='') +found = True +ret = 1 +print("\n" if found else " None") + +print("CPUs not in root cell CPU set:", end='') +found = False +for cell in non_root_cells: +diff = cell.cpu_set - root_cell.cpu_set +if diff: +print("\nIn cell '%s': %s" % (cell.name, str(diff)), end='') +found = True +ret = 1 +if found: +print("\nNote: root cell CPU set: %s\n" % str(root_cell.cpu_set)) +else: +print(" None") + exit(ret) -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200715212119.48052-8-andrej.utz%40st.oth-regensburg.de.
[PATCH v2 0/7] Configuration parser streamlining
Theses patches mostly improve non-functional aspects of the Jailhouse configuration parser. Logic for unpacking binary data is consolidated into CStruct class, which all parser classes are inherited from. To improve input flexibility, data slices are replaced with I/O streams (see Pythons 'io' package for how to use them). Finally, a CPU set check is added, which will list conflicting CPUs between cells and also usage of non-existing ones in the root-cell. Changes v1->v2: - reference class variables via class object or class name - drop implicit PEP8 formatting - dropped unused patches Andrej Utz (7): pyjailhouse: config_parser: store binary format specification in struct.Struct pyjailhouse: config_parser: move parsing into class methods pyjailhouse: config_parser: consolidate binary parsing into CStruct class pyjailhouse: config_parser: use I/O stream instead slice of bytes pyjailhouse: config_parser: parse pin_bitman in Irqchip as list pyjailhouse: config_parser: consolidate header parsing tools: config-check: add CPU overlap and boundary check pyjailhouse/config_parser.py | 296 ++- tools/jailhouse-config-check | 39 - 2 files changed, 217 insertions(+), 118 deletions(-) -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200715212119.48052-1-andrej.utz%40st.oth-regensburg.de.
[PATCH v2 6/7] pyjailhouse: config_parser: consolidate header parsing
This also enables probing for a configuration type. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 99 +++- tools/jailhouse-config-check | 5 +- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index a45aa7d7..2b47d9b6 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -197,8 +197,9 @@ class CellConfig(CStruct): '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT_HDR = struct.Struct('=6sH') _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" @@ -209,38 +210,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, stream, root_cell=False): -try: -if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from( -stream.read(cls._BIN_FMT_HDR.size)) -if signature != b'JHCELL': -raise RuntimeError('Not a cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set - -self.memory_regions = \ -cls.parse_array(MemRegion, self.memory_regions, stream) -self.cache_regions = \ -cls.parse_array(CacheRegion, self.cache_regions, stream) -self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) -self.pio_regions = \ -cls.parse_array(PIORegion, self.pio_regions, stream) - -return self -except struct.error: -raise RuntimeError('Not a %scell configuration' % - ('root ' if root_cell else '')) +def parse(cls, stream): +self = cls.parse_class(cls, stream) +self.name = self.name.decode().strip('\0') +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) + +return self class SystemConfig(CStruct): _BIN_FMT = struct.Struct('=4x') # ...followed by MemRegion as hypervisor memory _BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') +_BIN_SIGNATURE = b'JHSYST' # constructed fields __slots__ = 'hypervisor_memory', 'root_cell', @@ -251,22 +241,39 @@ class SystemConfig(CStruct): @classmethod def parse(cls, stream): -try: -hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = \ -hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) -if signature != b'JHSYST': -raise RuntimeError('Not a root cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.hypervisor_memory = MemRegion.parse(stream) - -offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += hdr_fmt.size # skip header inside rootcell -stream.seek(offs, io.SEEK_CUR) -self.root_cell = CellConfig.parse(stream, True) -return self -except struct.error: -raise RuntimeError('Not a root cell configuration') +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) + +offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size +offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream) +return self + + +def parse(stream, config_expect=None): +fmt = CellConfig._BIN_FMT_HDR + +try: +(signature, revision) = fmt.unpack_from(stream.read(fmt.size)) +except struct.error: +raise RuntimeError('Not a Jailhouse configuration') + +if config_expect == None: +# Try probing +if signature == CellConfig._BIN_SIGNATURE: +config_expect = CellConfig +elif signature == SystemConfig._BIN_SIGNATURE: +config_expect = SystemConfig +else: +raise RuntimeError('Not a Jailhouse configuration') +elif config_expect._BIN
[PATCH v2 3/7] pyjailhouse: config_parser: consolidate binary parsing into CStruct class
The class slots define component fields in a more grounded way. This greatly simplifies definition of parseable compoments. The first `__slots__` tuple in each class defines a constant list of fields and also the corresponding binary ones in the C struct. `_BIN_FIELD_NUM` ensures that subsequent slot additions are ignored by CStruct as they must be constructed by the owning class itself. For complex parsing the class method `parse` needs to be overridden, see `CellConfig`. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 169 +++ 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index a6780d61..961d3062 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -23,6 +23,47 @@ from .extendedenum import ExtendedEnum _CONFIG_REVISION = 13 +class CStruct: +def _slots(self): +attrs = [] +all_attr = [getattr(cls, '__slots__', ()) +for cls in type(self).__mro__] +for cls in all_attr: +num = getattr(self, '_BIN_FIELD_NUM', 0) +if len(cls) < num: +continue + +for i in range(num): +attrs += [cls[i]] + +return attrs + +@classmethod +def parse(cls, data): +obj, data = cls.parse_class(cls, data) +return obj, data + +@staticmethod +def parse_class(cls, data): +obj = cls() +slots = obj._slots() +if len(slots) > 0: +data_tuple = cls._BIN_FMT.unpack_from(data) +for assigment in zip(slots, data_tuple): +setattr(obj, *assigment) + +return obj, data[cls._BIN_FMT.size:] + +@staticmethod +def parse_array(cls, num, data): +array = [] +for i in range(num): +obj, data = cls.parse(data) +array += [obj] + +return array, data + + def flag_str(enum_class, value, separator=' | '): flags = [] while value: @@ -51,7 +92,9 @@ class JAILHOUSE_MEM(ExtendedEnum, int): } -class MemRegion: +class MemRegion(CStruct): +__slots__ = 'phys_start', 'virt_start', 'size', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('') def __init__(self): @@ -66,15 +109,6 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) -@classmethod -def parse(cls, data): -self = cls() -(self.phys_start, - self.virt_start, - self.size, - self.flags) = cls._BIN_FMT.unpack_from(data) -return self - def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -112,11 +146,15 @@ class MemRegion: self.virt_address_in_region(region.virt_start) -class CacheRegion: +class CacheRegion(CStruct): +__slots__ = 'start', 'size', 'type', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('IIBxH') -class Irqchip: +class Irqchip(CStruct): +__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('QIIQQ') def __init__(self): @@ -126,36 +164,29 @@ class Irqchip: self.pin_bitmap_lo = 0 self.pin_bitmap_hi = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.address, - self.id, - self.pin_base, - self.pin_bitmap_lo, - self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) -return self - def is_standard(self): return self.address == 0xfec0 -class PIORegion: +class PIORegion(CStruct): +__slots__ = 'base', 'length', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('HH') def __init__(self): self.base = 0 self.length = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.base, self.length) = cls._BIN_FMT.unpack_from(data) -return self - -class CellConfig: -_BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') +class CellConfig(CStruct): +# slots with a '_' prefix in name are private +__slots__ = 'name', '_flags', '_cpu_sets', \ +'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ +'_pci_devices', '_pci_caps', '_stream_ids', \ +'vpci_irq_base', 'cpu_reset_address', +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT = struct.Struct('=32s4xIIQ8x32x') def __init__(self): self.name = "" @@ -167,78 +198,62 @@ class CellConfig: @classmethod def parse(cls, data, root_cell=False): -self = cls() try: -(signature, - revision, -
Re: Need help to run Linux in non-root cell
Hi all, On 02/07/2020 18:26, Ralf Ramsauer wrote: On 02/07/2020 18:17, Jan Kiszka wrote: On 02.07.20 18:07, Moritz Walker wrote: Smells like a regression in that branch for non-root Linux. Is that 562b04e51bb5e2f04bf175383080333237067c63? Can you share you kernel config? Yes, its 562b04e51bb5e2f04bf175383080333237067c63. I attached the kernel config. I also tried the kernel from jailhouse-images (Linux version 5.4.17) which leads to the same error on my AMD-machine: Ah, AMD! Please see Yeah, AMD. It isn't always only Intel. :-) https://groups.google.com/forum/#!msg/jailhouse-dev/1wRKIiGN0GA/_p_NSIBpDwAJ - in fact a known issue (hardware misbehavior) that we didn't workaround yet. Yes, just wanted to mention! Looks familiar. Ralf, Andrej, any news here. Andrej? It's already been a while. Didn't we already have some preliminary patches for Linux? As a hacky workaround, you can try: diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c index d36c2033..5160d37d 100644 --- a/hypervisor/arch/x86/apic.c +++ b/hypervisor/arch/x86/apic.c @@ -350,7 +350,7 @@ void apic_clear(void) /* Finally, reset the TPR again and disable the APIC */ apic_ops.write(APIC_REG_TPR, 0); - apic_ops.write(APIC_REG_SVR, 0xff); + //apic_ops.write(APIC_REG_SVR, 0xff); } static void apic_send_ipi(unsigned int target_cpu_id, u32 orig_icr_hi, Or as an alternative apply the following patch to the kernel: https://groups.google.com/d/msg/jailhouse-dev/1wRKIiGN0GA/P5YeS3oqAQAJ @Jan: Any ideas how to bring the mentioned patch (or something equivalent) upstream? I know its a hack and will add proper CPU family checks, but the linux-x86 folks still may not even touch it. Meanwhile AMD gains market share and as such more people will try to run non-root Linux with their CPUs, run into this bug and request support here or decide to use other hypervisors. Andrej > Ralf Jan [ 5.879554] Switched APIC routing to physical flat. FATAL: Setting invalid LVT delivery mode (reg 35, value 0700) FATAL: Unhandled APIC access, offset 848, is_write: 1 The jailhouse-images kernel (5.4.17) works fine on a different machine (Intel). Might this problem be realted to my first machine beeing an AMD-x86 one? Moritz *Von:* Jan Kiszka *Gesendet:* Donnerstag, 2. Juli 2020 12:56 *An:* Moritz Walker ; jailhouse-dev@googlegroups.com *Betreff:* Re: Need help to run Linux in non-root cell On 02.07.20 14:04, Moritz Walker wrote: Hello, I need help to run Linux (queues/jailhouse branch) in a non-root cell on AMD. Root-cell and apic-demo both work fine. To run the root-cell i run the /jailhouse enable configs/x86/rootcell.cell/ comman, which produces the following output: Initializing Jailhouse hypervisor v0.12 (59-g4ce7658d-dirty) on CPU 1 Code location: 0xf050 Using xAPIC Page pool usage after early setup: mem 75/979, remap 1/131072 Initializing processors: CPU 1... (APIC ID 1) OK [...] CPU 7... (APIC ID 5) OK Initializing unit: AMD IOMMU AMD IOMMU @0xfeb8/0x8 Initializing unit: IOAPIC Initializing unit: PCI [...] Adding PCI device 31:00.0 to cell "RootCell" Page pool usage after late setup: mem 286/979, remap 16520/131072 Activating hypervisor After that i use the command /jailhouse cell linux configs/x86/linux-x86-cell.cell ../linux/arch/x86/boot/bzImage -c "console=ttyS0,9600"/ to start the non-root linux cell. This writes the following output via UART: Adding virtual PCI device 00:0c.0 to cell "linux-x86-demo" Adding virtual PCI device 00:0d.0 to cell "linux-x86-demo" Adding virtual PCI device 00:0e.0 to cell "linux-x86-demo" Adding virtual PCI device 00:0f.0 to cell "linux-x86-demo" Created cell "linux-x86-demo" Page pool usage after cell creation: mem 343/979, remap 16520/131072 IOMMU 0: Event Log overflow occurred, some events were lost! Cell "linux-x86-demo" can be loaded Started cell "linux-x86-demo" CPU 6 received SIPI, vector 100 CPU 7 received SIPI, vector 100 [ 0.00] Linux version 5.7.0-rc7+ (walker@wubuntu) (gcc version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2), GNU ld (GNU Binutils for Ubuntu) 2.33) #2 SMP Wed Jul 1 12:28:55 CEST 2020 [ 0.00] Command line: console=ttyS0,9600 [ 0.00] KERNEL supported cpus: [ 0.00] Intel GenuineIntel [ 0.00] AMD AuthenticAMD [ 0.00] Hygon HygonGenuine [ 0.00] Centaur CentaurHauls [ 0.00] zhaoxin Shanghai [ 0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers' [ 0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' [ 0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' [ 0.00] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256 [ 0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, using 'compacted' format. [ 0.00] BIOS-provided physical RAM map:
Re: [PATCH 09/11] pyjailhouse: config_parser: add missing components
On 05/07/2020 23:07, Jan Kiszka wrote: On 30.06.20 08:47, Andrej Utz wrote: They may not be used right now but will certainly ease the usage of the parser API in the future. Besides I already had written them long ago, so it would be a pity to withehld them. "withheld" Ack. While I understand and appreciate this completion, I'm concern it will simply bitrot if we do not start to use or at least test it soon. Any plans in this direction? Yes, the grand scheme is change all Python tools, which work with Jailhouse configs, to use config.py. This would, for example, allow config checks during the generation and prevent debugging-by-crashing from the start. But I agree with the sentiment about bitrot and will not include these last three patches in v2 but post them later when they are ready to be used. :) Andrej Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/4ed47956-d2c9-9f35-4b09-633a2c07b0d7%40st.oth-regensburg.de.
Re: [PATCH 03/11] pyjailhouse: config_parser: consolidate binary parsing into CStruct class
On 05/07/2020 22:57, Jan Kiszka wrote: On 30.06.20 08:42, Andrej Utz wrote: The class slots define component fields in a more grounded way. This greatly simplifies definition of parseable compoments. The first `__slots__` tuple in each class defines a constant list of fields and also the corresponding binary ones in the C struct. `_BIN_FIELD_NUM` ensures that subsequent slot additions are ignored by CStruct as they must be constructed by the owning class itself. _BIN_FIELD_NUM is always len(__slots__) - then why do we need that extra var at all? Because in come cases additional __slots__ fields are added (e.g. SystemConfig), which do not exist in C structs. To prevent the struct.Struct class from complaining about mismatching field count between slots and format, I've added a numerical boundary `_BIN_FIELD_NUM`. It contains the number of actual fields in the C structs, which CStruct class uses to restrict parsing. For complex parsing the class method `parse` needs to be overriden, see `CellConfig`. "overridden" Ack. Andrej Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/6284b928-851d-36bc-0b82-c7700a932c76%40st.oth-regensburg.de.
Re: [PATCH 01/11] pyjailhouse: config_parser: store binary format specification in struct.Struct
On 05/07/2020 22:55, Jan Kiszka wrote: On 30.06.20 08:42, Andrej Utz wrote: Improves its handling in the code and slightly increases the overall performance as well. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 51 ++-- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 6b9d9066..b75b9312 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -52,15 +52,13 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: - _REGION_FORMAT = '' - SIZE = struct.calcsize(_REGION_FORMAT) + _BIN_FMT = struct.Struct('') def __init__(self, region_struct): (self.phys_start, self.virt_start, self.size, - self.flags) = \ - struct.unpack_from(MemRegion._REGION_FORMAT, region_struct) + self.flags) = self._BIN_FMT.unpack_from(region_struct) Let's clarify that _BIN_FMT is a class var by accessing it via the class name (MemRegion._BIN_FMT). Same for the rest. While I'm not a fan of redundant naming for references, I can how would improve code clarity. Will change. Andrej Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/85d2aa12-22c1-5445-1d26-e55fe3c9dc4e%40st.oth-regensburg.de.
Re: [PATCH 01/11] pyjailhouse: config_parser: store binary format specification in struct.Struct
Hello Jan, thanks for reviewing. I will address the replies shortly. On 30/06/2020 21:12, Jan Kiszka wrote: On 30.06.20 08:42, Andrej Utz wrote: Improves its handling in the code and slightly increases the overall performance as well. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 51 ++-- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 6b9d9066..b75b9312 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -52,15 +52,13 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: - _REGION_FORMAT = '' - SIZE = struct.calcsize(_REGION_FORMAT) + _BIN_FMT = struct.Struct('') def __init__(self, region_struct): (self.phys_start, self.virt_start, self.size, - self.flags) = \ - struct.unpack_from(MemRegion._REGION_FORMAT, region_struct) + self.flags) = self._BIN_FMT.unpack_from(region_struct) def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -106,37 +104,32 @@ class MemRegion: class CacheRegion: - _REGION_FORMAT = 'IIBxH' - SIZE = struct.calcsize(_REGION_FORMAT) + _BIN_FMT = struct.Struct('IIBxH') class Irqchip: - _IRQCHIP_FORMAT = 'QIIQQ' - SIZE = struct.calcsize(_IRQCHIP_FORMAT) + _BIN_FMT = struct.Struct('QIIQQ') def __init__(self, irqchip_struct): (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = \ - struct.unpack_from(self._IRQCHIP_FORMAT, irqchip_struct) + self.pin_bitmap_hi) = self._BIN_FMT.unpack_from(irqchip_struct) def is_standard(self): return self.address == 0xfec0 class PIORegion: - _REGION_FORMAT = 'HH' - SIZE = struct.calcsize(_REGION_FORMAT) + _BIN_FMT = struct.Struct('HH') def __init__(self, pio_struct): - (self.base, self.length) = struct.unpack_from(self._REGION_FORMAT, - pio_struct) + (self.base, self.length) = self._BIN_FMT.unpack_from(pio_struct) class CellConfig: - _HEADER_FORMAT = '=6sH32s4xIIQ8x32x' + _BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') def __init__(self, data, root_cell=False): self.data = data @@ -155,8 +148,7 @@ class CellConfig: self.num_pci_caps, self.num_stream_ids, self.vpci_irq_base, - self.cpu_reset_address) = \ - struct.unpack_from(CellConfig._HEADER_FORMAT, self.data) + self.cpu_reset_address) = self._BIN_FMT.unpack_from(self.data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -164,55 +156,52 @@ class CellConfig: raise RuntimeError('Configuration file revision mismatch') self.name = str(name.decode()) - mem_region_offs = struct.calcsize(CellConfig._HEADER_FORMAT) + \ - self.cpu_set_size + mem_region_offs = self._BIN_FMT.size + self.cpu_set_size self.memory_regions = [] for n in range(self.num_memory_regions): self.memory_regions.append( MemRegion(self.data[mem_region_offs:])) - mem_region_offs += MemRegion.SIZE + mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ - self.num_cache_regions * CacheRegion.SIZE + self.num_cache_regions * CacheRegion._BIN_FMT.size self.irqchips = [] for n in range(self.num_irqchips): self.irqchips.append( Irqchip(self.data[irqchip_offs:])) - irqchip_offs += Irqchip.SIZE + irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs self.pio_regions = [] for n in range(self.num_pio_regions): self.pio_regions.append(PIORegion(self.data[pioregion_offs:])) - pioregion_offs += PIORegion.SIZE + pioregion_offs += PIORegion._BIN_FMT.size except struct.error: raise RuntimeError('Not a %scell configuration' % ('root ' if root_cell else '')) class SystemConfig: - _HEADER_FORMAT = '=6sH4x' + _BIN_FMT = struct.Struct('=6sH4x') # ...followed by MemRegion as hypervisor memory - _CONSOLE_AND_PLATFORM_FORMAT = '32x12x224x44x' + _BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') def __init__(self, data): self.data = data try: - (signature, - revision) = \ - struct.unpack_from(SystemConfig._HEADER_FORMAT,
[PATCH 09/11] pyjailhouse: config_parser: add missing components
They may not be used right now but will certainly ease the usage of the parser API in the future. Besides I already had written them long ago, so it would be a pity to withehld them. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 178 --- 1 file changed, 166 insertions(+), 12 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 2a0e6ec9..75e22fad 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -190,31 +190,77 @@ class PIORegion(CStruct): self.length = 0 +class PciDevice(CStruct): +__slots__ = 'type', 'iommu', 'domain', 'bdf', 'bar_mask', '_caps_start', \ +'_caps_num', 'num_msi_vectors', 'msi_bits', \ +'num_msix_vectors', 'msix_region_size', 'msix_address', \ +'shmem_regions_start', 'shmem_dev_id', 'shmem_peers', \ +'shmem_protocol' +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT_BAR_MASK = struct.Struct('6I') +_BIN_FMT = struct.Struct('BBHH%usHHBBHHQIBBH' % _BIN_FMT_BAR_MASK.size) + +# constructed fields +__slots__ += 'caps', + +TYPE_DEVICE = 1 +TYPE_BRIDGE = 2 +TYPE_IVSHMEM = 3 + +@classmethod +def parse(cls, stream): +self = cls.parse_class(cls, stream) +self.bar_mask = cls._BIN_FMT_BAR_MASK.unpack_from(self.bar_mask) +return self + + +class PciCapability(CStruct): +__slots__ = 'id', 'start', 'len', 'flags' +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT = struct.Struct('=') + + +class Console(CStruct): +__slots__ = 'address', 'size', 'type', 'flags', 'divider', 'gate_nr', \ +'clock_reg' +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT = struct.Struct('=QIHHIIQ') + + class CellConfig(CStruct): # slots with a '_' prefix in name are private -__slots__ = 'name', '_flags', 'cpu_set', \ +__slots__ = 'name', 'flags', 'cpu_set', \ 'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ -'_pci_devices', '_pci_caps', '_stream_ids', \ +'pci_devices', '_pci_caps', 'stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT = struct.Struct('=32s4xIIQ8x32x') +_BIN_FMT = struct.Struct('=32s4xIIQ8x') _BIN_FMT_HDR = struct.Struct('=6sH') -_BIN_FMT_CPU = struct.Struct('=Q') +_BIN_FMT_CPU = struct.Struct('Q') +_BIN_FMT_STREAM_ID = struct.Struct('I') _BIN_SIGNATURE = b'JHCELL' +# constructed fields +__slots__ += 'console', + def __init__(self): self.name = "" +self.flags = 0 self.cpu_set = set() self.memory_regions = [] self.irqchips = [] self.pio_regions = [] +self.pci_devices = [] +self.stream_ids = [] self.vpci_irq_base = 0 self.cpu_reset_address = 0 +self.console = Console() @classmethod def parse(cls, stream): self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') +self.console = Console.parse(stream) cpu_fmt = cls._BIN_FMT_CPU cpu_set_num = int(self.cpu_set / cpu_fmt.size) @@ -233,30 +279,138 @@ class CellConfig(CStruct): self.pio_regions = \ cls.parse_array(PIORegion, self.pio_regions, stream) +self.pci_devices = cls.parse_array(PciDevice, self.pci_devices, stream) +pci_caps = cls.parse_array(PciCapability, self._pci_caps, stream) +for pci_dev in self.pci_devices: +start = pci_dev._caps_start +end = start + pci_dev._caps_num +pci_dev.caps = pci_caps[start:end] + +sid_fmt = cls._BIN_FMT_STREAM_ID +sid_num = self.stream_ids +self.stream_ids = [] +for i in range(sid_num): +self.stream_ids += [sid_fmt.unpack_from(stream.read(sid_fmt.size))] + +return self + + +class IommuAmd(CStruct): +__slots__ = 'bdf', 'base_cap', 'msi_cap', 'features' +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT = struct.Struct('=HBBI') + + +class IommuPvu(CStruct): +__slots__ = 'tlb_base', 'tlb_size' +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT = struct.Struct('=QI') + + +class Iommu(CStruct): +__slots__ = 'type', 'base', 'size', +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT = struct.Struct('=IQI') + +# constructed fields +__slots__ += 'subtype', + +_MAX_UNITS = 8 + +TYPE_AMD = 1 +TYPE_INTEL = 2 +TYPE_SMMUV3 = 3 +TYPE_PVU = 4 + +@classmethod +def parse(cls, stream): +self = cls.parse_class(cls, stream) +sub_cls = None +if self.type == cls.TYPE_AMD: +sub_cls = IommuAmd +elif self.type == cls.TYPE_PVU: +sub_cls = IommuPvu + +if sub_cls: +self.subtype = sub_cls.parse(stream) + +# skip rest of the C union,
[PATCH 10/11] pyjailhouse: config_parser: add ability to serialize C structs
This enables to create Jailhouse configurations without relying on a C compiler. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 124 +-- 1 file changed, 118 insertions(+), 6 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 75e22fad..eaec4fa2 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -15,8 +15,9 @@ # to change the generated C-code. from __future__ import print_function -import struct +import copy import io +import struct from .extendedenum import ExtendedEnum @@ -39,6 +40,19 @@ class CStruct: return attrs +def save(self, stream): +values = tuple() +for field in self._slots(): +value = getattr(self, field, 0) +values = values + (value,) + +stream.write(self._BIN_FMT.pack(*values)) + +@staticmethod +def save_array(itr, stream): +for obj in itr: +obj.save(stream) + @classmethod def parse(cls, stream): obj = cls.parse_class(cls, stream) @@ -172,6 +186,10 @@ class Irqchip(CStruct): def is_standard(self): return self.address == 0xfec0 +def save(self, stream): +super(self.__class__, self).save(stream) +stream.write(self._BIN_FMT_PIN_MAP.pack(*self.pin_bitmap)) + @classmethod def parse(cls, stream): self = cls.parse_class(cls, stream) @@ -207,6 +225,12 @@ class PciDevice(CStruct): TYPE_BRIDGE = 2 TYPE_IVSHMEM = 3 +def save(self, stream): +temp = copy.copy(self) +temp.bar_mask = self._BIN_FMT_BAR_MASK.pack(*self.bar_mask) +temp._caps_num = len(self.caps) +return super(self.__class__, temp).save(stream) + @classmethod def parse(cls, stream): self = cls.parse_class(cls, stream) @@ -256,6 +280,62 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 self.console = Console() +def save(self, stream, is_rootcell=False): +# Flatten and deduplicate PCI capabilities +pci_caps = [] +for idx,dev in enumerate(self.pci_devices): +if not dev.caps: +continue + +duplicate = False +# look for duplicate capability patterns +for dev_prev in self.pci_devices[:idx]: +if dev_prev.caps == dev.caps: +dev._caps_start = dev_prev._caps_start +duplicate = True +break + +if not duplicate: +dev._caps_start = len(pci_caps) +pci_caps += dev.caps + +# Convert CPU set to bit array +cpu_bits_cap = self._BIN_FMT_CPU.size * 8 +cpu_sets = [0] +for cpu in self.cpu_set: +if cpu >= cpu_bits_cap: +cpu_sets += [0] +cpu_sets[-1] |= 1 << (cpu % cpu_bits_cap) +cpu_sets.reverse() # + +temp = copy.copy(self) +temp.name = self.name.encode() +temp.cpu_set = int(len(cpu_sets) * self._BIN_FMT_CPU.size) +temp.memory_regions = len(self.memory_regions) +temp.cache_regions = len(self.cache_regions) +temp.irqchips = len(self.irqchips) +temp.pio_regions = len(self.pio_regions) +temp.pci_devices = len(self.pci_devices) +temp.pci_caps = len(pci_caps) +temp.stream_ids = len(self.stream_ids) + +if not is_rootcell: +stream.write(self._BIN_FMT_HDR.pack(self._BIN_SIGNATURE, \ +_CONFIG_REVISION)) + +super(self.__class__, temp).save(stream) +self.console.save(stream) +for cpu_set in cpu_sets: +stream.write(self._BIN_FMT_CPU.pack(cpu_set)) +self.save_array(self.memory_regions, stream) +self.save_array(self.cache_regions, stream) +self.save_array(self.irqchips, stream) +self.save_array(self.pio_regions, stream) +self.save_array(self.pci_devices, stream) +self.save_array(pci_caps, stream) +for sid in self.stream_ids: +stream.write(self._BIN_FMT_STREAM_ID.pack(sid)) + @classmethod def parse(cls, stream): self = cls.parse_class(cls, stream) @@ -311,6 +391,7 @@ class Iommu(CStruct): __slots__ = 'type', 'base', 'size', _BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('=IQI') +_BIN_PAD_SIZE = max(IommuAmd._BIN_FMT.size, IommuPvu._BIN_FMT.size) # constructed fields __slots__ += 'subtype', @@ -322,6 +403,21 @@ class Iommu(CStruct): TYPE_SMMUV3 = 3 TYPE_PVU = 4 +def __init__(self, subtype = None): +self.type = 0 +self.base = 0 +self.size = 0 +self.subtype = subtype + +def save(self, stream): +super(self.__class__, self).save(stream) +padding = self._BIN_PAD_SIZE +if self.subtype: +self.subtype.sa
[PATCH 06/11] pyjailhouse: config_parser: strip tailing null-terminator from cell name
Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 7b4872e0..a45aa7d7 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -220,7 +220,7 @@ class CellConfig(CStruct): raise RuntimeError('Configuration file revision mismatch') self = cls.parse_class(cls, stream) -self.name = self.name.decode() +self.name = self.name.decode().strip('\0') stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set self.memory_regions = \ -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200630064228.4742-6-andrej.utz%40st.oth-regensburg.de.
[PATCH 03/11] pyjailhouse: config_parser: consolidate binary parsing into CStruct class
The class slots define component fields in a more grounded way. This greatly simplifies definition of parseable compoments. The first `__slots__` tuple in each class defines a constant list of fields and also the corresponding binary ones in the C struct. `_BIN_FIELD_NUM` ensures that subsequent slot additions are ignored by CStruct as they must be constructed by the owning class itself. For complex parsing the class method `parse` needs to be overriden, see `CellConfig`. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 169 +++ 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index e4ad2077..3bb2686a 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -23,6 +23,47 @@ from .extendedenum import ExtendedEnum _CONFIG_REVISION = 13 +class CStruct: +def _slots(self): +attrs = [] +all_attr = [getattr(cls, '__slots__', ()) +for cls in type(self).__mro__] +for cls in all_attr: +num = getattr(self, '_BIN_FIELD_NUM', 0) +if len(cls) < num: +continue + +for i in range(num): +attrs += [cls[i]] + +return attrs + +@classmethod +def parse(cls, data): +obj, data = cls.parse_class(cls, data) +return obj, data + +@staticmethod +def parse_class(cls, data): +obj = cls() +slots = obj._slots() +if len(slots) > 0: +data_tuple = cls._BIN_FMT.unpack_from(data) +for assigment in zip(slots, data_tuple): +setattr(obj, *assigment) + +return obj, data[cls._BIN_FMT.size:] + +@staticmethod +def parse_array(cls, num, data): +array = [] +for i in range(num): +obj, data = cls.parse(data) +array += [obj] + +return array, data + + def flag_str(enum_class, value, separator=' | '): flags = [] while value: @@ -51,7 +92,9 @@ class JAILHOUSE_MEM(ExtendedEnum, int): } -class MemRegion: +class MemRegion(CStruct): +__slots__ = 'phys_start', 'virt_start', 'size', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('') def __init__(self): @@ -66,15 +109,6 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) -@classmethod -def parse(cls, data): -self = cls() -(self.phys_start, - self.virt_start, - self.size, - self.flags) = cls._BIN_FMT.unpack_from(data) -return self - def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -112,11 +146,15 @@ class MemRegion: self.virt_address_in_region(region.virt_start) -class CacheRegion: +class CacheRegion(CStruct): +__slots__ = 'start', 'size', 'type', 'flags', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('IIBxH') -class Irqchip: +class Irqchip(CStruct): +__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('QIIQQ') def __init__(self): @@ -126,36 +164,29 @@ class Irqchip: self.pin_bitmap_lo = 0 self.pin_bitmap_hi = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.address, - self.id, - self.pin_base, - self.pin_bitmap_lo, - self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) -return self - def is_standard(self): return self.address == 0xfec0 -class PIORegion: +class PIORegion(CStruct): +__slots__ = 'base', 'length', +_BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('HH') def __init__(self): self.base = 0 self.length = 0 -@classmethod -def parse(cls, data): -self = cls() -(self.base, self.length) = self._BIN_FMT.unpack_from(data) -return self - -class CellConfig: -_BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') +class CellConfig(CStruct): +# slots with a '_' prefix in name are private +__slots__ = 'name', '_flags', '_cpu_sets', \ +'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ +'_pci_devices', '_pci_caps', '_stream_ids', \ +'vpci_irq_base', 'cpu_reset_address', +_BIN_FIELD_NUM = len(__slots__) +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT = struct.Struct('=32s4xIIQ8x32x') def __init__(self): self.name = "" @@ -167,78 +198,62 @@ class CellConfig: @classmethod def parse(cls, data, root_cell=False): -self = cls() try: -(signature, - revision, -
[PATCH 01/11] pyjailhouse: config_parser: store binary format specification in struct.Struct
Improves its handling in the code and slightly increases the overall performance as well. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 51 ++-- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 6b9d9066..b75b9312 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -52,15 +52,13 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: -_REGION_FORMAT = '' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('') def __init__(self, region_struct): (self.phys_start, self.virt_start, self.size, - self.flags) = \ -struct.unpack_from(MemRegion._REGION_FORMAT, region_struct) + self.flags) = self._BIN_FMT.unpack_from(region_struct) def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -106,37 +104,32 @@ class MemRegion: class CacheRegion: -_REGION_FORMAT = 'IIBxH' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('IIBxH') class Irqchip: -_IRQCHIP_FORMAT = 'QIIQQ' -SIZE = struct.calcsize(_IRQCHIP_FORMAT) +_BIN_FMT = struct.Struct('QIIQQ') def __init__(self, irqchip_struct): (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = \ -struct.unpack_from(self._IRQCHIP_FORMAT, irqchip_struct) + self.pin_bitmap_hi) = self._BIN_FMT.unpack_from(irqchip_struct) def is_standard(self): return self.address == 0xfec0 class PIORegion: -_REGION_FORMAT = 'HH' -SIZE = struct.calcsize(_REGION_FORMAT) +_BIN_FMT = struct.Struct('HH') def __init__(self, pio_struct): -(self.base, self.length) = struct.unpack_from(self._REGION_FORMAT, - pio_struct) +(self.base, self.length) = self._BIN_FMT.unpack_from(pio_struct) class CellConfig: -_HEADER_FORMAT = '=6sH32s4xIIQ8x32x' +_BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') def __init__(self, data, root_cell=False): self.data = data @@ -155,8 +148,7 @@ class CellConfig: self.num_pci_caps, self.num_stream_ids, self.vpci_irq_base, - self.cpu_reset_address) = \ -struct.unpack_from(CellConfig._HEADER_FORMAT, self.data) + self.cpu_reset_address) = self._BIN_FMT.unpack_from(self.data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -164,55 +156,52 @@ class CellConfig: raise RuntimeError('Configuration file revision mismatch') self.name = str(name.decode()) -mem_region_offs = struct.calcsize(CellConfig._HEADER_FORMAT) + \ -self.cpu_set_size +mem_region_offs = self._BIN_FMT.size + self.cpu_set_size self.memory_regions = [] for n in range(self.num_memory_regions): self.memory_regions.append( MemRegion(self.data[mem_region_offs:])) -mem_region_offs += MemRegion.SIZE +mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ -self.num_cache_regions * CacheRegion.SIZE +self.num_cache_regions * CacheRegion._BIN_FMT.size self.irqchips = [] for n in range(self.num_irqchips): self.irqchips.append( Irqchip(self.data[irqchip_offs:])) -irqchip_offs += Irqchip.SIZE +irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs self.pio_regions = [] for n in range(self.num_pio_regions): self.pio_regions.append(PIORegion(self.data[pioregion_offs:])) -pioregion_offs += PIORegion.SIZE +pioregion_offs += PIORegion._BIN_FMT.size except struct.error: raise RuntimeError('Not a %scell configuration' % ('root ' if root_cell else '')) class SystemConfig: -_HEADER_FORMAT = '=6sH4x' +_BIN_FMT = struct.Struct('=6sH4x') # ...followed by MemRegion as hypervisor memory -_CONSOLE_AND_PLATFORM_FORMAT = '32x12x224x44x' +_BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') def __init__(self, data): self.data = data try: -(signature, - revision) = \ -struct.unpack_from(SystemConfig._HEADER_FORMAT, self.data) +(signature, revision) = self._BIN_FMT.unpack_from(self.data) if signature != b'JHSYST': raise RuntimeError('Not a
[PATCH 04/11] pyjailhouse: config_parser: use I/O stream instead slice of bytes
This enables more flexibility in input types as long as they provide binary I/O capabilities. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 59 +++- tools/jailhouse-config-check | 4 +-- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 3bb2686a..3f20bc61 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -16,6 +16,7 @@ from __future__ import print_function import struct +import io from .extendedenum import ExtendedEnum @@ -39,29 +40,30 @@ class CStruct: return attrs @classmethod -def parse(cls, data): -obj, data = cls.parse_class(cls, data) -return obj, data +def parse(cls, stream): +obj = cls.parse_class(cls, stream) +return obj @staticmethod -def parse_class(cls, data): +def parse_class(cls, stream): +fmt = cls._BIN_FMT +data_tuple = fmt.unpack_from(stream.read(fmt.size)) obj = cls() slots = obj._slots() if len(slots) > 0: -data_tuple = cls._BIN_FMT.unpack_from(data) for assigment in zip(slots, data_tuple): setattr(obj, *assigment) -return obj, data[cls._BIN_FMT.size:] +return obj @staticmethod -def parse_array(cls, num, data): +def parse_array(cls, num, stream): array = [] for i in range(num): -obj, data = cls.parse(data) +obj = cls.parse(stream) array += [obj] -return array, data +return array def flag_str(enum_class, value, separator=' | '): @@ -197,28 +199,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, data, root_cell=False): +def parse(cls, stream, root_cell=False): try: if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from(data) +(signature, revision) = cls._BIN_FMT_HDR.unpack_from( +stream.read(cls._BIN_FMT_HDR.size)) if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -data = data[cls._BIN_FMT_HDR.size:] -self, data = cls.parse_class(cls, data) +self = cls.parse_class(cls, stream) self.name = self.name.decode() -data = data[self._cpu_sets:] # skip CPU set +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set -self.memory_regions, data = \ -cls.parse_array(MemRegion, self.memory_regions, data) -self.cache_regions, data = \ -cls.parse_array(CacheRegion, self.cache_regions, data) -self.irqchips, data = \ -cls.parse_array(Irqchip, self.irqchips, data) -self.pio_regions, data = \ -cls.parse_array(PIORegion, self.pio_regions, data) +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) return self except struct.error: @@ -239,21 +240,23 @@ class SystemConfig(CStruct): self.root_cell = CellConfig() @classmethod -def parse(cls, data): +def parse(cls, stream): try: hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = hdr_fmt.unpack_from(data) +(signature, revision) = \ +hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) if signature != b'JHSYST': raise RuntimeError('Not a root cell configuration') if revision != _CONFIG_REVISION: raise RuntimeError('Configuration file revision mismatch') -self, data = cls.parse_class(cls, data[hdr_fmt.size:]) -self.hypervisor_memory, data = MemRegion.parse(data) +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell -self.root_cell = CellConfig.parse(data[offs:], root_cell=True) +offs += hdr_fmt.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream, True) return self except struct.error: raise RuntimeError('Not a root cell configurat
[PATCH 02/11] pyjailhouse: config_parser: move parsing into class methods
... and use constructor for initialization only. This separation provides clarity on how to instantiate config components. This commit also serves as preparation for following one. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 88 +--- tools/jailhouse-config-check | 4 +- 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index b75b9312..e4ad2077 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -54,11 +54,11 @@ class JAILHOUSE_MEM(ExtendedEnum, int): class MemRegion: _BIN_FMT = struct.Struct('') -def __init__(self, region_struct): -(self.phys_start, - self.virt_start, - self.size, - self.flags) = self._BIN_FMT.unpack_from(region_struct) +def __init__(self): +self.phys_start = 0 +self.virt_start = 0 +self.size = 0 +self.flags = 0 def __str__(self): return (" phys_start: 0x%016x\n" % self.phys_start) + \ @@ -66,6 +66,15 @@ class MemRegion: (" size: 0x%016x\n" % self.size) + \ (" flags: " + flag_str(JAILHOUSE_MEM, self.flags)) +@classmethod +def parse(cls, data): +self = cls() +(self.phys_start, + self.virt_start, + self.size, + self.flags) = cls._BIN_FMT.unpack_from(data) +return self + def is_ram(self): return ((self.flags & (JAILHOUSE_MEM.READ | JAILHOUSE_MEM.WRITE | @@ -110,12 +119,22 @@ class CacheRegion: class Irqchip: _BIN_FMT = struct.Struct('QIIQQ') -def __init__(self, irqchip_struct): +def __init__(self): +self.address = 0 +self.id = 0 +self.pin_base = 0 +self.pin_bitmap_lo = 0 +self.pin_bitmap_hi = 0 + +@classmethod +def parse(cls, data): +self = cls() (self.address, self.id, self.pin_base, self.pin_bitmap_lo, - self.pin_bitmap_hi) = self._BIN_FMT.unpack_from(irqchip_struct) + self.pin_bitmap_hi) = cls._BIN_FMT.unpack_from(data) +return self def is_standard(self): return self.address == 0xfec0 @@ -124,16 +143,31 @@ class Irqchip: class PIORegion: _BIN_FMT = struct.Struct('HH') -def __init__(self, pio_struct): -(self.base, self.length) = self._BIN_FMT.unpack_from(pio_struct) +def __init__(self): +self.base = 0 +self.length = 0 + +@classmethod +def parse(cls, data): +self = cls() +(self.base, self.length) = self._BIN_FMT.unpack_from(data) +return self class CellConfig: _BIN_FMT = struct.Struct('=6sH32s4xIIQ8x32x') -def __init__(self, data, root_cell=False): -self.data = data - +def __init__(self): +self.name = "" +self.memory_regions = [] +self.irqchips = [] +self.pio_regions = [] +self.vpci_irq_base = 0 +self.cpu_reset_address = 0 + +@classmethod +def parse(cls, data, root_cell=False): +self = cls() try: (signature, revision, @@ -148,7 +182,7 @@ class CellConfig: self.num_pci_caps, self.num_stream_ids, self.vpci_irq_base, - self.cpu_reset_address) = self._BIN_FMT.unpack_from(self.data) + self.cpu_reset_address) = self._BIN_FMT.unpack_from(data) if not root_cell: if signature != b'JHCELL': raise RuntimeError('Not a cell configuration') @@ -157,40 +191,41 @@ class CellConfig: self.name = str(name.decode()) mem_region_offs = self._BIN_FMT.size + self.cpu_set_size -self.memory_regions = [] for n in range(self.num_memory_regions): -self.memory_regions.append( -MemRegion(self.data[mem_region_offs:])) + self.memory_regions.append(MemRegion.parse(data[mem_region_offs:])) mem_region_offs += MemRegion._BIN_FMT.size irqchip_offs = mem_region_offs + \ self.num_cache_regions * CacheRegion._BIN_FMT.size -self.irqchips = [] for n in range(self.num_irqchips): -self.irqchips.append( -Irqchip(self.data[irqchip_offs:])) +self.irqchips.append(Irqchip.parse(data[irqchip_offs:])) irqchip_offs += Irqchip._BIN_FMT.size pioregion_offs = irqchip_offs -self.pio_regions = [] for n in range(self.num_pio_regions): -self.pio_regions.append(PIORegion(self.data[pioregion_offs:])) +self.pio_regions.append(PIORegion.parse(data[pioregion_offs:])) pioregion_offs += PIORegio
[PATCH 07/11] pyjailhouse: config_parser: consolidate header parsing
This also enables probing for a configuration type. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 99 +++- tools/jailhouse-config-check | 5 +- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index a45aa7d7..2b47d9b6 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -197,8 +197,9 @@ class CellConfig(CStruct): '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT_HDR = struct.Struct('=6sH') _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') +_BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" @@ -209,38 +210,27 @@ class CellConfig(CStruct): self.cpu_reset_address = 0 @classmethod -def parse(cls, stream, root_cell=False): -try: -if not root_cell: -(signature, revision) = cls._BIN_FMT_HDR.unpack_from( -stream.read(cls._BIN_FMT_HDR.size)) -if signature != b'JHCELL': -raise RuntimeError('Not a cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set - -self.memory_regions = \ -cls.parse_array(MemRegion, self.memory_regions, stream) -self.cache_regions = \ -cls.parse_array(CacheRegion, self.cache_regions, stream) -self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) -self.pio_regions = \ -cls.parse_array(PIORegion, self.pio_regions, stream) - -return self -except struct.error: -raise RuntimeError('Not a %scell configuration' % - ('root ' if root_cell else '')) +def parse(cls, stream): +self = cls.parse_class(cls, stream) +self.name = self.name.decode().strip('\0') +stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +self.memory_regions = \ +cls.parse_array(MemRegion, self.memory_regions, stream) +self.cache_regions = \ +cls.parse_array(CacheRegion, self.cache_regions, stream) +self.irqchips = cls.parse_array(Irqchip, self.irqchips, stream) +self.pio_regions = \ +cls.parse_array(PIORegion, self.pio_regions, stream) + +return self class SystemConfig(CStruct): _BIN_FMT = struct.Struct('=4x') # ...followed by MemRegion as hypervisor memory _BIN_FMT_CONSOLE_AND_PLATFORM = struct.Struct('32x12x224x44x') +_BIN_SIGNATURE = b'JHSYST' # constructed fields __slots__ = 'hypervisor_memory', 'root_cell', @@ -251,22 +241,39 @@ class SystemConfig(CStruct): @classmethod def parse(cls, stream): -try: -hdr_fmt = CellConfig._BIN_FMT_HDR -(signature, revision) = \ -hdr_fmt.unpack_from(stream.read(hdr_fmt.size)) -if signature != b'JHSYST': -raise RuntimeError('Not a root cell configuration') -if revision != _CONFIG_REVISION: -raise RuntimeError('Configuration file revision mismatch') - -self = cls.parse_class(cls, stream) -self.hypervisor_memory = MemRegion.parse(stream) - -offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size -offs += hdr_fmt.size # skip header inside rootcell -stream.seek(offs, io.SEEK_CUR) -self.root_cell = CellConfig.parse(stream, True) -return self -except struct.error: -raise RuntimeError('Not a root cell configuration') +self = cls.parse_class(cls, stream) +self.hypervisor_memory = MemRegion.parse(stream) + +offs = cls._BIN_FMT_CONSOLE_AND_PLATFORM.size +offs += CellConfig._BIN_FMT_HDR.size # skip header inside rootcell +stream.seek(offs, io.SEEK_CUR) +self.root_cell = CellConfig.parse(stream) +return self + + +def parse(stream, config_expect=None): +fmt = CellConfig._BIN_FMT_HDR + +try: +(signature, revision) = fmt.unpack_from(stream.read(fmt.size)) +except struct.error: +raise RuntimeError('Not a Jailhouse configuration') + +if config_expect == None: +# Try probing +if signature == CellConfig._BIN_SIGNATURE: +config_expect = CellConfig +elif signature == SystemConfig._BIN_SIGNATURE: +config_expect = SystemConfig +else: +raise RuntimeError('Not a Jailhouse configuration') +elif config_expect._BIN
[PATCH 08/11] tools: config-check: add CPU overlap and boundary check
Also includes some minor PEP8 formatting. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 14 +-- tools/jailhouse-config-check | 48 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 2b47d9b6..2a0e6ec9 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -192,17 +192,19 @@ class PIORegion(CStruct): class CellConfig(CStruct): # slots with a '_' prefix in name are private -__slots__ = 'name', '_flags', '_cpu_sets', \ +__slots__ = 'name', '_flags', 'cpu_set', \ 'memory_regions', 'cache_regions', 'irqchips', 'pio_regions', \ '_pci_devices', '_pci_caps', '_stream_ids', \ 'vpci_irq_base', 'cpu_reset_address', _BIN_FIELD_NUM = len(__slots__) _BIN_FMT = struct.Struct('=32s4xIIQ8x32x') _BIN_FMT_HDR = struct.Struct('=6sH') +_BIN_FMT_CPU = struct.Struct('=Q') _BIN_SIGNATURE = b'JHCELL' def __init__(self): self.name = "" +self.cpu_set = set() self.memory_regions = [] self.irqchips = [] self.pio_regions = [] @@ -213,7 +215,15 @@ class CellConfig(CStruct): def parse(cls, stream): self = cls.parse_class(cls, stream) self.name = self.name.decode().strip('\0') -stream.seek(self._cpu_sets, io.SEEK_CUR) # skip CPU set + +cpu_fmt = cls._BIN_FMT_CPU +cpu_set_num = int(self.cpu_set / cpu_fmt.size) +self.cpu_set = set() +for set_idx in range(cpu_set_num): +cpu_bits = cpu_fmt.unpack_from(stream.read(cpu_fmt.size)) +for bit in range(cpu_fmt.size * 8): +if cpu_bits[0] & (1 << bit) > 0: +self.cpu_set.add(bit) self.memory_regions = \ cls.parse_array(MemRegion, self.memory_regions, stream) diff --git a/tools/jailhouse-config-check b/tools/jailhouse-config-check index 380f4a77..224c5fb5 100755 --- a/tools/jailhouse-config-check +++ b/tools/jailhouse-config-check @@ -64,10 +64,11 @@ for cfg in args.cellcfgs: cells.append(cell) print(" Non-root cell: %s (%s)" % (cell.name, cfg.name)) -ret=0 +ret = 0 +# Memory checks print("Overlapping memory regions inside cell:", end='') -found=False +found = False for cell in cells: for mem in cell.memory_regions: idx = cell.memory_regions.index(mem) @@ -79,16 +80,16 @@ for cell in cells: if (mem.virt_overlaps(mem2)): overlaps.append("virtually") if overlaps: -print("\n\nIn cell '%s', region %d" % (cell.name, idx)) +print("\nIn cell '%s', region %d" % (cell.name, idx)) print(str(mem)) -print(" and ".join(overlaps) + \ -" overlaps with region %d\n" % idx2 + str(mem2), end='') -found=True -ret=1 +print(" and ".join(overlaps) + + " overlaps with region %d\n" % idx2 + str(mem2), end='') +found = True +ret = 1 print("\n" if found else " None") print("Overlapping memory regions with hypervisor:", end='') -found=False +found = False for cell in cells: for mem in cell.memory_regions: if mem.phys_overlaps(sysconfig.hypervisor_memory): @@ -96,8 +97,35 @@ for cell in cells: print(str(mem)) print("overlaps with hypervisor memory region") print(str(sysconfig.hypervisor_memory), end='') -found=True -ret=1 +found = True +ret = 1 print("\n" if found else " None") +# CPU checks +print("Overlapping CPUs between cells:", end='') +found = False +for cell in non_root_cells: +cell_idx = cells.index(cell) +for cell2 in cells[cell_idx + 1:]: +overlap = cell.cpu_set & cell2.cpu_set +if overlap: +print("\nIn cell '%s' and '%s' following CPUs overlap: %s" % + (cell.name, cell2.name, str(overlap)), end='') +found = True +ret = 1 +print("\n" if found else " None") + +print("CPUs not in root cell CPU set:", end='') +found = False +for cell in non_root_cells: +diff = cell.cpu_set - root_cell.cpu_set +if diff: +print("\nIn cell '%s': %s" % (cell.name, str(diff)), end='') +found = True +ret = 1 +if found: +print("\nNote: root cell CPU set: %s\n" % str(root_cell.cpu_set)) +else: +print(" None") + exit(ret) -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200630064228.4742-8-andrej.utz%40st.oth-regensburg.de.
[PATCH 05/11] pyjailhouse: config_parser: parse pin_bitman in Irqchip as list
Just like the array of 4 in the C struct. Signed-off-by: Andrej Utz --- pyjailhouse/config_parser.py | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/pyjailhouse/config_parser.py b/pyjailhouse/config_parser.py index 3f20bc61..7b4872e0 100644 --- a/pyjailhouse/config_parser.py +++ b/pyjailhouse/config_parser.py @@ -155,20 +155,30 @@ class CacheRegion(CStruct): class Irqchip(CStruct): -__slots__ = 'address', 'id', 'pin_base', 'pin_bitmap_lo', 'pin_bitmap_hi', +__slots__ = 'address', 'id', 'pin_base', _BIN_FIELD_NUM = len(__slots__) -_BIN_FMT = struct.Struct('QIIQQ') +_BIN_FMT = struct.Struct('QII') +_BIN_FMT_PIN_MAP = struct.Struct('4I') + +# constructed fields +__slots__ += 'pin_bitmap', def __init__(self): self.address = 0 self.id = 0 self.pin_base = 0 -self.pin_bitmap_lo = 0 -self.pin_bitmap_hi = 0 +self.pin_bitmap = [0,0,0,0] def is_standard(self): return self.address == 0xfec0 +@classmethod +def parse(cls, stream): +self = cls.parse_class(cls, stream) +pin_fmt = cls._BIN_FMT_PIN_MAP +self.pin_bitmap = pin_fmt.unpack_from(stream.read(pin_fmt.size)) +return self + class PIORegion(CStruct): __slots__ = 'base', 'length', -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200630064228.4742-5-andrej.utz%40st.oth-regensburg.de.
[PATCH 3/3] configs: x86: f2a88xm-hd3: remove unreferenced PCI capabilities
Signed-off-by: Andrej Utz --- configs/x86/f2a88xm-hd3.c | 43 +++ 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/configs/x86/f2a88xm-hd3.c b/configs/x86/f2a88xm-hd3.c index e5dfd78f..b529e6a6 100644 --- a/configs/x86/f2a88xm-hd3.c +++ b/configs/x86/f2a88xm-hd3.c @@ -27,7 +27,7 @@ struct { struct jailhouse_irqchip irqchips[2]; struct jailhouse_pio pio_regions[8]; struct jailhouse_pci_device pci_devices[26]; - struct jailhouse_pci_capability pci_caps[27]; + struct jailhouse_pci_capability pci_caps[24]; } __attribute__((packed)) config = { .header = { .signature = JAILHOUSE_SYSTEM_SIGNATURE, @@ -384,7 +384,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x8, - .caps_start = 3, + .caps_start = 0, .num_caps = 4, .num_msi_vectors = 1, .msi_64bits = 1, @@ -398,7 +398,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x9, - .caps_start = 3, + .caps_start = 0, .num_caps = 4, .num_msi_vectors = 1, .msi_64bits = 1, @@ -440,7 +440,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x19, - .caps_start = 7, + .caps_start = 4, .num_caps = 5, .num_msi_vectors = 1, .msi_64bits = 1, @@ -468,7 +468,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x80, - .caps_start = 12, + .caps_start = 9, .num_caps = 4, .num_msi_vectors = 8, .msi_64bits = 1, @@ -482,7 +482,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x81, - .caps_start = 12, + .caps_start = 9, .num_caps = 4, .num_msi_vectors = 8, .msi_64bits = 1, @@ -496,7 +496,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x88, - .caps_start = 16, + .caps_start = 13, .num_caps = 2, .num_msi_vectors = 8, .msi_64bits = 1, @@ -524,7 +524,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x92, - .caps_start = 18, + .caps_start = 15, .num_caps = 2, .num_msi_vectors = 0, .msi_64bits = 0, @@ -552,7 +552,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x9a, - .caps_start = 18, + .caps_start = 15, .num_caps = 2, .num_msi_vectors = 0, .msi_64bits = 0, @@ -580,7 +580,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0xa2, - .caps_start = 20, + .caps_start = 17, .num_caps = 1, .num_msi_vectors = 0, .msi_64bits = 0, @@ -678,7 +678,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0xc3, - .caps_start = 21, + .caps_start = 18, .num_caps = 1, .num_msi_vectors = 0, .msi_64bits = 0, @@ -720,7 +720,7 @@ struct { .iommu = 0, .domain = 0x0, .bdf = 0x100, - .caps_start = 22, + .caps_start = 19, .num_caps = 5, .num_msi_vectors = 1, .msi_64bits = 1, @@ -731,25 +731,6 @@ struct { }, .pci_caps = { - /* PCIDevice: 00:00.2 */ - { - .id = PCI_CAP_ID_SECDEV, - .start = 0x40, - .len = 2, - .flags = 0, - }, - { - .id = PCI_CAP_ID_MSI, - .start = 0x54, - .len
[PATCH 1/3] tools: jailhouse-config-create: Improve code readability in template preprocessing
Move lines with similar context together and add some comments. This commit serves also as a preparation for the following commit. No functional change. Signed-off-by: Andrej Utz --- tools/jailhouse-config-create | 80 --- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create index 1e2df742..709cf2ef 100755 --- a/tools/jailhouse-config-create +++ b/tools/jailhouse-config-create @@ -240,41 +240,43 @@ if jh_enabled == '1': file=sys.stderr) sys.exit(1) -IOAPIC_MAX_PINS = 120 -int_src_count = IOAPIC_MAX_PINS - -(pcidevices, pcicaps, cnt) = sysfs_parser.parse_pcidevices() - -int_src_count += cnt -vtd_interrupt_limit = 2**math.ceil(math.log(int_src_count, 2)) - -product = [input_readline('/sys/class/dmi/id/sys_vendor', - True).rstrip(), - input_readline('/sys/class/dmi/id/product_name', - True).rstrip() - ] - -inmatemem = kmg_multiply_str(options.mem_inmates) -hvmem = [0, kmg_multiply_str(options.mem_hv)] - -(mem_regions, dmar_regions) = sysfs_parser.parse_iomem(pcidevices) -ourmem = parse_kernel_cmdline() -total = hvmem[1] + inmatemem +# Information collection +# +debug_console = DebugConsole(options.console) +# System infromation +product = [ +input_readline('/sys/class/dmi/id/sys_vendor', True).rstrip(), +input_readline('/sys/class/dmi/id/product_name', True).rstrip(), +] +cpu_count = count_cpus() mmconfig = MMConfig.parse() +# Query devices +(pci_devices, pci_caps, int_src_count) = sysfs_parser.parse_pcidevices() +(mem_regions, dmar_regions) = sysfs_parser.parse_iomem(pci_devices) +(port_regions, pm_timer_base) = sysfs_parser.parse_ioports() ioapics = sysfs_parser.parse_madt() - vendor = sysfs_parser.get_cpu_vendor() if vendor == 'GenuineIntel': -(iommu_units, extra_memregs) = sysfs_parser.parse_dmar(pcidevices, ioapics, +(iommu_units, extra_memregs) = sysfs_parser.parse_dmar(pci_devices, ioapics, dmar_regions) else: -(iommu_units, extra_memregs) = sysfs_parser.parse_ivrs(pcidevices, ioapics) +(iommu_units, extra_memregs) = sysfs_parser.parse_ivrs(pci_devices, ioapics) mem_regions += extra_memregs -# kernel does not have memmap region, pick one +IOAPIC_MAX_PINS = 120 +int_src_count += IOAPIC_MAX_PINS +vtd_interrupt_limit = 2**math.ceil(math.log(int_src_count, 2)) + +# Determine hypervisor memory +inmatemem = kmg_multiply_str(options.mem_inmates) +hvmem = [0, kmg_multiply_str(options.mem_hv)] +total = hvmem[1] + inmatemem + +ourmem = parse_kernel_cmdline() if ourmem is None: +# kernel does not have memmap region, pick one ourmem = alloc_mem(mem_regions, total) elif (total > ourmem[1]): raise RuntimeError('Your memmap reservation is too small you need >="' + @@ -282,22 +284,10 @@ elif (total > ourmem[1]): '"memmap=' + hex(total) + '$' + hex(ourmem[0]) + '"') hvmem[0] = ourmem[0] +mem_regions.append(sysfs_parser.MemRegion(ourmem[0] + hvmem[1], + ourmem[0] + hvmem[1] + inmatemem - 1, + 'JAILHOUSE Inmate Memory')) -inmatereg = sysfs_parser.MemRegion(ourmem[0] + hvmem[1], - ourmem[0] + hvmem[1] + inmatemem - 1, - 'JAILHOUSE Inmate Memory') -mem_regions.append(inmatereg) - -cpucount = count_cpus() - -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 = { 'mem_regions': mem_regions, 'port_regions': port_regions, @@ -305,9 +295,9 @@ kwargs = { 'argstr': ' '.join(sys.argv), 'hvmem': hvmem, 'product': product, -'pcidevices': pcidevices, -'pcicaps': pcicaps, -'cpucount': cpucount, +'pcidevices': pci_devices, +'pcicaps': pci_caps, +'cpucount': cpu_count, 'irqchips': ioapics, 'pm_timer_base': pm_timer_base, 'vtd_interrupt_limit': vtd_interrupt_limit, @@ -316,6 +306,8 @@ kwargs = { 'debug_console': debug_console, } -f.write(tmpl.render(**kwargs)) +tmpl = Template(filename=os.path.join(options.template_dir, + 'root-cell-config.c.tmpl')) -f.close() +with open(options.file, 'w') as f: +f.write(tmpl.render(**kwargs)) -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/2020060913
[PATCH 2/3] tools: jailhouse-config-create: move PCI capability collector from sysfs_parser
Fixes generation of unreferenced PCI capabilities inside cell configs on AMD systems. They occur due to removal of the IOMMU from the PCI devices list after its capabilities have been collected. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py| 19 +-- tools/jailhouse-config-create | 24 ++-- tools/jailhouse-hardware-check | 2 +- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index e59586cd..8debf460 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -280,31 +280,14 @@ def parse_ioports(): def parse_pcidevices(): -int_src_cnt = 0 devices = [] -caps = [] basedir = '/sys/bus/pci/devices' list = input_listdir(basedir, ['*/config']) for dir in list: d = PCIDevice.parse_pcidevice_sysfsdir(basedir, dir) if d is not None: -if d.caps: -duplicate = False -# look for duplicate capability patterns -for d2 in devices: -if d2.caps == d.caps: -# reused existing capability list, but record all users -d2.caps[0].comments.append(str(d)) -d.caps_start = d2.caps_start -duplicate = True -break -if not duplicate: -d.caps[0].comments.append(str(d)) -d.caps_start = len(caps) -caps.extend(d.caps) -int_src_cnt += max(d.num_msi_vectors, d.num_msix_vectors) devices.append(d) -return (devices, caps, int_src_cnt) +return devices def parse_madt(): diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create index 709cf2ef..0f75be4f 100755 --- a/tools/jailhouse-config-create +++ b/tools/jailhouse-config-create @@ -253,7 +253,7 @@ cpu_count = count_cpus() mmconfig = MMConfig.parse() # Query devices -(pci_devices, pci_caps, int_src_count) = sysfs_parser.parse_pcidevices() +pci_devices = sysfs_parser.parse_pcidevices() (mem_regions, dmar_regions) = sysfs_parser.parse_iomem(pci_devices) (port_regions, pm_timer_base) = sysfs_parser.parse_ioports() ioapics = sysfs_parser.parse_madt() @@ -266,7 +266,27 @@ else: mem_regions += extra_memregs IOAPIC_MAX_PINS = 120 -int_src_count += IOAPIC_MAX_PINS +int_src_count = IOAPIC_MAX_PINS + +# Collect all PCI capabilities +pci_caps = [] +for i,d in enumerate(pci_devices): +if d.caps: +duplicate = False +# look for duplicate capability patterns +for d2 in pci_devices[:i]: +if d2.caps == d.caps: +# reused existing capability list, but record all users +d2.caps[0].comments.append(str(d)) +d.caps_start = d2.caps_start +duplicate = True +break +if not duplicate: +d.caps[0].comments.append(str(d)) +d.caps_start = len(pci_caps) +pci_caps.extend(d.caps) +int_src_count += max(d.num_msi_vectors, d.num_msix_vectors) + vtd_interrupt_limit = 2**math.ceil(math.log(int_src_count, 2)) # Determine hypervisor memory diff --git a/tools/jailhouse-hardware-check b/tools/jailhouse-hardware-check index 9e90250d..f9b2cf58 100755 --- a/tools/jailhouse-hardware-check +++ b/tools/jailhouse-hardware-check @@ -127,7 +127,7 @@ if os.uname()[4] not in ('x86_64', 'i686'): ioapics = sysfs_parser.parse_madt() -pci_devices, _, _ = sysfs_parser.parse_pcidevices() +pci_devices = sysfs_parser.parse_pcidevices() (cpu_vendor, cpu_features, cpu_count) = parse_cpuinfo() -- 2.27.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20200609131143.2133316-2-andrej.utz%40st.oth-regensburg.de.
Re: AMD: non-root linux inmates
Hello everyone, On 27.02.20 15:46, Ralf Ramsauer wrote: Hi, On 27/02/2020 15:24, rayma...@gmail.com wrote: Hi all, I'd just like to add that I am experiencing the exact same issue as described by Ralf on an AMD EPYC 7351P. aah, 'good' to hear! The stacktrace is the same as well: FATAL: Setting invalid LVT delivery mode (reg 35, value 0700 However, on my side I cannot get past this by just handing over the xAPIC enabled; I get the same trace.. I added the printk's from Ralf's diff and get the same output: ... Before disabling: 1 After disabling: 0 After reenabling: 0 Great. So we definitely have a systematic hardware bug that doesn't only affect our CPU. ... Did I already post my local hacky workaround? diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c index de691329..7f51b062 100644 --- a/hypervisor/arch/x86/apic.c +++ b/hypervisor/arch/x86/apic.c @@ -340,7 +340,7 @@ void apic_clear(void) /* Finally, reset the TPR again and disable the APIC */ apic_ops.write(APIC_REG_TPR, 0); - apic_ops.write(APIC_REG_SVR, 0xff); + //apic_ops.write(APIC_REG_SVR, 0xff); } static bool apic_valid_ipi_mode(u32 lo_val) Maybe we could try to reach out to AMD via some kernel mailing list? ... I presume that by "hand over the xAPIC enabled" you mean disabling the write to APIC_REG_SVR? That's what I did but it did not do the trick unfortunately. Yep. Andrej, did we have to adjust anything else? It's been a while that we've been working on that issue, and I don't have the exact details in mind. However, Andrej wanted to pick up the topic again soon. Thanks, Ralf From what I've gathered, Jailhouse works as expected. The inmates may (or may not) require the APIC, so they also are responsible for handling possible hardware bugs. For this specific case the Linux kernel (inmate) needs a workaround during APIC initialization. However it already has a ancient (pre-git) quirk[1] to delegate interrupts to 8259A in case LVTs are not masked after reset. Nowadays this code seems strange, as the masked state is required per APIC specification (for Intel and AMD). Jan, do you know more about this? Interestingly enough this quirk is also the actual trigger of the aforementioned "invalid LVT delivery mode" error in Jailhouse. For now I've appended a patch with a simple workaround for the kernel. Hope that helps. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/x86/kernel/apic/apic.c?h=v5.4.24#n1716 Thanks, Andrej Utz -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/bef2d9ff-b6b4-8927-e36a-e9ec41d0ed1a%40st.othr.de. diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 0428ad289899..836436ce05ba 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1699,6 +1699,14 @@ static void setup_local_APIC(void) value |= SPURIOUS_APIC_VECTOR; apic_write(APIC_SPIV, value); + // HACK: some CPUs (e.g. the AMD Ryzen family) fail to reset LVT_LINT + // registers according to specification, so we help them do that + if (((value = apic_read(APIC_LVT0)) & APIC_LVT_MASKED) == 0) + apic_write(APIC_LVT0, value | APIC_LVT_MASKED); + + if (((value = apic_read(APIC_LVT1)) & APIC_LVT_MASKED) == 0) + apic_write(APIC_LVT1, value | APIC_LVT_MASKED); + perf_events_lapic_init(); /*
[PATCH 2/2] tools: gcov: Adjust Gcov counter count for GCC versions >= 7.0
As in linux/kernel/gcov/gcov.h. Otherwise the extract tool will access Gcov data using garbage as an index and segfault. Signed-off-by: Andrej Utz --- tools/jailhouse-gcov-extract.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/jailhouse-gcov-extract.c b/tools/jailhouse-gcov-extract.c index c126f976..5bb337a5 100644 --- a/tools/jailhouse-gcov-extract.c +++ b/tools/jailhouse-gcov-extract.c @@ -43,7 +43,9 @@ typedef long gcov_type; typedef long long gcov_type; #endif -#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1) +#if (__GNUC__ >= 7) +#define GCOV_COUNTERS 9 +#elif (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1) #define GCOV_COUNTERS 10 #elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9 #define GCOV_COUNTERS 9 -- 2.24.1 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20191220154126.1136-2-andrej.utz%40st.oth-regensburg.de.
[PATCH 1/2] tools: gcov: Fix missing symbol when compiling on newer GCC versions
Starting from GCC 7.1, __gcov_exit is a new symbol expected to be implemented when using GCOV. Signed-off-by: Andrej Utz --- hypervisor/gcov.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hypervisor/gcov.c b/hypervisor/gcov.c index 6055bdd5..ec3a0d9e 100644 --- a/hypervisor/gcov.c +++ b/hypervisor/gcov.c @@ -35,6 +35,7 @@ void gcov_init(void) { void __gcov_init(struct gcov_min_info *info); void __gcov_merge_add(void *counters, unsigned int n_counters); +void __gcov_exit(void); /* just link them all together and leave the head in the header * where a processing tool can find it */ @@ -48,3 +49,7 @@ void __gcov_init(struct gcov_min_info *info) void __gcov_merge_add(void *counters, unsigned int n_counters) { } + +void __gcov_exit(void) +{ +} -- 2.24.1 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20191220154126.1136-1-andrej.utz%40st.oth-regensburg.de.
Re: [PATCH v4 12/13] pyjailhouse: x86: implement pio_regions generator
Hi Ralf, On 15.10.19 18:22, Ralf Ramsauer wrote: From: Andrej Utz This replaces the former static port list with actual port regions as listed in /proc/ioports. A whitelist selectively allows access to known ports (if present). PCI devices above 0xcff are permitted as well. However, not all ports that are in use are listed in ioports, so the generated list may still ne further tuning. Signed-off-by: Andrej Utz [ralf: s/permitted/whitelist/, autodetect VGA, whitelist serial, whitelist PCI devices, amend commit message, improve __str__ methods, ensure pep8 conformity] Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 89 +++ tools/jailhouse-config-create | 15 +- tools/root-cell-config.c.tmpl | 12 ++--- 3 files changed, 96 insertions(+), 20 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 1d00f364..3cefc2c7 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -18,6 +18,7 @@ # to change the generated C-code. +import re import struct import os import fnmatch @@ -25,6 +26,7 @@ import fnmatch from .pci_defs import PCI_CAP_ID, PCI_EXT_CAP_ID root_dir = "/" +bdf_regex = re.compile(r'\w{4}:\w{2}:\w{2}\.\w') def set_root_dir(dir): @@ -147,6 +149,65 @@ def parse_iomem(pcidevices): return ret, dmar_regions +def ioports_search_pci_devices(tree): +ret = [] + +if tree.region and bdf_regex.match(tree.region.typestr): +ret.append(tree.region) +else: +for subtree in tree: +ret += ioports_search_pci_devices(subtree) + +return ret + + +def parse_ioports(): +tree = IORegionTree.parse_io_file('/proc/ioports', PortRegion) + +pm_timer_base = tree.find_regions_by_name('ACPI PM_TMR') +if len(pm_timer_base) != 1: +raise RuntimeError('Found %u entries for ACPI PM_TMR (expected 1)' % + len(pm_timer_base)) +pm_timer_base = pm_timer_base[0].start + +leaves = tree.get_leaves() + +# Never expose PCI config space ports to the user +leaves = list(filter(lambda p: p.start != 0xcf8, leaves)) + +# Drop everything above 0xd00 +leaves = list(filter(lambda p: p.start < 0xd00, leaves)) + +whitelist = [ +0x40, # PIT +0x60, # keyboard +0x61, # HACK: NMI status/control +0x64, # I8042 +0x70, # RTC +0x2f8, # serial +0x3f8, # serial I see you added the onboard UARTs to the whitelist. Shouldn't we disallow them if they collide with Jailhouse' own debug port? Thanks, Andrej Utz +] + +pci_devices = ioports_search_pci_devices(tree) + +# Drop devices below 0xd00 as leaves already contains them. Access should +# not be permitted by default. +pci_devices = list(filter(lambda p: p.start >= 0xd00, pci_devices)) +for pci_device in pci_devices: +pci_device.permit = True + +for r in leaves: +typestr = r.typestr.lower() +if r.start in whitelist or \ + True in [vga in typestr for vga in ['vesa', 'vga']]: +r.permit = True + +leaves += pci_devices +leaves.sort(key=lambda r: r.start) + +return leaves, pm_timer_base + + def parse_pcidevices(): int_src_cnt = 0 devices = [] @@ -831,6 +892,19 @@ class MemRegion(IORegion): return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE' +class PortRegion(IORegion): +def __init__(self, start, stop, typestr, permit=False, comments=None): +super(PortRegion, self).__init__(start, stop, typestr, comments) +self.permit = permit + +def __str__(self): +return 'Port I/O: %04x-%04x : %s' % \ +(self.start, self.stop, super(PortRegion, self).__str__()) + +def size(self): +return super(PortRegion, self).size() + 1 + + class IOAPIC: def __init__(self, id, address, gsi_base, iommu=0, bdf=0): self.id = id @@ -854,6 +928,21 @@ class IORegionTree: self.parent = None self.children = [] +def __iter__(self): +for child in self.children: +yield child + +def get_leaves(self): +leaves = [] + +if len(self.children): +for child in self.children: +leaves.extend(child.get_leaves()) +elif self.region is not None: +leaves.append(self.region) + +return leaves + # find specific regions in tree def find_regions_by_name(self, name): regions = [] diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create index c3226dde..250785af 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 lin
[PATCH v3 14/14] pyjailhouse: x86: implement pio_regions generator
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 --- pyjailhouse/sysfs_parser.py | 60 +++ tools/jailhouse-config-create | 15 ++--- tools/root-cell-config.c.tmpl | 11 +++ 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 0dcc7475..baed6a40 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -147,6 +147,43 @@ def parse_iomem(pcidevices): return ret, dmar_regions +def parse_ioports(): +tree = IORegionTree.parse_io_file('/proc/ioports', PortRegion) + +pm_timer_base = tree.find_regions_by_name('ACPI PM_TMR') +if len(pm_timer_base) != 1: +raise RuntimeError('Found %u entries for ACPI PM_TMR (expected 1)' % + len(pm_timer_base)) +pm_timer_base = pm_timer_base[0].start + +leaves = tree.get_leaves() + +# Never expose PCI config space ports to the user +leaves = list(filter(lambda p: p.start != 0xcf8, leaves)) + +static_regions = [PortRegion(0xd00, 0x, 'HACK: PCI bus', True), + PortRegion(0x3b0, 0x3df, 'VGA', True)] + +leaves += static_regions + +permitted = [ +0x40, # PIT +0x60, # keyboard +0x61, # HACK: NMI status/control +0x64, # I8042 +0x70, # RTC +0x3b0, # VGA +] + +for r in leaves: +if r.start in permitted: +r.permit = True + +leaves.sort(key=lambda r: r.start) + +return leaves, pm_timer_base + + def parse_pcidevices(): int_src_cnt = 0 devices = [] @@ -829,6 +866,18 @@ class MemRegion(IORegion): return 'JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE' +class PortRegion(IORegion): +def __init__(self, start, stop, typestr, permit=False): +super(PortRegion, self).__init__(start, stop, typestr) +self.permit = permit + +def __str__(self): +return self.typestr + +def size(self): +return super(PortRegion, self).size() + 1 + + class IOAPIC: def __init__(self, id, address, gsi_base, iommu=0, bdf=0): self.id = id @@ -852,6 +901,17 @@ class IORegionTree: self.parent = None self.children = [] +def get_leaves(self): +leaves = [] + +if len(self.children): +for child in self.children: +leaves.extend(child.get_leaves()) +elif self.region is not None: +leaves.append(self.region) + +return leaves + # find specific regions in tree def find_regions_by_name(self, name): regions = [] diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create index cfa7fbad..e1888ea6 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 @@ -302,7 +290,7 @@ 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) @@ -312,6 +300,7 @@ tmpl = Template(filename=os.path.join(options.template_dir, 'root-cell-config.c.tmpl')) kwargs = { '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 d884089a..0afe7e86 100644 --- a/tools/root-cell-config.c.tmpl +++ b/tools/root-cell-config.c.tmpl @@ -47,7 +47,7 @@ struct { __u64 cpus[${int((cpucount + 63) / 64)}]; struct jailhouse_memory mem_regions[${len(mem_regions)}]; struct jailhouse_irqchip irqchips[${len(irqchips)}]; - struct jailhouse_pio pio_regions[6]; + struct jailhouse_pio pio_regions[${len([1 for r in port_regions if r.permit])}]; struct jailhouse_pci_device pci_devices[${len(pcidevices)}]; struct jailhouse_pci_capability pci_caps[${len(pcicaps)}]; } __attribute__((packed)) config = { @@ -154,12 +154,9 @@ struct { }, .pio_regions = { - PIO_RANGE(0x40, 4), /* PIT */ - PIO_RANGE(0x60, 2), /* HACK: NMI status/control */ - PIO_RANGE(0x64, 1), /* I8042
[PATCH v3 13/14] pyjailhouse: sysfs_parser: simplify integer formatting for regions in config template
Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 11 +++ tools/root-cell-config.c.tmpl | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 0f91d928..0dcc7475 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -791,6 +791,17 @@ class IORegion: def size(self): return int(self.stop - self.start) +def start_str(self): +# This method is used in root-cell-config.c.tmpl + +# Python 2 appends a 'L' to hexadecimal format of large integers, +# therefore .strip('L') is necessary. +return hex(self.start).strip('L') + +def size_str(self): +# Comments from start_str() apply here as well. +return hex(self.size()).strip('L') + class MemRegion(IORegion): def __init__(self, start, stop, typestr, comments=None): diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl index b6522ce1..d884089a 100644 --- a/tools/root-cell-config.c.tmpl +++ b/tools/root-cell-config.c.tmpl @@ -132,9 +132,9 @@ struct { /* ${c} */ % endfor { - .phys_start = ${hex(r.start).strip('L')}, - .virt_start = ${hex(r.start).strip('L')}, - .size = ${hex(r.size()).strip('L')}, + .phys_start = ${r.start_str()}, + .virt_start = ${r.start_str()}, + .size = ${r.size_str()}, .flags = ${r.flagstr('\t\t')}, }, % endfor -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930193857.2866-5-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 10/14] pyjailhouse: sysfs_parser: remove parse_iomem_file
From: Ralf Ramsauer We don't need it, call IORegionTree parser directly. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 50ca62fc..d2b36876 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -97,8 +97,8 @@ def input_listdir(dir, wildcards): def parse_iomem(pcidevices): -(regions, dmar_regions) = IOMemRegionTree.parse_iomem_tree( -IOMemRegionTree.parse_iomem_file()) +tree = IORegionTree.parse_io_file('/proc/iomem', MemRegion) +regions, dmar_regions = IOMemRegionTree.parse_iomem_tree(tree) rom_region = MemRegion(0xc, 0xd, 'ROMs') add_rom_region = False @@ -902,10 +902,6 @@ class IOMemRegionTree: return [before_kernel, kernel_region, after_kernel] -@staticmethod -def parse_iomem_file(): -return IORegionTree.parse_io_file('/proc/iomem', MemRegion) - # find specific regions in tree @staticmethod def find_regions_by_name(tree, name): -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930193857.2866-2-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 12/14] pyjailhouse: sysfs_parser: abstract parts of MemRegion into IORegion
This prepares for the refactor in following commits. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 3027f82e..0f91d928 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -779,20 +779,30 @@ class PCIPCIBridge(PCIDevice): return (secondbus, subordinate) -class MemRegion: -def __init__(self, start, stop, typestr, comments=None): +class IORegion: +def __init__(self, start, stop, typestr): self.start = start self.stop = stop self.typestr = typestr + +def __str__(self): +return '%08x-%08x : %s' % (self.start, self.stop, self.typestr) + +def size(self): +return int(self.stop - self.start) + + +class MemRegion(IORegion): +def __init__(self, start, stop, typestr, comments=None): +super(MemRegion, self).__init__(start, stop, typestr) self.comments = comments or [] def __str__(self): -return 'MemRegion: %08x-%08x : %s' % \ -(self.start, self.stop, self.typestr) +return 'MemRegion: %s' % super(MemRegion, self).__str__() def size(self): # round up to full PAGE_SIZE -return int((self.stop - self.start + 0xfff) / 0x1000) * 0x1000 +return int((super(MemRegion, self).size() + 0xfff) / 0x1000) * 0x1000 def flagstr(self, p=''): if ( -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930193857.2866-4-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 11/14] pyjailhouse: sysfs_parser: move find_regions_by_name to IORegionTree
From: Ralf Ramsauer We will reuse that function, move it to IORegionTree. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 42 + 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index d2b36876..3027f82e 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -831,6 +831,23 @@ class IORegionTree: self.parent = None self.children = [] +# find specific regions in tree +def find_regions_by_name(self, name): +regions = [] + +for tree in self.children: +r = tree.region +s = r.typestr + +if s.find(name) >= 0: +regions.append(r) + +# if the tree continues recurse further down ... +if len(tree.children) > 0: +regions.extend(tree.find_regions_by_name(name)) + +return regions + @staticmethod def parse_io_line(line, TargetClass): region, type = line.split(' : ', 1) @@ -902,25 +919,6 @@ class IOMemRegionTree: return [before_kernel, kernel_region, after_kernel] -# find specific regions in tree -@staticmethod -def find_regions_by_name(tree, name): -regions = [] - -for tree in tree.children: -r = tree.region -s = r.typestr - -if s.find(name) >= 0: -regions.append(r) - -# if the tree continues recurse further down ... -if len(tree.children) > 0: -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, name)) - -return regions - # recurse down the tree @staticmethod def parse_iomem_tree(tree): @@ -944,10 +942,8 @@ class IOMemRegionTree: # generally blacklisted, with a few exceptions if s.lower() == 'reserved': -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'HPET')) -dmar_regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'dmar')) +regions.extend(tree.find_regions_by_name('HPET')) +dmar_regions.extend(tree.find_regions_by_name('dmar')) continue # if the tree continues recurse further down ... -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930193857.2866-3-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 09/14] pyjailhouse: sysfs_parser: entirely separate IO parsers
From: Ralf Ramsauer Everything is in place, we can separate IOMemRegionTree from IORegionTree. Let's give IORegionTree its own constructor. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index cbd6069d..50ca62fc 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -825,6 +825,12 @@ class IOAPIC: class IORegionTree: +def __init__(self, region, level): +self.region = region +self.level = level +self.parent = None +self.children = [] + @staticmethod def parse_io_line(line, TargetClass): region, type = line.split(' : ', 1) @@ -836,13 +842,13 @@ class IORegionTree: @staticmethod def parse_io_file(filename, TargetClass): -root = IOMemRegionTree(None, 0) +root = IORegionTree(None, 0) f = input_open(filename) lastlevel = 0 lastnode = root for line in f: level, r = IORegionTree.parse_io_line(line, TargetClass) -t = IOMemRegionTree(r, level) +t = IORegionTree(r, level) if t.level > lastlevel: t.parent = lastnode if t.level == lastlevel: @@ -862,12 +868,6 @@ class IORegionTree: class IOMemRegionTree: -def __init__(self, region, level): -self.region = region -self.level = level -self.parent = None -self.children = [] - @staticmethod def regions_split_by_kernel(tree): kernel = [x for x in tree.children if -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930193857.2866-1-andrej.utz%40st.oth-regensburg.de.
Re: [PATCH v3 00/14] pyjailhouse: x86: Implement config generator for port I/O
Hi Jan, On 30.09.19 21:19, Jan Kiszka wrote: On 30.09.19 21:13, Andrej Utz wrote: This patch series eases configuration of port I/O devices for x86 plattforms by generating an initial PIO region list. To sustain previous behavior, most entries are disabled (commented out). Only whitelisted device ports are allowed. This includes the peripheral PCI port space. Did you also try what explodes when enforcing the generated list? I mean, if there is no mess like with hidden memory regions, things just Just Work (TM). Not yet. Analysis of additional whitelist candidates shall follow. Thanks, Andrej Utz Will have a look at the details soon. Thanks, Jan Additionally the code paths for memory region generation are cleaned up and streamlined. Andrej Utz (4): tools: jailhouse-config-create: Rename regions to mem_regions in preparation for port_regions pyjailhouse: sysfs_parser: abstract parts of MemRegion into IORegion pyjailhouse: sysfs_parser: simplify integer formatting for regions in config template pyjailhouse: x86: implement pio_regions generator Ralf Ramsauer (10): tools: jailhoues-cell-linux: cosmetic fixup pyjailhouse: sysfs_parser: remove dead code pyjailhouse: sysfs_parser: minor stylistic fixups pyjailhouse: sysfs_parser: simplify statement pyjailhouse: sysfs_parser: introduce new class IORegionTree pyjailhouse: sysfs_parser: move parse_iomem_file to the new parser pyjailhouse: sysfs_parser: make regions_split_by_kernel static pyjailhouse: sysfs_parser: entirely separate IO parsers pyjailhouse: sysfs_parser: remove parse_iomem_file pyjailhouse: sysfs_parser: move find_regions_by_name to IORegionTree pyjailhouse/sysfs_parser.py | 245 ++ tools/jailhouse-cell-linux | 8 +- tools/jailhouse-config-create | 25 +--- tools/root-cell-config.c.tmpl | 21 ++- 4 files changed, 176 insertions(+), 123 deletions(-) -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/724ad1a6-00b9-b921-122c-25c26e75349c%40st.oth-regensburg.de.
[PATCH v3 07/14] pyjailhouse: sysfs_parser: move parse_iomem_file to the new parser
From: Ralf Ramsauer Move the next part to the new class: the whole file parser. For the moment, this leaves an ugly one-liner in parse_iomem_file, but let's keep it for the moment -- we'll clean that up later. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 50 - 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index a179461d..b72be367 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -834,6 +834,32 @@ class IORegionTree: return level, TargetClass(int(region[0], 16), int(region[1], 16), type) +@staticmethod +def parse_io_file(filename, TargetClass): +root = IOMemRegionTree(None, 0) +f = input_open(filename) +lastlevel = 0 +lastnode = root +for line in f: +level, r = IORegionTree.parse_io_line(line, TargetClass) +t = IOMemRegionTree(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 + class IOMemRegionTree: def __init__(self, region, level): @@ -877,29 +903,7 @@ class IOMemRegionTree: @staticmethod def parse_iomem_file(): -root = IOMemRegionTree(None, 0) -f = input_open('/proc/iomem') -lastlevel = 0 -lastnode = root -for line in f: -level, r = IORegionTree.parse_io_line(line, MemRegion) -t = IOMemRegionTree(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 +return IORegionTree.parse_io_file('/proc/iomem', MemRegion) # find specific regions in tree @staticmethod -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-8-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 05/14] pyjailhouse: sysfs_parser: simplify statement
From: Ralf Ramsauer No functional change. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 464b6a80..b0a9bf44 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -113,7 +113,7 @@ def parse_iomem(pcidevices): append_r = True # filter the list for MSI-X pages for d in pcidevices: -if d.msix_address >= r.start and d.msix_address <= r.stop: +if r.start <= d.msix_address <= r.stop: if d.msix_address > r.start: head_r = MemRegion(r.start, d.msix_address - 1, r.typestr, r.comments) -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-6-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 04/14] pyjailhouse: sysfs_parser: minor stylistic fixups
From: Ralf Ramsauer The sysfs_parser is written in python and not in C. We can save some parentheses. No functional change. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index f65eacc8..464b6a80 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -125,7 +125,7 @@ def parse_iomem(pcidevices): append_r = False break # filter out the ROMs -if (r.start >= rom_region.start and r.stop <= rom_region.stop): +if r.start >= rom_region.start and r.stop <= rom_region.stop: add_rom_region = True append_r = False # filter out and save DMAR regions @@ -141,7 +141,7 @@ def parse_iomem(pcidevices): # newer Linux kernels will report the first page as reserved # it is needed for CPU init so include it anyways -if (ret[0].typestr == 'System RAM' and ret[0].start == 0x1000): +if ret[0].typestr == 'System RAM' and ret[0].start == 0x1000: ret[0].start = 0 return ret, dmar_regions @@ -835,7 +835,7 @@ class IOMemRegionTree: kernel = [x for x in self.children if x.region.typestr.startswith('Kernel ')] -if (len(kernel) == 0): +if len(kernel) == 0: return [self.region] r = self.region @@ -846,20 +846,20 @@ class IOMemRegionTree: # align this for 16M, but only if we have enough space kernel_stop = (kernel_stop & ~0xFF) + 0xFF -if (kernel_stop > r.stop): +if kernel_stop > r.stop: kernel_stop = r.stop before_kernel = None after_kernel = None # before Kernel if any -if (r.start < kernel_start): +if r.start < kernel_start: before_kernel = MemRegion(r.start, kernel_start - 1, s) kernel_region = MemRegion(kernel_start, kernel_stop, "Kernel") # after Kernel if any -if (r.stop > kernel_stop): +if r.stop > kernel_stop: after_kernel = MemRegion(kernel_stop + 1, r.stop, s) return [before_kernel, kernel_region, after_kernel] @@ -907,11 +907,11 @@ class IOMemRegionTree: r = tree.region s = r.typestr -if (s.find(name) >= 0): +if s.find(name) >= 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_regions_by_name(tree, name)) @@ -934,15 +934,12 @@ class IOMemRegionTree: regions.extend(tree.regions_split_by_kernel()) continue -# blacklisted on all levels -if ( -(s.find('PCI MMCONFIG') >= 0) or -(s.find('APIC') >= 0) # covers both APIC and IOAPIC -): +# blacklisted on all levels, covers both APIC and IOAPIC +if (s.find('PCI MMCONFIG') >= 0) or (s.find('APIC') >= 0): continue # generally blacklisted, with a few exceptions -if (s.lower() == 'reserved'): +if s.lower() == 'reserved': regions.extend( IOMemRegionTree.find_regions_by_name(tree, 'HPET')) dmar_regions.extend( @@ -950,8 +947,8 @@ class IOMemRegionTree: continue # if the tree continues recurse further down ... -if (len(tree.children) > 0): -(temp_regions, temp_dmar_regions) = \ +if len(tree.children) > 0: +temp_regions, temp_dmar_regions = \ IOMemRegionTree.parse_iomem_tree(tree) regions.extend(temp_regions) dmar_regions.extend(temp_dmar_regions) -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-5-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 00/14] pyjailhouse: x86: Implement config generator for port I/O
This patch series eases configuration of port I/O devices for x86 plattforms by generating an initial PIO region list. To sustain previous behavior, most entries are disabled (commented out). Only whitelisted device ports are allowed. This includes the peripheral PCI port space. Additionally the code paths for memory region generation are cleaned up and streamlined. Andrej Utz (4): tools: jailhouse-config-create: Rename regions to mem_regions in preparation for port_regions pyjailhouse: sysfs_parser: abstract parts of MemRegion into IORegion pyjailhouse: sysfs_parser: simplify integer formatting for regions in config template pyjailhouse: x86: implement pio_regions generator Ralf Ramsauer (10): tools: jailhoues-cell-linux: cosmetic fixup pyjailhouse: sysfs_parser: remove dead code pyjailhouse: sysfs_parser: minor stylistic fixups pyjailhouse: sysfs_parser: simplify statement pyjailhouse: sysfs_parser: introduce new class IORegionTree pyjailhouse: sysfs_parser: move parse_iomem_file to the new parser pyjailhouse: sysfs_parser: make regions_split_by_kernel static pyjailhouse: sysfs_parser: entirely separate IO parsers pyjailhouse: sysfs_parser: remove parse_iomem_file pyjailhouse: sysfs_parser: move find_regions_by_name to IORegionTree pyjailhouse/sysfs_parser.py | 245 ++ tools/jailhouse-cell-linux| 8 +- tools/jailhouse-config-create | 25 +--- tools/root-cell-config.c.tmpl | 21 ++- 4 files changed, 176 insertions(+), 123 deletions(-) -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-1-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 08/14] pyjailhouse: sysfs_parser: make regions_split_by_kernel static
From: Ralf Ramsauer No need to access our own tree, make this method static. This allows us to fully seperate IORegionTree from IOMemRegionTree soon. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index b72be367..cbd6069d 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -868,14 +868,15 @@ class IOMemRegionTree: self.parent = None self.children = [] -def regions_split_by_kernel(self): -kernel = [x for x in self.children if +@staticmethod +def regions_split_by_kernel(tree): +kernel = [x for x in tree.children if x.region.typestr.startswith('Kernel ')] if len(kernel) == 0: -return [self.region] +return [tree.region] -r = self.region +r = tree.region s = r.typestr kernel_start = kernel[0].region.start @@ -937,8 +938,8 @@ class IOMemRegionTree: # System RAM on the first level will be added completely, # if they don't contain the kernel itself, if they do, # we split them -if (tree.level == 1 and s == 'System RAM'): -regions.extend(tree.regions_split_by_kernel()) +if tree.level == 1 and s == 'System RAM': +regions.extend(IOMemRegionTree.regions_split_by_kernel(tree)) continue # blacklisted on all levels, covers both APIC and IOAPIC -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-9-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 03/14] pyjailhouse: sysfs_parser: remove dead code
From: Ralf Ramsauer There are no callers of __str__. Remove it. Seems to be a development artifact of earlier versions. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 11 --- 1 file changed, 11 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 67dc85d0..f65eacc8 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -831,17 +831,6 @@ class IOMemRegionTree: self.parent = None self.children = [] -def __str__(self): -s = '' -if (self.region): -s = (' ' * (self.level - 1)) + str(self.region) -if self.parent and self.parent.region: -s += ' --> ' + self.parent.region.typestr -s += '\n' -for c in self.children: -s += str(c) -return s - def regions_split_by_kernel(self): kernel = [x for x in self.children if x.region.typestr.startswith('Kernel ')] -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-4-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 02/14] tools: jailhoues-cell-linux: cosmetic fixup
From: Ralf Ramsauer Just for the sake of consistency: s/memregion/mem_region/. This is the only spot where we still had memregion instead of mem_region. Signed-off-by: Ralf Ramsauer --- tools/jailhouse-cell-linux | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/jailhouse-cell-linux b/tools/jailhouse-cell-linux index e43f8e42..007a5c46 100755 --- a/tools/jailhouse-cell-linux +++ b/tools/jailhouse-cell-linux @@ -603,15 +603,15 @@ class Config: exit(1) self.name = str(name.decode()) -memregion_offs = struct.calcsize(Config._HEADER_FORMAT) + \ +mem_region_offs = struct.calcsize(Config._HEADER_FORMAT) + \ self.cpu_set_size self.memory_regions = [] for n in range(self.num_memory_regions): self.memory_regions.append( -MemoryRegion(self.data[memregion_offs:])) -memregion_offs += MemoryRegion.SIZE +MemoryRegion(self.data[mem_region_offs:])) +mem_region_offs += MemoryRegion.SIZE -irqchip_offs = memregion_offs + \ +irqchip_offs = mem_region_offs + \ self.num_cache_regions * CacheRegion.SIZE self.irqchips = [] for n in range(self.num_irqchips): -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-3-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 06/14] pyjailhouse: sysfs_parser: introduce new class IORegionTree
From: Ralf Ramsauer Do this step by step. First of all, let's create a new routine that is able to parse a line from /proc/iomem or /proc/ioports. Both files share the same layout, so we can use a common parser. Passing the destination type of the entry to the parser allows to share code. Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index b0a9bf44..a179461d 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -824,6 +824,17 @@ class IOAPIC: return (self.iommu << 16) | self.bdf +class IORegionTree: +@staticmethod +def parse_io_line(line, TargetClass): +region, type = line.split(' : ', 1) +level = int(region.count(' ') / 2) + 1 +type = type.strip() +region = [r.strip() for r in region.split('-', 1)] + +return level, TargetClass(int(region[0], 16), int(region[1], 16), type) + + class IOMemRegionTree: def __init__(self, region, level): self.region = region @@ -864,14 +875,6 @@ class IOMemRegionTree: 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(): root = IOMemRegionTree(None, 0) @@ -879,7 +882,7 @@ class IOMemRegionTree: lastlevel = 0 lastnode = root for line in f: -(level, r) = IOMemRegionTree.parse_iomem_line(line) +level, r = IORegionTree.parse_io_line(line, MemRegion) t = IOMemRegionTree(r, level) if (t.level > lastlevel): t.parent = lastnode -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-7-andrej.utz%40st.oth-regensburg.de.
[PATCH v3 01/14] tools: jailhouse-config-create: Rename regions to mem_regions in preparation for port_regions
Signed-off-by: Andrej Utz Signed-off-by: Ralf Ramsauer --- tools/jailhouse-config-create | 10 +- tools/root-cell-config.c.tmpl | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/jailhouse-config-create b/tools/jailhouse-config-create index 55601a6d..cfa7fbad 100755 --- a/tools/jailhouse-config-create +++ b/tools/jailhouse-config-create @@ -269,7 +269,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 +283,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,7 +298,7 @@ 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() @@ -311,7 +311,7 @@ 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, '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 f9805dcd..b6522ce1 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)}]; struct jailhouse_pio pio_regions[6]; struct jailhouse_pci_device pci_devices[${len(pcidevices)}]; @@ -126,7 +126,7 @@ struct { }, .mem_regions = { - % for r in regions: + % for r in mem_regions: /* ${str(r)} */ % for c in r.comments: /* ${c} */ -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930191323.32266-2-andrej.utz%40st.oth-regensburg.de.
Re: [PATCH] pyjailhouse: Remove superfluous definition and fix linter warnings
Hi Jan, On 30.09.19 18:37, Jan Kiszka wrote: On 30.09.19 16:52, Andrej Utz wrote: 'vector_size' was set, but not used. No functional change. Fixes: f6fef92ffaba ("pyjailhouse: sysfs_parser: Add more precise length of some extended caps") Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 67dc85d0..e6b07716 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -490,7 +490,7 @@ def parse_ivrs(pcidevices, ioapics): regions.append( MemRegion(mem_addr, mem_addr + mem_len - 1, 'ACPI IVRS', - comment)) + comment)) elif type == 0x40: raise RuntimeError( @@ -665,13 +665,13 @@ class PCICapability: len = 4 + (vsec_len >> 20) elif id == PCI_EXT_CAP_ID.ACS: len = 8 - vector_size = 0 - (acs_cap, acs_ctrl) = struct.unpack('f.read(4)) + acs_cap, acs_ctrl = struct.unpack(' Was that a warning as well, or just style preference? We have plenty of those cases, though. This didn't raise a warning, but I removed the braces as a cosmetic drive-by fix. Besides that we have more cosmetic changes like that, which we will send soon. Thanks, Andrej Utz if acs_cap & (1 << 5) and acs_ctrl & (1 << 5): vector_bits = acs_cap >> 8 if vector_bits == 0: vector_bits = 256 + vector_bytes = int((vector_bits + 31) / (8 * 4)) len += vector_bytes elif id in [PCI_EXT_CAP_ID.VC, PCI_EXT_CAP_ID.VC9]: @@ -679,7 +679,8 @@ class PCICapability: len = 4 * 4 elif id == PCI_EXT_CAP_ID.MFVC: len = 4 - elif id in [PCI_EXT_CAP_ID.LTR, PCI_EXT_CAP_ID.ARI, PCI_EXT_CAP_ID.PASID]: + elif id in [PCI_EXT_CAP_ID.LTR, PCI_EXT_CAP_ID.ARI, + PCI_EXT_CAP_ID.PASID]: len = 8 elif id in [PCI_EXT_CAP_ID.DSN, PCI_EXT_CAP_ID.PTM]: len = 12 Rest looks good. Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/49c87e4d-7600-4180-5f2f-b7525385237c%40st.oth-regensburg.de.
[PATCH] pyjailhouse: Remove superfluous definition and fix linter warnings
'vector_size' was set, but not used. No functional change. Fixes: f6fef92ffaba ("pyjailhouse: sysfs_parser: Add more precise length of some extended caps") Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 67dc85d0..e6b07716 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -490,7 +490,7 @@ def parse_ivrs(pcidevices, ioapics): regions.append( MemRegion(mem_addr, mem_addr + mem_len - 1, 'ACPI IVRS', -comment)) + comment)) elif type == 0x40: raise RuntimeError( @@ -665,13 +665,13 @@ class PCICapability: len = 4 + (vsec_len >> 20) elif id == PCI_EXT_CAP_ID.ACS: len = 8 -vector_size = 0 -(acs_cap, acs_ctrl) = struct.unpack('> 8 if vector_bits == 0: vector_bits = 256 + vector_bytes = int((vector_bits + 31) / (8 * 4)) len += vector_bytes elif id in [PCI_EXT_CAP_ID.VC, PCI_EXT_CAP_ID.VC9]: @@ -679,7 +679,8 @@ class PCICapability: len = 4 * 4 elif id == PCI_EXT_CAP_ID.MFVC: len = 4 -elif id in [PCI_EXT_CAP_ID.LTR, PCI_EXT_CAP_ID.ARI, PCI_EXT_CAP_ID.PASID]: +elif id in [PCI_EXT_CAP_ID.LTR, PCI_EXT_CAP_ID.ARI, +PCI_EXT_CAP_ID.PASID]: len = 8 elif id in [PCI_EXT_CAP_ID.DSN, PCI_EXT_CAP_ID.PTM]: len = 12 -- 2.23.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190930145239.16506-1-andrej.utz%40st.oth-regensburg.de.
Re: [PATCH v2 2/2] pyjailhouse: x86: implement pio_bitmap generator
Hi all, On 12.07.19 17:59, Ralf Ramsauer wrote: 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: [ 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 */ Now we have a hole between [0x90/8 ... 0x1f/8]. A hole means that this area will be initialised with zero -> access is permitted. Ack, this is not intended. 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 Unless there is some bit trickery optimization, I would also prefer "0 = disallowed". Also including Hennings concerns: >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. I will reconsider my approach how to generate ioports list. Parsing PCI config space seems like a better source instead of /proc/ioports. Depending how secretive PCI devices are with their I/O specs, refactoring MemRegion generation may also be necessary to use the same logic. Need to investigate further. But for now thanks for reviewing. Consider this patch queue retracted. Andrej Utz -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/011d894f-8cc0-1603-0c72-df3aedff943c%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] pyjailhouse: sysfs_parser: Ignore empty PCI bus regions
On 02.07.19 16:12, Jan Kiszka wrote: On 02.07.19 15:58, Andrej Utz wrote: On some systems the config generator permissively maps huge chunks of PCI Bus MMIO space. This area needs to be intercepted by the hypervisor, as parts of ivshmem-net devices may be behind that area. Just the make the boundary conditions clearer: This mapping only happens when the PCI bus memory region has no device resources so far. If ivshmem then adds a region later on, we are doomed. But if there is device already, nothing was broken so far. Am I right? Correct. Hence, ignore such regions. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 4 1 file changed, 4 insertions(+) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index f48f249f..4a06ccdc 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -105,6 +105,10 @@ def parse_iomem(pcidevices): ret = [] for r in regions: + # filter empty PCI buses "Filter PCI buses in order to avoid mapping empty ones that might require interception when becoming non-empty." Or so. Certainly an improvement. + if r.typestr.startswith('PCI Bus'): + continue + append_r = True # filter the list for MSI-X pages for d in pcidevices: Looks good. Jan Andrej -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/caa1784e-d110-ffcc-6fd0-d0aadd71ea04%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
[PATCH] pyjailhouse: sysfs_parser: Ignore empty PCI bus regions
On some systems the config generator permissively maps huge chunks of PCI Bus MMIO space. This area needs to be intercepted by the hypervisor, as parts of ivshmem-net devices may be behind that area. Hence, ignore such regions. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 4 1 file changed, 4 insertions(+) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index f48f249f..4a06ccdc 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -105,6 +105,10 @@ def parse_iomem(pcidevices): ret = [] for r in regions: +# filter empty PCI buses +if r.typestr.startswith('PCI Bus'): +continue + append_r = True # filter the list for MSI-X pages for d in pcidevices: -- 2.22.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190702135846.20045-1-andrej.utz%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH v2 2/2] pyjailhouse: x86: implement pio_bitmap generator
Hello Henning, On 28/06/2019 10:29, Henning Schild wrote: > 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. Thanks for the hint, I didn't really search for previous attempts. I see now that I forgot the PEP8 formatting, will note for v3. > 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. That was my goal: old static port list and new generated ones must be functionally the same. Access control for rest of devices is in the hands of user, as it was before, only more handier now. As for device detection: work is still underway. Andrej > > Henning > -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/acbe4ed8-5e5e-dc59-abfd-b2692a0ec6f8%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout. pEpkey.asc Description: application/pgp-keys
[PATCH v2 1/2] pyjailhouse: sysfs_parser: generalize IOMemRegion as IOMap
This allows us to use the 'iomem' file format parsing more flexible i.e. for ioports. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 3db61980..d612c6d3 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -97,8 +97,8 @@ def input_listdir(dir, wildcards): def parse_iomem(pcidevices): -(regions, dmar_regions) = IOMemRegionTree.parse_iomem_tree( -IOMemRegionTree.parse_iomem_file()) +(regions, dmar_regions) = IOMapTree.parse_iomem_tree( +IOMapTree.parse_iomap_file('/proc/iomem', MemRegion)) rom_region = MemRegion(0xc, 0xd, 'ROMs') add_rom_region = False @@ -788,7 +788,7 @@ class IOAPIC: return (self.iommu << 16) | self.bdf -class IOMemRegionTree: +class IOMapTree: def __init__(self, region, level): self.region = region self.level = level @@ -840,22 +840,22 @@ class IOMemRegionTree: return [before_kernel, kernel_region, after_kernel] @staticmethod -def parse_iomem_line(line): +def parse_iomap_line(line, io_type): 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, io_type(int(region[0], 16), int(region[1], 16), a[1]) @staticmethod -def parse_iomem_file(): -root = IOMemRegionTree(None, 0) -f = input_open('/proc/iomem') +def parse_iomap_file(file_path, io_type): +root = IOMapTree(None, 0) +f = input_open(file_path) lastlevel = 0 lastnode = root for line in f: -(level, r) = IOMemRegionTree.parse_iomem_line(line) -t = IOMemRegionTree(r, level) +(level, r) = IOMapTree.parse_iomap_line(line, io_type) +t = IOMapTree(r, level) if (t.level > lastlevel): t.parent = lastnode if (t.level == lastlevel): @@ -887,8 +887,7 @@ class IOMemRegionTree: # if the tree continues recurse further down ... if (len(tree.children) > 0): -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, name)) +regions.extend(IOMapTree.find_regions_by_name(tree, name)) return regions @@ -918,16 +917,15 @@ class IOMemRegionTree: # generally blacklisted, with a few exceptions if (s.lower() == 'reserved'): -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'HPET')) +regions.extend(IOMapTree.find_regions_by_name(tree, 'HPET')) dmar_regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'dmar')) +IOMapTree.find_regions_by_name(tree, 'dmar')) continue # if the tree continues recurse further down ... if (len(tree.children) > 0): (temp_regions, temp_dmar_regions) = \ -IOMemRegionTree.parse_iomem_tree(tree) +IOMapTree.parse_iomem_tree(tree) regions.extend(temp_regions) dmar_regions.extend(temp_dmar_regions) continue -- 2.22.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190620220614.23450-1-andrej.utz%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
[PATCH v2 2/2] pyjailhouse: x86: implement pio_bitmap generator
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 --- 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, 0x, '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: +
Re: [PATCH 2/2] pyjailhouse: x86: implement pio_bitmap generator
On 18.06.19 18:04, Jan Kiszka wrote: On 18.06.19 17:55, Andrej Utz wrote: Hi Jan, On 07.06.19 09:23, Jan Kiszka wrote: On 05.06.19 18:17, 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. IOW, the whole PCI IO space remains accessible, is now just partitioned in order to ease manual disabling? I wonder if we could not go one step further and only allow known regions. But isn't this the same as the static regions ... Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 135 ++ tools/jailhouse-config-create | 14 +--- tools/root-cell-config.c.tmpl | 15 ++-- 3 files changed, 142 insertions(+), 22 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 56265fb5..d06a476a 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -142,6 +142,57 @@ 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), # NMI status/control ... do here? Or how do you define "known regions"? There are a number of known platform regions in the lower IO range, like the above. And then there are the IO regions of PCI devices, according to their BAR settings. Currently, we permit access to the whole PCI range to the root cell. Does that also mean we need to read the PCI config space to whitelist port regions? If so, I'd like it to be another commit on top of this patch. Andrej Jan -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/0b53c6d6-1810-fe2f-f077-20fe8272ff36%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH 2/2] pyjailhouse: x86: implement pio_bitmap generator
Hi Jan, On 07.06.19 09:23, Jan Kiszka wrote: On 05.06.19 18:17, 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. IOW, the whole PCI IO space remains accessible, is now just partitioned in order to ease manual disabling? I wonder if we could not go one step further and only allow known regions. But isn't this the same as the static regions ... Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 135 ++ tools/jailhouse-config-create | 14 +--- tools/root-cell-config.c.tmpl | 15 ++-- 3 files changed, 142 insertions(+), 22 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 56265fb5..d06a476a 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -142,6 +142,57 @@ 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), # NMI status/control ... do here? Or how do you define "known regions"? We should preserve the property "hack" for this one, and that ideally as comment in the generated config. Could you enhance this in that direction? Benefit: you could add the originating device as comment to other port ranges. Ack. Andrej -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/8d1f8f9c-8cdd-e4df-acd3-cecff65a2612%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
[PATCH 1/2] pyjailhouse: sysfs_parser: generalize IOMemRegion as IOMap
This refactor allows us amore flexible use of the 'iomem' file format parsing i.e. for ioports in the following commit. No functional change. Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index fa32ba48..56265fb5 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -97,8 +97,8 @@ def input_listdir(dir, wildcards): def parse_iomem(pcidevices): -(regions, dmar_regions) = IOMemRegionTree.parse_iomem_tree( -IOMemRegionTree.parse_iomem_file()) +(regions, dmar_regions) = IOMapTree.parse_iomem_tree( +IOMapTree.parse_iomap_file('/proc/iomem', MemRegion)) rom_region = MemRegion(0xc, 0xd, 'ROMs') add_rom_region = False @@ -785,7 +785,7 @@ class IOAPIC: return (self.iommu << 16) | self.bdf -class IOMemRegionTree: +class IOMapTree: def __init__(self, region, level): self.region = region self.level = level @@ -837,22 +837,22 @@ class IOMemRegionTree: return [before_kernel, kernel_region, after_kernel] @staticmethod -def parse_iomem_line(line): +def parse_iomap_line(line, io_type): 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, io_type(int(region[0], 16), int(region[1], 16), a[1]) @staticmethod -def parse_iomem_file(): -root = IOMemRegionTree(None, 0) -f = input_open('/proc/iomem') +def parse_iomap_file(file_path, io_type): +root = IOMapTree(None, 0) +f = input_open(file_path) lastlevel = 0 lastnode = root for line in f: -(level, r) = IOMemRegionTree.parse_iomem_line(line) -t = IOMemRegionTree(r, level) +(level, r) = IOMapTree.parse_iomap_line(line, io_type) +t = IOMapTree(r, level) if (t.level > lastlevel): t.parent = lastnode if (t.level == lastlevel): @@ -884,8 +884,7 @@ class IOMemRegionTree: # if the tree continues recurse further down ... if (len(tree.children) > 0): -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, name)) +regions.extend(IOMapTree.find_regions_by_name(tree, name)) return regions @@ -915,16 +914,15 @@ class IOMemRegionTree: # generally blacklisted, with a few exceptions if (s.lower() == 'reserved'): -regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'HPET')) +regions.extend(IOMapTree.find_regions_by_name(tree, 'HPET')) dmar_regions.extend( -IOMemRegionTree.find_regions_by_name(tree, 'dmar')) +IOMapTree.find_regions_by_name(tree, 'dmar')) continue # if the tree continues recurse further down ... if (len(tree.children) > 0): (temp_regions, temp_dmar_regions) = \ -IOMemRegionTree.parse_iomem_tree(tree) +IOMapTree.parse_iomem_tree(tree) regions.extend(temp_regions) dmar_regions.extend(temp_dmar_regions) continue -- 2.21.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190605161745.2389-1-andrej.utz%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
[PATCH 2/2] pyjailhouse: x86: implement pio_bitmap generator
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 --- pyjailhouse/sysfs_parser.py | 135 ++ tools/jailhouse-config-create | 14 +--- tools/root-cell-config.c.tmpl | 15 ++-- 3 files changed, 142 insertions(+), 22 deletions(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 56265fb5..d06a476a 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -142,6 +142,57 @@ 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), # NMI status/control +PortRegion(0x64, 0x64, 'NMI', allowed=True), # ditto +PortRegion(0x70, 0x71, 'RTC', allowed=True), +PortRegion(0x3b0, 0x3df, 'VGA', allowed=True), +PortRegion(0xd00, 0x, 'PCI bus', allowed=True), +] + +pm_timer_base = None +for r in regions: +if r.typestr == 'ACPI PM_TMR': +pm_timer_base = r.start + +if r.typestr.startswith('ACPI'): +r.allowed = True + +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 devices = [] @@ -769,6 +820,65 @@ 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, chained=False): +# as in MemRegion this method is purely for C comment generation + +# deviceless region +if self.typestr == '': +return '' + +s = '' +if chained == False: +s += 'PortRegion: %04x-%04x : ' % (self.start, self.stop) + +s += self.typestr +if self.neighbor: +s += ' +' + self.neighbor.__str__(True) + +return s + +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 = allowed, +# outside: 0 = disallowed +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 + +def bits_str(self): +b = self.bits() +if b == 0: +return '0' +elif b == 0xff: +return '-1' +else: +return hex(b) + + class IOAPIC: def __init__(self, id, address, gsi_base, iommu=0, bdf=0): self.id = id @@ -932,6 +1042,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
[PATCH] pyjailhouse: sysfs_parser: fix IVDM memory region definition
Second parameter to MemRegion must be its end (inclusive). Fixes: 5fe206927c05 ("tools: Implement ACPI IVRS table parser") Signed-off-by: Andrej Utz --- pyjailhouse/sysfs_parser.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index fa32ba48..83222c4a 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -481,7 +481,10 @@ 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( +MemRegion(mem_addr, mem_addr + mem_len - 1, 'ACPI IVRS', +comment)) + elif type == 0x40: raise RuntimeError( 'You board uses IVRS Rev. 2 feature Jailhouse doesn\'t ' -- 2.21.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190605153900.32177-1-andrej.utz%40st.oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
Re: Config generator ignoring usefull iomem regions
On 04/04/2019 19:19, Jan Kiszka wrote: > On 04.04.19 19:02, Andrej Utz wrote: >> Hello everyone, >> >> after recompiling the kernel (minor changes), I noticed a big >> reduction of mmio regions after regenerating the root cell config with >> critical hardware like AHCI within. Turned out a big chunk of iomem >> regions went a level deeper in the tree with "Reserved" as its parent >> node (see appendix for more). I'm not sure yet, but I think enabling >> AMD IOMMU v2 resulted in this. > > You should see a different result then when disabling it on the command > line (amd_iommu=off). Sorry for not mentioning, but amd_iommu is always "off". >> From reading sysfs_parser.py:905 I gathered that "Reserved" region >> nodes are, with exception of HPET, completely ignored. In my case the >> whole PCI bus goes missing so I'd like to know the rationale behind this. > > Well, the kernel used to report regions as "reserved" that were actually > reserved to it during runtime, thus won't be touched anymore. > Apparently, this definition changed. It would be good to under stand a > pattern, but the simplest workaround for now would be either including > reserved regions again (though we need to see if this doesn't overshoot) > or making this at least configurable (--with-reserved-regions or so). After booting the mainline kernel and still having that reserved region in iomem I'm pretty sure the BIOS update I did earlier is responsible. Somewhat strange is that this kind of region is reserved on other systems too (delayed change from 'usable' by e820 in early boot), but Linux doesn't show it in iomem there. Keeping that in mind the "--with-reserved-regions" option fits perfectly for such shenanigans. I will test a bit more over weekend and probably write a patch for it. Thanks, Andrej Utz -- 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 jailhouse-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. pEpkey.asc Description: application/pgp-keys
Config generator ignoring usefull iomem regions
Hello everyone, after recompiling the kernel (minor changes), I noticed a big reduction of mmio regions after regenerating the root cell config with critical hardware like AHCI within. Turned out a big chunk of iomem regions went a level deeper in the tree with "Reserved" as its parent node (see appendix for more). I'm not sure yet, but I think enabling AMD IOMMU v2 resulted in this. From reading sysfs_parser.py:905 I gathered that "Reserved" region nodes are, with exception of HPET, completely ignored. In my case the whole PCI bus goes missing so I'd like to know the rationale behind this. Thanks, Andrej Utz -- 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 jailhouse-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. -0fff : Reserved 1000-0009 : System RAM 000a-000f : Reserved 000a-000b : PCI Bus :00 000c-000d : PCI Bus :00 000f-000f : System ROM 0010-09df : System RAM 0200-02c031d0 : Kernel code 02c031d1-033456ff : Kernel data 03723000-039f : Kernel bss 09e0-09ff : Reserved 0a00-0a1f : System RAM 0a20-0a20afff : ACPI Non-volatile Storage 0a20b000-0aff : System RAM 0b00-0b01 : Reserved 0b02-39ff : System RAM 3a00-3f1f : Reserved 3f20-d8250017 : System RAM d8250018-d826e657 : System RAM d826e658-d83aa017 : System RAM d83aa018-d83b8057 : System RAM d83b8058-dba1afff : System RAM dba1b000-dbb96fff : Reserved dbb97000-dc022fff : System RAM dc023000-dc138fff : ACPI Non-volatile Storage dc139000-dcd7dfff : Reserved dcd7e000-deff : System RAM df00-dfff : Reserved e000- : Reserved e000-fec2 : PCI Bus :00 e000-efff : PCI Bus :06 e000-efff : :06:00.0 e000-e02f : BOOTFB f000-f00f : PCI Bus :01 f000-f00f : PCI Bus :02 f000-f00f : PCI Bus :04 f000-f0007fff : :04:00.0 f800-fbff : PCI MMCONFIG [bus 00-3f] f800-fbff : Reserved f800-fbff : pnp 00:00 fc80-fcaf : PCI Bus :07 fc80-fc8f : :07:00.3 fc80-fc8f : xhci-hcd fc90-fc9f : :07:00.2 fc90-fc9f : ccp fca0-fca01fff : :07:00.2 fca0-fca01fff : ccp fcb0-fcdf : PCI Bus :01 fcb0-fccf : PCI Bus :02 fcb0-fcbf : PCI Bus :04 fcb0-fcb07fff : :04:00.0 fcc0-fccf : PCI Bus :03 fcc0-fcc03fff : :03:00.0 fcc04000-fcc04fff : :03:00.0 fcc04000-fcc04fff : r8169 fcd0-fcd7 : :01:00.1 fcd8-fcd9 : :01:00.1 fcd8-fcd9 : ahci fcda-fcda7fff : :01:00.0 fcda-fcda7fff : xhci-hcd fce0-fcef : PCI Bus :08 fce0-fce07fff : :08:00.3 fce08000-fce08fff : :08:00.2 fce08000-fce08fff : ahci fcf0-fcff : PCI Bus :06 fcf0-fcf1 : :06:00.0 fcf2-fcf3 : :06:00.0 fcf4-fcf43fff : :06:00.1 fec0-fec003ff : IOAPIC 0 fec01000-fec013ff : IOAPIC 1 fec1-fec10fff : pnp 00:06 fec3-fec30fff : AMDIF030:00 fed0-fed003ff : HPET 0 fed0-fed003ff : PNP0103:00 fed81500-fed818ff : AMDI0030:00 fedc-fedc0fff : pnp 00:06 fee0- : PCI Bus :00 fee0-fee00fff : Local APIC fee0-fee00fff : pnp 00:06 ff00- : pnp 00:06 1-41f37 : System RAM 41f38-41fff : RAM buffer
[PATCH] pyjailhouse: sysfs_parser: Fix wrong IVRS device ID for IOAPIC
If the type of an IVHD device entry is a Special Device (0x48), then device ID is stored at a different location. The code already parsed the value to device_id_b, but it was never used. This set the ID of all IOAPICs to 0x0 in the system configuration. Fixes: 5fe206927c05 ("tools: Implement ACPI IVRS table parses") Signed-off-by: Andrej Utz Signed-off-by: Ralf Ramsauer --- pyjailhouse/sysfs_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyjailhouse/sysfs_parser.py b/pyjailhouse/sysfs_parser.py index 552c9462..46c95fc2 100644 --- a/pyjailhouse/sysfs_parser.py +++ b/pyjailhouse/sysfs_parser.py @@ -441,7 +441,7 @@ def parse_ivrs(pcidevices, ioapics): if variety == 0x01: # IOAPIC for chip in ioapics: if chip.id == handle: -chip.bdf = device_id +chip.bdf = device_id_b chip.iommu = len(units) - 1 else: # Reserved or ignored entries -- 2.21.0 -- 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 jailhouse-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Running on AMD Ryzen
Greetings Jailhouse developers, I am trying to run Jailhouse on AMD Ryzen 2700X (x86_64) with B450 chipset and got into some problems. After whitelisting some I/O ports and putting "amd_iommu=off mce=off" I managed to enable Jailhouse, but instantly lost some USB ports (keyboard being one of them). After some retries I noticed this happens only 80 % of the time and it seems that some interrupts are never acknowledged and keep blocking the USB hub. This probably also happens after creating and starting tiny-demo.cell: network on Ethernet becomes unresponsive. The IP stack still works, but receives no packets whatsoever. Coincidentally Ethernet interrupts are handled by CPU2 on which tiny-demo.cell starts. This buggy behaviour can be mitigated by setting Ethernet IRQ affinities to another CPU before cell start. Also destroying said cell doesn't bring CPU2 back online. Linux version: 4.19.15-rt11-1-rt-lts-jailhouse+ SMP PREEMPT RT (from github/siemens/linux) HyperThreading: disabled in BIOS Thanks, Andrej Utz -- 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 jailhouse-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. config.gz Description: application/gzip /* * Jailhouse, a Linux-based partitioning hypervisor * * Copyright (c) Siemens AG, 2014-2017 * * This work is licensed under the terms of the GNU GPL, version 2. See * the COPYING file in the top-level directory. * * Alternatively, you can use or redistribute this file under the following * BSD license: * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * * 1. Redistributions of source code must retain the above copyright *notice, this list of conditions and the following disclaimer. * * 2. Redistributions in binary form must reproduce the above copyright *notice, this list of conditions and the following disclaimer in the *documentation and/or other materials provided with the distribution. * * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF * THE POSSIBILITY OF SUCH DAMAGE. * * Configuration for Gigabyte Technology Co., Ltd. B450 AORUS M * created with '/usr/local/libexec/jailhouse/jailhouse config create ryzen.c' * * NOTE: This config expects the following to be appended to your kernel cmdline * "memmap=0x520$0x3a00" */ #include #include #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) struct { struct jailhouse_system header; __u64 cpus[1]; struct jailhouse_memory mem_regions[39]; struct jailhouse_irqchip irqchips[2]; __u8 pio_bitmap[0x2000]; struct jailhouse_pci_device pci_devices[37]; struct jailhouse_pci_capability pci_caps[95]; } __attribute__((packed)) config = { .header = { .signature = JAILHOUSE_SYSTEM_SIGNATURE, .revision = JAILHOUSE_CONFIG_REVISION, .flags = JAILHOUSE_SYS_VIRTUAL_DEBUG_CONSOLE, .hypervisor_memory = { .phys_start = 0x3a00, .size = 0x60, }, .debug_console = { .address = 0x3f8, .type = JAILHOUSE_CON_TYPE_8250, .flags = JAILHOUSE_CON_ACCESS_PIO | JAILHOUSE_CON_REGDIST_1, }, .platform_info = { .pci_mmconfig_base = 0xf800, .pci_mmconfig_end_bus = 0x3f, .x86 = { .pm_timer_address = 0x808, .vtd_interrupt_limit = 256, .iommu_units = { { .base = 0xfeb8, .size = 0x8, .amd_bdf = 0x2, .amd_base_cap = 0x40, .amd_msi_cap = 0x64, .amd_features = 0x80048f6f, }, }, }, }, .root_cell = { .name = "RootCell", .cpu_set_size = sizeof(config.cpus), .num_memory_regions = ARRAY_SIZE(config.mem_regions), .num_irqchips = ARRAY_SIZE(config.irqchips), .pio_bitmap_size = ARRAY_SIZE(config.pio_bitmap), .num_pci_devices = ARRAY_SIZE(config.pci_devices), .num_pci_caps = ARRAY_SIZE(config.pci_caps), }, }, .cpus = { 0x00ff, }, .mem_regions = { /* MemRegion: -0009 : System RAM */ { .phys_start = 0x0, .virt_start = 0x0, .size = 0xa,