> > > > > Uh, right, interrupts are not masked during those windows...
> > > > >
> > > > > What I am currently considering on this would be disabling
> > > > > IRQs while manipulating the register, and introducing a new flag
> > > > > to indicate whether the PMUSERENR for the guest EL0 is loaded,
> > > > > and having kvm_set_pmuserenr() check the new flag.
> > > > >
> > > > > The code would be something like below (local_irq_save/local_irq_restore
> > > > > needs to be excluded for NVHE though).
> > >
> > > It shouldn't need to be excluded. It should be fairly harmless, unless
> > > I'm missing something really obvious?
> >
> > The reason why I think local_irq_{save,restore} should be excluded
> > are because they use trace_hardirqs_{on,off} (Since IRQs are
> > masked here for NVHE, practically, they shouldn't be called with
> > the current KVM implementation though).
>
> Gah. Indeed, we end-up with a lot of unwanted crap, and absolutely no
> way to locally override it.
>
> > I'm looking at using "ifndef __KVM_NVHE_HYPERVISOR__" or other
> > ways to organize the code for this.
>
> I'd vote for something like the code below:
Thank you for the suggestion.
Considering that we may have similar cases in the future,
I will implement as you suggested in v3.
Thank you,
Reiji
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 530347cdebe3..1796fadb26cc 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -10,7 +10,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS
> # will explode instantly (Words of Marc Zyngier). So introduce a generic flag
> # __DISABLE_TRACE_MMIO__ to disable MMIO tracing for nVHE KVM.
> ccflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS -D__DISABLE_TRACE_MMIO__
> -ccflags-y += -fno-stack-protector \
> +ccflags-y += -fno-stack-protector -DNO_TRACE_IRQFLAGS \
> -DDISABLE_BRANCH_PROFILING \
> $(DISABLE_STACKLEAK_PLUGIN)
>
> diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
> index 5ec0fa71399e..ab0ae58dd797 100644
> --- a/include/linux/irqflags.h
> +++ b/include/linux/irqflags.h
> @@ -198,9 +198,10 @@ extern void warn_bogus_irq_restore(void);
>
> /*
> * The local_irq_*() APIs are equal to the raw_local_irq*()
> - * if !TRACE_IRQFLAGS.
> + * if !TRACE_IRQFLAGS or if NO_TRACE_IRQFLAGS is localy
> + * set.
> */
> -#ifdef CONFIG_TRACE_IRQFLAGS
> +#if defined(CONFIG_TRACE_IRQFLAGS) && !defined(NO_TRACE_IRQFLAGS)
>
> #define local_irq_enable() \
> do { \
>
>
> > Since {__activate,__deactivate}_traps_common() are pretty lightweight
> > functions, I'm also considering disabling IRQs in their call sites
> > (i.e. activate_traps_vhe_load/deactivate_traps_vhe_put), instead of in
> > __{de}activate_traps_common() (Thanks for this suggestion, Oliver).
>
> That would work too.
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
