On 7/21/25 17:39, Collin Walling wrote:
> On 7/21/25 17:26, Collin Walling wrote:
>> On 7/11/25 17:10, Zhuoying Cai wrote:
>
> [...]
>
>>> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
>>> index 8f655a4b7f..d5b3694600 100644
>>> --- a/target/s390x/kvm/kvm.c
>>> +++ b/target/s390x/kvm/kvm.c
>>> @@ -98,6 +98,7 @@
>>> #define DIAG_TIMEREVENT 0x288
>>> #define DIAG_IPL 0x308
>>> #define DIAG_SET_CONTROL_PROGRAM_CODES 0x318
>>> +#define DIAG_CERT_STORE 0x320
>>> #define DIAG_KVM_HYPERCALL 0x500
>>> #define DIAG_KVM_BREAKPOINT 0x501
>>>
>>> @@ -1560,6 +1561,16 @@ static void handle_diag_318(S390CPU *cpu, struct
>>> kvm_run *run)
>>> }
>>> }
>>>
>>> +static void kvm_handle_diag_320(S390CPU *cpu, struct kvm_run *run)
>>> +{
>>> + uint64_t r1, r3;
>>> +
>>> + r1 = (run->s390_sieic.ipa & 0x00f0) >> 4;
>>> + r3 = run->s390_sieic.ipa & 0x000f;
>>> +
>>> + handle_diag_320(&cpu->env, r1, r3, RA_IGNORED);
>>> +}
>
> One more piece I forgot to add here:
>
> This handler function should check if the required facilities are
> installed for DIAG 320. For now, the Certificate Store facility is used
> to indicate DIAG 320 is provided and possibly subcodes 0-3 are
> available. Please add a feature check here and throw a program
> specification interrupt if the facility is not available before
> handle_diag_320 is called.
>
Ack, sorry. I think I was too far into the weeds w.r.t. to fencing this
off that I more-or-less duplicated this feedback in another spot. I
realize now that fencing this feature off in handle_diag_320() makes
more sense, as TCG also invokes it and should handled for that case as well.
My other sentence in the previous reply "In patch 4, you introduce the
feature bit for DIAG 320" should be disregarded, as I see it's already
there in that function. My bad.
>>> +
>>> #define DIAG_KVM_CODE_MASK 0x000000000000ffff
>>>
>>> static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
>
> [...]
>
--
Regards,
Collin
