Oh sorry, I didn't skim this one before replying to the first.
Excerpts from Fabiano Rosas's message of April 16, 2021 9:09 am:
> Since commit 73937deb4b2d ("KVM: PPC: Book3S HV: Sanitise hv_regs on
> nested guest entry") we have been disabling for the nested guest the
> hypervisor facility bits that its nested hypervisor don't have access
> to.
>
> If the nested guest tries to use one of those facilities, the hardware
> will cause a Hypervisor Facility Unavailable interrupt. The HFSCR
> register is modified by the hardware to contain information about the
> cause of the interrupt.
>
> We have been returning the cause bits to the nested hypervisor but
> since commit 549e29b458c5 ("KVM: PPC: Book3S HV: Sanitise vcpu
> registers in nested path") we are reducing the amount of information
> exposed to L1, so it seems like a good idea to restrict some of the
> cause bits as well.
>
> With this patch the L1 guest will be allowed to handle only the
> interrupts caused by facilities it has disabled for L2. The interrupts
> caused by facilities that L0 denied will cause a Program Interrupt in
> L1.
I'm not sure if this is a good solution. This would be randomly killing
guest processes or kernels with no way for them to understand what's going
on or deal with it.
The problem is really a nested hypervisor mismatch / configuration
error, so it should be handled between the L0 and L1. Returning failure
from H_ENTER_NESTED, for example (which is probe-able, but not really
any less probe-able than this approach).
Thanks,
Nick
>
> Signed-off-by: Fabiano Rosas <farosas@xxxxxxxxxxxxx>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 270552dd42c5..912a2bcdf7b0 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -138,6 +138,23 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> hr->heir = vcpu->arch.emul_inst;
> break;
> + case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
> + {
> + u8 cause = vcpu->arch.hfscr >> 56;
> +
> + WARN_ON_ONCE(cause >= BITS_PER_LONG);
> +
> + if (hr->hfscr & (1UL << cause)) {
> + hr->hfscr &= ~HFSCR_INTR_CAUSE;
> + /*
> + * We have not restored L1 state yet, so queue
> + * this interrupt instead of delivering it
> + * immediately.
> + */
> + kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_PROGRAM);
> + }
> + break;
> + }
> }
> }
>
> --
> 2.29.2
>
>
