| Message ID | 20210415230948.3563415-3-farosas@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: PPC: Book3S HV: Nested guest HFSCR changes | expand |
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@linux.ibm.com> > --- > 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 > >
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; + } } }
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. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- arch/powerpc/kvm/book3s_hv_nested.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)