On 02/14/2018 11:06 AM, David Hildenbrand wrote:
> On 14.02.2018 09:34, Christian Borntraeger wrote:
>> If the guest runs with bp isolation when doing a SIE instruction,
>> we must also run the nested guest with bp isolation when emulating
>> that SIE instruction.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>> arch/s390/kvm/vsie.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index ec772700ff96..b8e7660d7207 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -821,6 +821,7 @@ static int do_vsie_run(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>> {
>> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
>> struct kvm_s390_sie_block *scb_o = vsie_page->scb_o;
>> + int guest_bp_isolation;
>> int rc;
>>
>> handle_last_fault(vcpu, vsie_page);
>> @@ -831,6 +832,15 @@ static int do_vsie_run(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>> s390_handle_mcck();
>>
>> srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
>> +
>> + /* save current guest state of bp isolation override */
>> + guest_bp_isolation = test_thread_flag(TIF_ISOLATE_BP_GUEST);
>
> If I am not wrong, this is not "guest state". The guest state is
> vcpu->arch.sie_block->fpf . This is host state of a thread.
Yes, this is the host thread that is going to "emulate" the vsie instruction
by calling sie64a.
>
>> +
>> + /* if guest runs with bp isolation force it on nested guest */
>> + if (test_kvm_facility(vcpu->kvm, 82) &&
>> + vcpu->arch.sie_block->fpf & FPF_BPBC)
>> + set_thread_flag(TIF_ISOLATE_BP_GUEST);
>> +
>> local_irq_disable();
>> guest_enter_irqoff();
>> local_irq_enable();
>> @@ -840,6 +850,11 @@ static int do_vsie_run(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>> local_irq_disable();
>> guest_exit_irqoff();
>> local_irq_enable();
>> +
>> + /* restore guest state for bp isolation override */
>> + if (!guest_bp_isolation)
>> + clear_thread_flag(TIF_ISOLATE_BP_GUEST);
>> +
>> vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
>>
>> if (rc == -EINTR) {
>>
>
> You are trying to optimize the following case here:
I am trying to fix a case where vsie would allow to disable branch prediction blocking.
>
> 1. TIF_ISOLATE_BP_GUEST is not set
> 2. The guest has facility 82 and enabled FPF_BPBC
> As the vSIE guest can change its FPF_BPBC, there is basically no
> guarantee to that. So, when entering/leaving the nested guest, you act
> like the hardware would be doing FPF_BPBC - as it could be disabled for
> the nested guest / the nested guest can change the state itself.
The BPBC is an effective control, so if you enter SIE with bp blocking,
then the guest will have bp blocking "forced" on.
>
> However I wonder what the semantics of FPF_BPBC should be. Shouldn't it
> be the case that if the guest has enabled FPF_BPBC, that it is forced on
> for the nested guest? (HW is missing a control to force it on).
the forcing happens by being an effective control. Imagine it like setting
the TIF bit will basically turn on FPF_BPBC on the LPAR level before going
into SIE and the effective value for guest3 running via vsie as guest2
will be that this guest3 can do ppa12/13 as it likes, it will always run
with bp blocking.
So this covers the case where guest2 runs a guest3 with bp blocking.
> Unfortunately, I don't have access to documentation, can you clarify?
>
