Re: [PATCH v1 07/14] s390x/s390-hypercall: introduce DIAG500 STORAGE_LIMIT

[Halil Pasic <pasic@linux.ibm.com>]

On Mon, 30 Sep 2024 13:11:31 +0200
Christian Borntraeger <borntraeger@linux.ibm.com> wrote:

> We do have kvm_stat counters for 500, not sure if people debugging virtio
> will care.

Could end up being confusing, as currently we can assume each and every
DIAG 500 is a virtio doorbell. But I don't think the chance of this
causing real headache is big.

> The only important question for me is, what code is generated by gcc for
> the switch statement and will any variant slow down the virtio doorbell.
> diag.c has
>          if (!vcpu->kvm->arch.css_support ||
>              (vcpu->run->s.regs.gprs[1] != KVM_S390_VIRTIO_CCW_NOTIFY))
>                  return -EOPNOTSUPP;
> 
> So 500+4 should probably not cause any harm apart from branch prediction
> going wrong the first 2 or 3 notifies.
> 
> 502 will make kvm_s390_handle_diag larger.

What do you mean by this last paragraph?

I suppose we are talking about
int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)                                 
{                                                                               
        int code = kvm_s390_get_base_disp_rs(vcpu, NULL) & 0xffff;              
                                                                                
        if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)                  
                return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);    
                                                                                
        trace_kvm_s390_handle_diag(vcpu, code);                                 
        switch (code) {                                                         
        case 0x10:                                                              
                return diag_release_pages(vcpu);                                
        case 0x44:                                                              
                return __diag_time_slice_end(vcpu);                             
        case 0x9c:                                                              
                return __diag_time_slice_end_directed(vcpu);                    
        case 0x258:                                                             
                return __diag_page_ref_service(vcpu);                           
        case 0x308:                                                             
                return __diag_ipl_functions(vcpu);                              
        case 0x500:                                                             
                return __diag_virtio_hypercall(vcpu);                           
        default:                                                                
                vcpu->stat.instruction_diagnose_other++;                        
                return -EOPNOTSUPP;                                             
        }                                                                       
}

and my understanding is that the default branch of the switch 
statement would be already suitable for DIAG 502 as it is today
for DIAG 502. So I'm quite confused by your statement that
502 will make kvm_s390_handle_diag larger (as the only meaning
of larger I can think of is more code). Can you please clarify?

Regards,
Halil