diff mbox series

[v2,06/13] s390x: protvirt: KVM intercept changes

Message ID 20191129094809.26684-7-frankja@linux.ibm.com
State New
Headers show
Series s390x: Protected Virtualization support | expand

Commit Message

Janosch Frank Nov. 29, 2019, 9:48 a.m. UTC
Secure guests no longer intercept with code 4 for an instruction
interception. Instead they have codes 104 and 108 for secure
instruction interception and secure instruction notification
respectively.

The 104 mirrors the 4 interception.

The 108 is a notification interception to let KVM and QEMU know that
something changed and we need to update tracking information or
perform specific tasks. It's currently taken for the following
instructions:

* stpx (To inform about the changed prefix location)
* sclp (On incorrect SCCB values, so we can inject a IRQ)
* sigp (All but "stop and store status")
* diag308 (Subcodes 0/1)

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 target/s390x/kvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

David Hildenbrand Nov. 29, 2019, 10:34 a.m. UTC | #1
On 29.11.19 10:48, Janosch Frank wrote:
> Secure guests no longer intercept with code 4 for an instruction
> interception. Instead they have codes 104 and 108 for secure
> instruction interception and secure instruction notification
> respectively.
> 
> The 104 mirrors the 4 interception.
> 
> The 108 is a notification interception to let KVM and QEMU know that
> something changed and we need to update tracking information or
> perform specific tasks. It's currently taken for the following
> instructions:
> 
> * stpx (To inform about the changed prefix location)
> * sclp (On incorrect SCCB values, so we can inject a IRQ)
> * sigp (All but "stop and store status")
> * diag308 (Subcodes 0/1)
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  target/s390x/kvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index ad6e38c876..3d9c44ba9d 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -115,6 +115,8 @@
>  #define ICPT_CPU_STOP                   0x28
>  #define ICPT_OPEREXC                    0x2c
>  #define ICPT_IO                         0x40
> +#define ICPT_PV_INSTR                   0x68
> +#define ICPT_PV_INSTR_NOTIFICATION      0x6c
>  
>  #define NR_LOCAL_IRQS 32
>  /*
> @@ -151,6 +153,7 @@ static int cap_s390_irq;
>  static int cap_ri;
>  static int cap_gs;
>  static int cap_hpage_1m;
> +static int cap_protvirt;
>  
>  static int active_cmma;
>  
> @@ -342,6 +345,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>      cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
>      cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
> +    cap_protvirt = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>  

Introduced but not used. This has to be moved to the place where it is
actually needed ...
Cornelia Huck Dec. 5, 2019, 5:15 p.m. UTC | #2
On Fri, 29 Nov 2019 04:48:02 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

> Secure guests no longer intercept with code 4 for an instruction
> interception. Instead they have codes 104 and 108 for secure
> instruction interception and secure instruction notification
> respectively.
> 
> The 104 mirrors the 4 interception.
> 
> The 108 is a notification interception to let KVM and QEMU know that
> something changed and we need to update tracking information or
> perform specific tasks. It's currently taken for the following
> instructions:
> 
> * stpx (To inform about the changed prefix location)
> * sclp (On incorrect SCCB values, so we can inject a IRQ)
> * sigp (All but "stop and store status")
> * diag308 (Subcodes 0/1)
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  target/s390x/kvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index ad6e38c876..3d9c44ba9d 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -115,6 +115,8 @@
>  #define ICPT_CPU_STOP                   0x28
>  #define ICPT_OPEREXC                    0x2c
>  #define ICPT_IO                         0x40
> +#define ICPT_PV_INSTR                   0x68
> +#define ICPT_PV_INSTR_NOTIFICATION      0x6c
>  
>  #define NR_LOCAL_IRQS 32
>  /*
> @@ -151,6 +153,7 @@ static int cap_s390_irq;
>  static int cap_ri;
>  static int cap_gs;
>  static int cap_hpage_1m;
> +static int cap_protvirt;
>  
>  static int active_cmma;
>  
> @@ -342,6 +345,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>      cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
>      cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
> +    cap_protvirt = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>  
>      if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
>          || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
>              (long)cs->kvm_run->psw_addr);
>      switch (icpt_code) {
>          case ICPT_INSTRUCTION:
> +        case ICPT_PV_INSTR:
> +        case ICPT_PV_INSTR_NOTIFICATION:
>              r = handle_instruction(cpu, run);

I'm still a bit uneasy about going through the same path for both 104
and 108. How does the handler figure out whether it should emulate an
instruction, or just process a notification? Is it guaranteed that a
given instruction is always showing up as either a 104 or a 108, so
that the handler can check the pv state?

[Even if that works, it still feels a bit unclean to me.]

>              break;
>          case ICPT_PROGRAM:
Janosch Frank Dec. 5, 2019, 5:34 p.m. UTC | #3
On 12/5/19 6:15 PM, Cornelia Huck wrote:
> On Fri, 29 Nov 2019 04:48:02 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Secure guests no longer intercept with code 4 for an instruction
>> interception. Instead they have codes 104 and 108 for secure
>> instruction interception and secure instruction notification
>> respectively.
>>
>> The 104 mirrors the 4 interception.
>>
>> The 108 is a notification interception to let KVM and QEMU know that
>> something changed and we need to update tracking information or
>> perform specific tasks. It's currently taken for the following
>> instructions:
>>
>> * stpx (To inform about the changed prefix location)
>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
>> * sigp (All but "stop and store status")
>> * diag308 (Subcodes 0/1)
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  target/s390x/kvm.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index ad6e38c876..3d9c44ba9d 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -115,6 +115,8 @@
>>  #define ICPT_CPU_STOP                   0x28
>>  #define ICPT_OPEREXC                    0x2c
>>  #define ICPT_IO                         0x40
>> +#define ICPT_PV_INSTR                   0x68
>> +#define ICPT_PV_INSTR_NOTIFICATION      0x6c
>>  
>>  #define NR_LOCAL_IRQS 32
>>  /*
>> @@ -151,6 +153,7 @@ static int cap_s390_irq;
>>  static int cap_ri;
>>  static int cap_gs;
>>  static int cap_hpage_1m;
>> +static int cap_protvirt;
>>  
>>  static int active_cmma;
>>  
>> @@ -342,6 +345,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>      cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
>>      cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
>>      cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
>> +    cap_protvirt = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
>>  
>>      if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
>>          || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
>>              (long)cs->kvm_run->psw_addr);
>>      switch (icpt_code) {
>>          case ICPT_INSTRUCTION:
>> +        case ICPT_PV_INSTR:
>> +        case ICPT_PV_INSTR_NOTIFICATION:
>>              r = handle_instruction(cpu, run);
> 
> I'm still a bit uneasy about going through the same path for both 104
> and 108. How does the handler figure out whether it should emulate an
> instruction, or just process a notification? Is it guaranteed that a
> given instruction is always showing up as either a 104 or a 108, so
> that the handler can check the pv state?

diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
104 (if they are an exit at all)...

> 
> [Even if that works, it still feels a bit unclean to me.]
> 
>>              break;
>>          case ICPT_PROGRAM:
> 
>
Cornelia Huck Dec. 5, 2019, 5:46 p.m. UTC | #4
On Thu, 5 Dec 2019 18:34:32 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 12/5/19 6:15 PM, Cornelia Huck wrote:
> > On Fri, 29 Nov 2019 04:48:02 -0500
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Secure guests no longer intercept with code 4 for an instruction
> >> interception. Instead they have codes 104 and 108 for secure
> >> instruction interception and secure instruction notification
> >> respectively.
> >>
> >> The 104 mirrors the 4 interception.
> >>
> >> The 108 is a notification interception to let KVM and QEMU know that
> >> something changed and we need to update tracking information or
> >> perform specific tasks. It's currently taken for the following
> >> instructions:
> >>
> >> * stpx (To inform about the changed prefix location)
> >> * sclp (On incorrect SCCB values, so we can inject a IRQ)
> >> * sigp (All but "stop and store status")
> >> * diag308 (Subcodes 0/1)
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>  target/s390x/kvm.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>

> >> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
> >>              (long)cs->kvm_run->psw_addr);
> >>      switch (icpt_code) {
> >>          case ICPT_INSTRUCTION:
> >> +        case ICPT_PV_INSTR:
> >> +        case ICPT_PV_INSTR_NOTIFICATION:
> >>              r = handle_instruction(cpu, run);  
> > 
> > I'm still a bit uneasy about going through the same path for both 104
> > and 108. How does the handler figure out whether it should emulate an
> > instruction, or just process a notification? Is it guaranteed that a
> > given instruction is always showing up as either a 104 or a 108, so
> > that the handler can check the pv state?  
> 
> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
> 104 (if they are an exit at all)...

I think that's a reason to really split 108 from 4/104, or at least add
an parameter...

> 
> > 
> > [Even if that works, it still feels a bit unclean to me.]
> >   
> >>              break;
> >>          case ICPT_PROGRAM:  
> > 
> >   
> 
>
Janosch Frank Dec. 6, 2019, 7:44 a.m. UTC | #5
On 12/5/19 6:46 PM, Cornelia Huck wrote:
> On Thu, 5 Dec 2019 18:34:32 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 12/5/19 6:15 PM, Cornelia Huck wrote:
>>> On Fri, 29 Nov 2019 04:48:02 -0500
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> Secure guests no longer intercept with code 4 for an instruction
>>>> interception. Instead they have codes 104 and 108 for secure
>>>> instruction interception and secure instruction notification
>>>> respectively.
>>>>
>>>> The 104 mirrors the 4 interception.
>>>>
>>>> The 108 is a notification interception to let KVM and QEMU know that
>>>> something changed and we need to update tracking information or
>>>> perform specific tasks. It's currently taken for the following
>>>> instructions:
>>>>
>>>> * stpx (To inform about the changed prefix location)
>>>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
>>>> * sigp (All but "stop and store status")
>>>> * diag308 (Subcodes 0/1)
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> ---
>>>>  target/s390x/kvm.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
> 
>>>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
>>>>              (long)cs->kvm_run->psw_addr);
>>>>      switch (icpt_code) {
>>>>          case ICPT_INSTRUCTION:
>>>> +        case ICPT_PV_INSTR:
>>>> +        case ICPT_PV_INSTR_NOTIFICATION:
>>>>              r = handle_instruction(cpu, run);  
>>>
>>> I'm still a bit uneasy about going through the same path for both 104
>>> and 108. How does the handler figure out whether it should emulate an
>>> instruction, or just process a notification? Is it guaranteed that a
>>> given instruction is always showing up as either a 104 or a 108, so
>>> that the handler can check the pv state?  
>>
>> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
>> 104 (if they are an exit at all)...
> 
> I think that's a reason to really split 108 from 4/104, or at least add
> an parameter...

And still call the diag 308 handler or have separate handlers?

> 
>>
>>>
>>> [Even if that works, it still feels a bit unclean to me.]
>>>   
>>>>              break;
>>>>          case ICPT_PROGRAM:  
>>>
>>>   
>>
>>
>
Cornelia Huck Dec. 6, 2019, 8:29 a.m. UTC | #6
On Fri, 6 Dec 2019 08:44:52 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 12/5/19 6:46 PM, Cornelia Huck wrote:
> > On Thu, 5 Dec 2019 18:34:32 +0100
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> On 12/5/19 6:15 PM, Cornelia Huck wrote:  
> >>> On Fri, 29 Nov 2019 04:48:02 -0500
> >>> Janosch Frank <frankja@linux.ibm.com> wrote:
> >>>     
> >>>> Secure guests no longer intercept with code 4 for an instruction
> >>>> interception. Instead they have codes 104 and 108 for secure
> >>>> instruction interception and secure instruction notification
> >>>> respectively.
> >>>>
> >>>> The 104 mirrors the 4 interception.
> >>>>
> >>>> The 108 is a notification interception to let KVM and QEMU know that
> >>>> something changed and we need to update tracking information or
> >>>> perform specific tasks. It's currently taken for the following
> >>>> instructions:
> >>>>
> >>>> * stpx (To inform about the changed prefix location)
> >>>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
> >>>> * sigp (All but "stop and store status")
> >>>> * diag308 (Subcodes 0/1)
> >>>>
> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>>> ---
> >>>>  target/s390x/kvm.c | 6 ++++++
> >>>>  1 file changed, 6 insertions(+)
> >>>>  
> >   
> >>>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
> >>>>              (long)cs->kvm_run->psw_addr);
> >>>>      switch (icpt_code) {
> >>>>          case ICPT_INSTRUCTION:
> >>>> +        case ICPT_PV_INSTR:
> >>>> +        case ICPT_PV_INSTR_NOTIFICATION:
> >>>>              r = handle_instruction(cpu, run);    
> >>>
> >>> I'm still a bit uneasy about going through the same path for both 104
> >>> and 108. How does the handler figure out whether it should emulate an
> >>> instruction, or just process a notification? Is it guaranteed that a
> >>> given instruction is always showing up as either a 104 or a 108, so
> >>> that the handler can check the pv state?    
> >>
> >> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
> >> 104 (if they are an exit at all)...  
> > 
> > I think that's a reason to really split 108 from 4/104, or at least add
> > an parameter...  
> 
> And still call the diag 308 handler or have separate handlers?

I'd probably split it into a "normal" one and one for pv special
handling... does that make sense?
Janosch Frank Dec. 6, 2019, 8:45 a.m. UTC | #7
On 12/6/19 9:29 AM, Cornelia Huck wrote:
> On Fri, 6 Dec 2019 08:44:52 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 12/5/19 6:46 PM, Cornelia Huck wrote:
>>> On Thu, 5 Dec 2019 18:34:32 +0100
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> On 12/5/19 6:15 PM, Cornelia Huck wrote:  
>>>>> On Fri, 29 Nov 2019 04:48:02 -0500
>>>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>>>     
>>>>>> Secure guests no longer intercept with code 4 for an instruction
>>>>>> interception. Instead they have codes 104 and 108 for secure
>>>>>> instruction interception and secure instruction notification
>>>>>> respectively.
>>>>>>
>>>>>> The 104 mirrors the 4 interception.
>>>>>>
>>>>>> The 108 is a notification interception to let KVM and QEMU know that
>>>>>> something changed and we need to update tracking information or
>>>>>> perform specific tasks. It's currently taken for the following
>>>>>> instructions:
>>>>>>
>>>>>> * stpx (To inform about the changed prefix location)
>>>>>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
>>>>>> * sigp (All but "stop and store status")
>>>>>> * diag308 (Subcodes 0/1)
>>>>>>
>>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>>> ---
>>>>>>  target/s390x/kvm.c | 6 ++++++
>>>>>>  1 file changed, 6 insertions(+)
>>>>>>  
>>>   
>>>>>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
>>>>>>              (long)cs->kvm_run->psw_addr);
>>>>>>      switch (icpt_code) {
>>>>>>          case ICPT_INSTRUCTION:
>>>>>> +        case ICPT_PV_INSTR:
>>>>>> +        case ICPT_PV_INSTR_NOTIFICATION:
>>>>>>              r = handle_instruction(cpu, run);    
>>>>>
>>>>> I'm still a bit uneasy about going through the same path for both 104
>>>>> and 108. How does the handler figure out whether it should emulate an
>>>>> instruction, or just process a notification? Is it guaranteed that a
>>>>> given instruction is always showing up as either a 104 or a 108, so
>>>>> that the handler can check the pv state?    
>>>>
>>>> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
>>>> 104 (if they are an exit at all)...  
>>>
>>> I think that's a reason to really split 108 from 4/104, or at least add
>>> an parameter...  
>>
>> And still call the diag 308 handler or have separate handlers?
> 
> I'd probably split it into a "normal" one and one for pv special
> handling... does that make sense?
> 
IMHO: not really
We still need to do ipa/ipb parsing for both paths, which will result in
code duplication. Looking at diag308 subcode 4, we would have a code 4
one which just does the device resets and reboots and one which does all
that, plus the teardown of the protected guest.

I tried to inline as much as possible to have as little changes as
possible. Notable exception is sclp, which has more checks than
emulation code...
Cornelia Huck Dec. 6, 2019, 9:08 a.m. UTC | #8
On Fri, 6 Dec 2019 09:45:41 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 12/6/19 9:29 AM, Cornelia Huck wrote:
> > On Fri, 6 Dec 2019 08:44:52 +0100
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> On 12/5/19 6:46 PM, Cornelia Huck wrote:  
> >>> On Thu, 5 Dec 2019 18:34:32 +0100
> >>> Janosch Frank <frankja@linux.ibm.com> wrote:
> >>>     
> >>>> On 12/5/19 6:15 PM, Cornelia Huck wrote:    
> >>>>> On Fri, 29 Nov 2019 04:48:02 -0500
> >>>>> Janosch Frank <frankja@linux.ibm.com> wrote:
> >>>>>       
> >>>>>> Secure guests no longer intercept with code 4 for an instruction
> >>>>>> interception. Instead they have codes 104 and 108 for secure
> >>>>>> instruction interception and secure instruction notification
> >>>>>> respectively.
> >>>>>>
> >>>>>> The 104 mirrors the 4 interception.
> >>>>>>
> >>>>>> The 108 is a notification interception to let KVM and QEMU know that
> >>>>>> something changed and we need to update tracking information or
> >>>>>> perform specific tasks. It's currently taken for the following
> >>>>>> instructions:
> >>>>>>
> >>>>>> * stpx (To inform about the changed prefix location)
> >>>>>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
> >>>>>> * sigp (All but "stop and store status")
> >>>>>> * diag308 (Subcodes 0/1)
> >>>>>>
> >>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>>>>> ---
> >>>>>>  target/s390x/kvm.c | 6 ++++++
> >>>>>>  1 file changed, 6 insertions(+)
> >>>>>>    
> >>>     
> >>>>>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
> >>>>>>              (long)cs->kvm_run->psw_addr);
> >>>>>>      switch (icpt_code) {
> >>>>>>          case ICPT_INSTRUCTION:
> >>>>>> +        case ICPT_PV_INSTR:
> >>>>>> +        case ICPT_PV_INSTR_NOTIFICATION:
> >>>>>>              r = handle_instruction(cpu, run);      
> >>>>>
> >>>>> I'm still a bit uneasy about going through the same path for both 104
> >>>>> and 108. How does the handler figure out whether it should emulate an
> >>>>> instruction, or just process a notification? Is it guaranteed that a
> >>>>> given instruction is always showing up as either a 104 or a 108, so
> >>>>> that the handler can check the pv state?      
> >>>>
> >>>> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
> >>>> 104 (if they are an exit at all)...    
> >>>
> >>> I think that's a reason to really split 108 from 4/104, or at least add
> >>> an parameter...    
> >>
> >> And still call the diag 308 handler or have separate handlers?  
> > 
> > I'd probably split it into a "normal" one and one for pv special
> > handling... does that make sense?
> >   
> IMHO: not really
> We still need to do ipa/ipb parsing for both paths, which will result in
> code duplication. Looking at diag308 subcode 4, we would have a code 4
> one which just does the device resets and reboots and one which does all
> that, plus the teardown of the protected guest.
> 
> I tried to inline as much as possible to have as little changes as
> possible. Notable exception is sclp, which has more checks than
> emulation code...

Fair enough.

But taking a step back: What's the purpose of the new exits, then?
IIUC, we have the following cases:

- code 4: normal guest, nothing special
- code 104: protected guest, emulate the instruction
- code 108: protected guest, notification for the instruction

The backend code can figure out what to do simply by checking whether
the guest is protected or not (as whatever needs to be done is simply
determined by that anyway).

Are we overlooking something? Or is the information contained in the
different exits simply redundant?
Janosch Frank Dec. 6, 2019, 9:30 a.m. UTC | #9
On 12/6/19 10:08 AM, Cornelia Huck wrote:
> On Fri, 6 Dec 2019 09:45:41 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 12/6/19 9:29 AM, Cornelia Huck wrote:
>>> On Fri, 6 Dec 2019 08:44:52 +0100
>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>   
>>>> On 12/5/19 6:46 PM, Cornelia Huck wrote:  
>>>>> On Thu, 5 Dec 2019 18:34:32 +0100
>>>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>>>     
>>>>>> On 12/5/19 6:15 PM, Cornelia Huck wrote:    
>>>>>>> On Fri, 29 Nov 2019 04:48:02 -0500
>>>>>>> Janosch Frank <frankja@linux.ibm.com> wrote:
>>>>>>>       
>>>>>>>> Secure guests no longer intercept with code 4 for an instruction
>>>>>>>> interception. Instead they have codes 104 and 108 for secure
>>>>>>>> instruction interception and secure instruction notification
>>>>>>>> respectively.
>>>>>>>>
>>>>>>>> The 104 mirrors the 4 interception.
>>>>>>>>
>>>>>>>> The 108 is a notification interception to let KVM and QEMU know that
>>>>>>>> something changed and we need to update tracking information or
>>>>>>>> perform specific tasks. It's currently taken for the following
>>>>>>>> instructions:
>>>>>>>>
>>>>>>>> * stpx (To inform about the changed prefix location)
>>>>>>>> * sclp (On incorrect SCCB values, so we can inject a IRQ)
>>>>>>>> * sigp (All but "stop and store status")
>>>>>>>> * diag308 (Subcodes 0/1)
>>>>>>>>
>>>>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>>>>> ---
>>>>>>>>  target/s390x/kvm.c | 6 ++++++
>>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>>    
>>>>>     
>>>>>>>> @@ -1664,6 +1668,8 @@ static int handle_intercept(S390CPU *cpu)
>>>>>>>>              (long)cs->kvm_run->psw_addr);
>>>>>>>>      switch (icpt_code) {
>>>>>>>>          case ICPT_INSTRUCTION:
>>>>>>>> +        case ICPT_PV_INSTR:
>>>>>>>> +        case ICPT_PV_INSTR_NOTIFICATION:
>>>>>>>>              r = handle_instruction(cpu, run);      
>>>>>>>
>>>>>>> I'm still a bit uneasy about going through the same path for both 104
>>>>>>> and 108. How does the handler figure out whether it should emulate an
>>>>>>> instruction, or just process a notification? Is it guaranteed that a
>>>>>>> given instruction is always showing up as either a 104 or a 108, so
>>>>>>> that the handler can check the pv state?      
>>>>>>
>>>>>> diag 308 subcode 0/1 are 108, but all other subcodes are defined as a
>>>>>> 104 (if they are an exit at all)...    
>>>>>
>>>>> I think that's a reason to really split 108 from 4/104, or at least add
>>>>> an parameter...    
>>>>
>>>> And still call the diag 308 handler or have separate handlers?  
>>>
>>> I'd probably split it into a "normal" one and one for pv special
>>> handling... does that make sense?
>>>   
>> IMHO: not really
>> We still need to do ipa/ipb parsing for both paths, which will result in
>> code duplication. Looking at diag308 subcode 4, we would have a code 4
>> one which just does the device resets and reboots and one which does all
>> that, plus the teardown of the protected guest.
>>
>> I tried to inline as much as possible to have as little changes as
>> possible. Notable exception is sclp, which has more checks than
>> emulation code...
> 
> Fair enough.
> 
> But taking a step back: What's the purpose of the new exits, then?
> IIUC, we have the following cases:
> 
> - code 4: normal guest, nothing special
> - code 104: protected guest, emulate the instruction
> - code 108: protected guest, notification for the instruction
> 
> The backend code can figure out what to do simply by checking whether
> the guest is protected or not (as whatever needs to be done is simply
> determined by that anyway).
> 
> Are we overlooking something? Or is the information contained in the
> different exits simply redundant?

The difference is in the entry after the exit:

On a 104 we have a "continuation", i.e. the data that's a result of the
emulation by KVM/QEMU is used to complete the instruction. Copying the
sccb from the satellite block into guest2 memory, etc.

For a 108 we don't have any special handling (except for maybe state
checking) and just continue with the next instruction.
diff mbox series

Patch

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index ad6e38c876..3d9c44ba9d 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -115,6 +115,8 @@ 
 #define ICPT_CPU_STOP                   0x28
 #define ICPT_OPEREXC                    0x2c
 #define ICPT_IO                         0x40
+#define ICPT_PV_INSTR                   0x68
+#define ICPT_PV_INSTR_NOTIFICATION      0x6c
 
 #define NR_LOCAL_IRQS 32
 /*
@@ -151,6 +153,7 @@  static int cap_s390_irq;
 static int cap_ri;
 static int cap_gs;
 static int cap_hpage_1m;
+static int cap_protvirt;
 
 static int active_cmma;
 
@@ -342,6 +345,7 @@  int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
     cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
     cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
+    cap_protvirt = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
 
     if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
         || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
@@ -1664,6 +1668,8 @@  static int handle_intercept(S390CPU *cpu)
             (long)cs->kvm_run->psw_addr);
     switch (icpt_code) {
         case ICPT_INSTRUCTION:
+        case ICPT_PV_INSTR:
+        case ICPT_PV_INSTR_NOTIFICATION:
             r = handle_instruction(cpu, run);
             break;
         case ICPT_PROGRAM: