Re: [PATCH v1 4/4] s390/kvm: VSIE: correctly handle MVPG when in VSIE

[David Hildenbrand]

On 04.01.21 17:36, Claudio Imbrenda wrote:
> On Mon, 4 Jan 2021 17:08:15 +0100
> David Hildenbrand <david@xxxxxxxxxx> wrote:
> 
>> On 04.01.21 16:22, Claudio Imbrenda wrote:
>>> On Sun, 20 Dec 2020 11:13:57 +0100
>>> David Hildenbrand <david@xxxxxxxxxx> wrote:
>>>   
>>>> On 18.12.20 15:18, Claudio Imbrenda wrote:  
>>>>> Correctly handle the MVPG instruction when issued by a VSIE guest.
>>>>>     
>>>>
>>>> I remember that MVPG SIE documentation was completely crazy and
>>>> full of corner cases. :)  
>>>
>>> you remember correctly
>>>   
>>>> Looking at arch/s390/kvm/intercept.c:handle_mvpg_pei(), I can spot
>>>> that
>>>>
>>>> 1. "This interception can only happen for guests with DAT disabled
>>>> ..." 2. KVM does not make use of any mvpg state inside the SCB.
>>>>
>>>> Can this be observed with Linux guests?  
>>>
>>> a Linux guest will typically not run with DAT disabled
>>>   
>>>> Can I get some information on what information is stored at [0xc0,
>>>> 0xd) inside the SCB? I assume it's:
>>>>
>>>> 0xc0: guest physical address of source PTE
>>>> 0xc8: guest physical address of target PTE  
>>>
>>> yes (plus 3 flags in the lower bits of each)  
>>
>> Thanks! Do the flags tell us what the deal with the PTE was? If yes,
>> what's the meaning of the separate flags?
>>
>> I assume something like "invalid, proteced, ??"
> 
> bit 61 indicates that the address is a region or segment table entry,
> when EDAT applies
> bit 62 is "protected" when the protected bit is set in the segment
> table entry (or region, if EDAT applies) 
> bit 63 is set when the operand was translated with a real-space ASCE

Thanks!

> but you can check if the PTE is valid just by dereferencing the
> pointers...

The pgtable might already have been unshadowed and repurposed I think.
So for vSIE, the PTE content, therefore, is a little unreliable.

We could, of course, try using them to make a guess.

"Likely valid"
"Likely invalid"

A rerun of the vSIE will fixup any wrong guess.

> 
>> I'm asking because I think we can handle this a little easier.
> 
> what is your idea?

I was wondering if we can

1. avoid essentially two translations per PTE, obtaining the information
we need while tying to shadow. kvm_s390_shadow_fault() on steroids that

a) gives us the last guest pte address (tricky for segment.region table
I think ... will have to think about this)
b) the final protection

2. avoid faulting/shadowing in case we know an entry is not problematic.
E.g., no need to shadow/fault the source in case the PTE is there and
not invalid. "likely valid" case above.


The idea would be to call the new kvm_s390_shadow_fault() two times (or
only once due to our guesses) and either rerun the vsie, inject an
interrupt, or create the partial intercept.

Essentially avoiding kvm_s390_vsie_mvpg_check(). Will have to think
about this.

[...]
>>
>> arch/s390/kvm/intercept.c:handle_partial_execution() we only seem to
>> handle
>>
>> 1. MVPG
>> 2. SIGP PEI
>>
>> The latter is only relevant for external calls. IIRC, this is only
>> active with sigp interpretation - which is never active under vsie
>> (ECA_SIGPI).
> 
> I think putting an explicit check is better than just a jump in the
> dark.

Agreed, but that should then be called out somewhere why the change as
done. (e.g., separate cleanup patch)

-- 
Thanks,

David / dhildenb