On 23.03.21 07:25, [email protected] wrote: > From: Peng Fan <[email protected]> > > SGI_INJECT, SGI_EVENT, and maint interrupt are needed by Jailhouse, > should not allow cells to overwrite the settings, otherwise inmate > cells might not respond to root cell command 'jailhouse cell destroy [x]'. >
Uhh, this is a nasty whole... Let's rephrase a bit: "SGI_INJECT, SGI_EVENT, and maint interrupt are needed by Jailhouse and must not be controlled by the inmate. E.g., we allowed the inmate to disabled those interrupts, stalling Jailhouse on management operations." > Signed-off-by: Peng Fan <[email protected]> > --- > hypervisor/arch/arm-common/gic-v3.c | 13 ++++++++++--- > hypervisor/arch/arm-common/include/asm/control.h | 1 + > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/hypervisor/arch/arm-common/gic-v3.c > b/hypervisor/arch/arm-common/gic-v3.c > index 2293f844..8778f919 100644 > --- a/hypervisor/arch/arm-common/gic-v3.c > +++ b/hypervisor/arch/arm-common/gic-v3.c > @@ -348,6 +348,7 @@ static enum mmio_result gicv3_handle_redist_access(void > *arg, > struct mmio_access *mmio) > { > struct public_per_cpu *cpu_public = arg; > + unsigned int mnt_irq = system_config->platform_info.arm.maintenance_irq; > > switch (mmio->address) { > case GICR_TYPER: > @@ -368,15 +369,21 @@ static enum mmio_result gicv3_handle_redist_access(void > *arg, > case GICR_SYNCR: > mmio->value = 0; > return MMIO_HANDLED; > - case GICR_CTLR: > - case GICR_STATUSR: > - case GICR_WAKER: > case GICR_SGI_BASE + GICR_ISENABLER: > case GICR_SGI_BASE + GICR_ICENABLER: > case GICR_SGI_BASE + GICR_ISPENDR: > case GICR_SGI_BASE + GICR_ICPENDR: > case GICR_SGI_BASE + GICR_ISACTIVER: > case GICR_SGI_BASE + GICR_ICACTIVER: > + if (this_cell() != cpu_public->cell) { > + /* ignore access to foreign redistributors */ > + return MMIO_HANDLED; > + } > + mmio->value &= ~(SGI_MASK | (1 << mnt_irq)); > + break; If we fall through here, we can save the "this_cell() != cpu_public->cell" check and reuse the one below. > + case GICR_CTLR: > + case GICR_STATUSR: > + case GICR_WAKER: > case REG_RANGE(GICR_SGI_BASE + GICR_IPRIORITYR, 8, 4): > case REG_RANGE(GICR_SGI_BASE + GICR_ICFGR, 2, 4): > if (this_cell() != cpu_public->cell) { > diff --git a/hypervisor/arch/arm-common/include/asm/control.h > b/hypervisor/arch/arm-common/include/asm/control.h > index acebef32..e48269d1 100644 > --- a/hypervisor/arch/arm-common/include/asm/control.h > +++ b/hypervisor/arch/arm-common/include/asm/control.h > @@ -15,6 +15,7 @@ > > #define SGI_INJECT 0 > #define SGI_EVENT 1 > +#define SGI_MASK ((1 << SGI_EVENT) | (1 << SGI_INJECT)) > > #ifndef __ASSEMBLY__ > > Good catch! Hope there isn't more in the GICv3 path. v2 should have been reviewed better. Jan -- Siemens AG, T RDA IOT 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/8f80ddff-7131-2a43-3d53-e06860a181da%40siemens.com.
