On 13.07.19 20:10, Ralf Ramsauer wrote:
> Whitelist-based permissions align better with the Jailhouse philosophy.
>
> This patch switches the permission bitmap (which basically was (almost) a
> duplicate of the final io_bitmap) to a structure that whitelists a port range,
> given a base port and a length.
>
> As a side effect, this patch drastically reduces the size of config files:
> 8404 -> 224 apic-demo.cell
> 8488 -> 312 e1000-demo.cell
> 11450 -> 3290 f2a88xm-hd3.cell
> 11634 -> 3474 imb-a180.cell
> 8424 -> 252 ioapic-demo.cell
> 8480 -> 296 ivshmem-demo.cell
> 8788 -> 608 linux-x86-demo.cell
> 8488 -> 308 pci-demo.cell
> 9730 -> 1586 qemu-x86.cell
> 8392 -> 212 smp-demo.cell
> 8404 -> 224 tiny-demo.cell
>
> If no whitelist is given, all PIO access will be denied. Additionally,
> increase
> the config file revision header.
>
> Signed-off-by: Ralf Ramsauer <[email protected]>
> ---
> configs/x86/apic-demo.c | 16 +++----
> configs/x86/e1000-demo.c | 19 +++-----
> configs/x86/f2a88xm-hd3.c | 27 +++++------
> configs/x86/imb-a180.c | 27 +++++------
> configs/x86/ioapic-demo.c | 22 ++++-----
> configs/x86/ivshmem-demo.c | 13 ++----
> configs/x86/linux-x86-demo.c | 16 +++----
> configs/x86/pci-demo.c | 16 +++----
> configs/x86/qemu-x86.c | 41 ++++++----------
> configs/x86/smp-demo.c | 16 +++----
> configs/x86/tiny-demo.c | 16 +++----
> hypervisor/arch/x86/vcpu.c | 83 +++++++++++++++++++++------------
> include/jailhouse/cell-config.h | 17 +++----
> tools/jailhouse-hardware-check | 2 +-
> 14 files changed, 151 insertions(+), 180 deletions(-)
>
[...]
> diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
> index 9caba92f..3a660ef1 100644
> --- a/hypervisor/arch/x86/vcpu.c
> +++ b/hypervisor/arch/x86/vcpu.c
> @@ -26,6 +26,12 @@
> #include <jailhouse/percpu.h>
> #include <asm/vcpu.h>
>
> +#define for_each_pio_whitelist(pio, config, counter) \
> + for ((pio) = jailhouse_cell_pio_whitelist(config), \
> + (counter) = 0; \
> + (counter) < (config)->num_pio_whitelists; \
> + (pio)++, (counter)++)
> +
> static u8 __attribute__((aligned(PAGE_SIZE))) parking_code[PAGE_SIZE] = {
> 0xfa, /* 1: cli */
> 0xf4, /* hlt */
> @@ -83,15 +89,26 @@ static inline void vcpu_get_cell_io_bitmap(struct cell
> *cell,
> iobm->size = vcpu_vendor_get_io_bitmap_pages() * PAGE_SIZE;
> }
>
> +static void pio_allow_access(struct vcpu_io_bitmap *bm,
> + const struct jailhouse_pio_whitelist *pio,
> + bool access)
> +{
> + void (*access_method)(unsigned int, volatile unsigned long*) =
> + access ? clear_bit : set_bit;
> + unsigned int start_bit = pio->base;
> + unsigned int length = pio->length;
> +
> + for (; length; length--, start_bit++)
I would prefer "length > 0" here.
> + access_method(start_bit, (unsigned long*)bm->data);
> +}
> +
> int vcpu_cell_init(struct cell *cell)
> {
> const unsigned int io_bitmap_pages = vcpu_vendor_get_io_bitmap_pages();
> - const u8 *pio_bitmap = jailhouse_cell_pio_bitmap(cell->config);
> - u32 pio_bitmap_size = cell->config->pio_bitmap_size;
> + const struct jailhouse_pio_whitelist *pio_whitelist;
> struct vcpu_io_bitmap cell_iobm, root_cell_iobm;
> unsigned int n, pm_timer_addr;
> - u32 size;
> - int err;
> + int err, i;
You already have "n", can be recycled, and it has the better type.
> u8 *b;
>
> cell->arch.io_bitmap = page_alloc(&mem_pool, io_bitmap_pages);
> @@ -109,19 +126,20 @@ int vcpu_cell_init(struct cell *cell)
> /* initialize io bitmap to trap all accesses */
> memset(cell_iobm.data, -1, cell_iobm.size);
>
> - /* copy io bitmap from cell config */
> - size = pio_bitmap_size > cell_iobm.size ?
> - cell_iobm.size : pio_bitmap_size;
> - memcpy(cell_iobm.data, pio_bitmap, size);
> + /* cells have no access to i8042, unless the port is whitelisted */
> + cell->arch.pio_i8042_allowed = false;
>
> - /* always intercept access to i8042 command register */
> - cell_iobm.data[I8042_CMD_REG / 8] |= 1 << (I8042_CMD_REG % 8);
> + for_each_pio_whitelist(pio_whitelist, cell->config, i) {
> + pio_allow_access(&cell_iobm, pio_whitelist, true);
>
> - /* but moderate only if the config allows i8042 access */
> - cell->arch.pio_i8042_allowed =
> - pio_bitmap_size >= (I8042_CMD_REG + 7) / 8 ?
> - !(pio_bitmap[I8042_CMD_REG / 8] & (1 << (I8042_CMD_REG % 8))) :
> - false;
> + /* moderate i8042 only if the config allows it */
> + if (pio_whitelist->base <= I8042_CMD_REG &&
> + pio_whitelist->base + pio_whitelist->length > I8042_CMD_REG)
> + cell->arch.pio_i8042_allowed = true;
> + }
> +
> + /* but always intercept access to i8042 command register */
> + cell_iobm.data[I8042_CMD_REG / 8] |= 1 << (I8042_CMD_REG % 8);
>
> if (cell != &root_cell) {
> /*
> @@ -129,9 +147,8 @@ int vcpu_cell_init(struct cell *cell)
> * access rights.
> */
> vcpu_get_cell_io_bitmap(&root_cell, &root_cell_iobm);
> - for (b = root_cell_iobm.data; pio_bitmap_size > 0;
> - b++, pio_bitmap++, pio_bitmap_size--)
> - *b |= ~*pio_bitmap;
> + for_each_pio_whitelist(pio_whitelist, cell->config, i)
> + pio_allow_access(&root_cell_iobm, pio_whitelist, false);
> }
>
> /* permit access to the PM timer if there is any */
> @@ -148,21 +165,29 @@ int vcpu_cell_init(struct cell *cell)
>
> void vcpu_cell_exit(struct cell *cell)
> {
> - const u8 *root_pio_bitmap =
> - jailhouse_cell_pio_bitmap(root_cell.config);
> - const u8 *pio_bitmap = jailhouse_cell_pio_bitmap(cell->config);
> - u32 pio_bitmap_size = cell->config->pio_bitmap_size;
> + const struct jailhouse_pio_whitelist *cell_wl, *root_wl;
> + struct jailhouse_pio_whitelist refund;
> struct vcpu_io_bitmap root_cell_iobm;
> - u8 *b;
> + unsigned int interval_start, interval_end;
> + unsigned int i, j;
"i" suggests signed integer. We usually use "n" for unsigned int counter. j may
be m or, if that's too close, k.
Also, the unsigned int vars can go into a single line.
>
> vcpu_get_cell_io_bitmap(&root_cell, &root_cell_iobm);
>
> - if (root_cell.config->pio_bitmap_size < pio_bitmap_size)
> - pio_bitmap_size = root_cell.config->pio_bitmap_size;
> -
> - for (b = root_cell_iobm.data; pio_bitmap_size > 0;
> - b++, pio_bitmap++, root_pio_bitmap++, pio_bitmap_size--)
> - *b &= *pio_bitmap | *root_pio_bitmap;
> + /* Hand back ports to the root cell. But only hand back those ports
> + * that overlap with the root cell's config. This is done by pairwise
> + * comparison of the cell's and the root cell's whitelist entries. */
> + for_each_pio_whitelist(cell_wl, cell->config, i)
> + for_each_pio_whitelist(root_wl, root_cell.config, j) {
> + interval_start = MAX(cell_wl->base, root_wl->base);
> + interval_end = MIN(cell_wl->base + cell_wl->length,
> + root_wl->base + root_wl->length);
> + if (interval_start < interval_end) {
> + refund.base = interval_start;
> + refund.length = interval_end - interval_start;
> + pio_allow_access(&root_cell_iobm, &refund,
> + true);
> + }
> + }
>
> page_free(&mem_pool, cell->arch.io_bitmap,
> vcpu_vendor_get_io_bitmap_pages());
> diff --git a/include/jailhouse/cell-config.h b/include/jailhouse/cell-config.h
> index a57fb9ef..b20a2b09 100644
> --- a/include/jailhouse/cell-config.h
> +++ b/include/jailhouse/cell-config.h
> @@ -50,7 +50,7 @@
> * Incremented on any layout or semantic change of system or cell config.
> * Also update HEADER_REVISION in tools.
> */
> -#define JAILHOUSE_CONFIG_REVISION 10
> +#define JAILHOUSE_CONFIG_REVISION 11
>
> #define JAILHOUSE_CELL_NAME_MAXLEN 31
>
> @@ -92,7 +92,7 @@ struct jailhouse_cell_desc {
> __u32 num_memory_regions;
> __u32 num_cache_regions;
> __u32 num_irqchips;
> - __u32 pio_bitmap_size;
> + __u32 num_pio_whitelists;
Naming again: we should talk about "pio_regions" here, analogously to the other
regions.
> __u32 num_pci_devices;
> __u32 num_pci_caps;
>
> @@ -277,7 +277,7 @@ jailhouse_cell_config_size(struct jailhouse_cell_desc
> *cell)
> cell->num_memory_regions * sizeof(struct jailhouse_memory) +
> cell->num_cache_regions * sizeof(struct jailhouse_cache) +
> cell->num_irqchips * sizeof(struct jailhouse_irqchip) +
> - cell->pio_bitmap_size +
> + cell->num_pio_whitelists * sizeof(struct
> jailhouse_pio_whitelist) +
> cell->num_pci_devices * sizeof(struct jailhouse_pci_device) +
> cell->num_pci_caps * sizeof(struct jailhouse_pci_capability);
> }
> @@ -319,10 +319,11 @@ jailhouse_cell_irqchips(const struct
> jailhouse_cell_desc *cell)
> cell->num_cache_regions * sizeof(struct jailhouse_cache));
> }
>
> -static inline const __u8 *
> -jailhouse_cell_pio_bitmap(const struct jailhouse_cell_desc *cell)
> +static inline const struct jailhouse_pio_whitelist *
> +jailhouse_cell_pio_whitelist(const struct jailhouse_cell_desc *cell)
> {
> - return (const __u8 *)((void *)jailhouse_cell_irqchips(cell) +
> + return (const struct jailhouse_pio_whitelist *)
> + ((void *)jailhouse_cell_irqchips(cell) +
> cell->num_irqchips * sizeof(struct jailhouse_irqchip));
> }
>
> @@ -330,8 +331,8 @@ static inline const struct jailhouse_pci_device *
> jailhouse_cell_pci_devices(const struct jailhouse_cell_desc *cell)
> {
> return (const struct jailhouse_pci_device *)
> - ((void *)jailhouse_cell_pio_bitmap(cell) +
> - cell->pio_bitmap_size);
> + ((void *)jailhouse_cell_pio_whitelist(cell) +
> + cell->num_pio_whitelists * sizeof(struct
> jailhouse_pio_whitelist));
> }
>
> static inline const struct jailhouse_pci_capability *
> diff --git a/tools/jailhouse-hardware-check b/tools/jailhouse-hardware-check
> index b86756ac..afd1139b 100755
> --- a/tools/jailhouse-hardware-check
> +++ b/tools/jailhouse-hardware-check
> @@ -136,7 +136,7 @@ class Sysconfig:
> X86_MAX_IOMMU_UNITS = 8
> X86_IOMMU_SIZE = 20
>
> - HEADER_REVISION = 10
> + HEADER_REVISION = 11
> HEADER_FORMAT = '6sH'
>
> def __init__(self, path):
>
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/e049667a-6d1e-cd7b-783b-0739191e227c%40siemens.com.
