Re: [Qemu-devel] [PATCH v2 1/2] s390x/tcg: wire up pci instructions

[David Hildenbrand <david@redhat.com>]

On 01.02.2018 13:48, Cornelia Huck wrote:
> On Thu, 1 Feb 2018 13:42:52 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 31.01.2018 19:17, Cornelia Huck wrote:
> 
>>> +#ifndef CONFIG_USER_ONLY
>>> +void HELPER(clp)(CPUS390XState *env, uint32_t r2)
>>> +{
>>> +    S390CPU *cpu = s390_env_get_cpu(env);
>>> +    int r;
>>> +
>>> +    qemu_mutex_lock_iothread();
>>> +    r = clp_service_call(cpu, r2, GETPC());
>>> +    qemu_mutex_unlock_iothread();
>>> +    if (r) {
>>> +        s390_program_interrupt(env, PGM_OPERATION, 4, GETPC());
>>> +    }  
>>
>> We don't need the if (r) ... so I suggest dropping all these. (as I
>> said, will be handled later via the generic flag checking in translation
>> code). We can ignore any error from these functions.
> 
> I did not check the instruction implementations in detail... was the
> error really only for the !CONFIG_PCI case?

!FEATURE_ZPCI (which includes !CONFIG_PCI)

Yes, that's how I remember.

> 
> (I really should know that...)
> 
>> A sane guest will newer trigger this. (if we have no CONFIG_PCI, the
>> also the ZPCI feature will not be available)
> 
> Hopefully we can also handle non-sane guests correctly...

Usually, if you call instructions that are not indicated via STFL, there
is no guarantee what will happen. Some time in the future, we will
handle this globally (but haven't done so to allow new applications to
run with old CPU models - which was necessary before we bumped the CPU
model to a z12).

If you don't want to wait until that support is added, you should be
save with something like this:


diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 02cd4b2627..a5db014730 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5907,6 +5907,10 @@ static ExitStatus translate_one(CPUS390XState
*env, DisasContext *s)
         gen_illegal_opcode(s);
         return EXIT_NORETURN;
     }
+    if (insn->fac == FAC_PCI && !s390_has_feat(FAC_PCI)) {
+        gen_illegal_opcode(s);
+        return EXIT_NORETURN;
+    }

 #ifndef CONFIG_USER_ONLY
     if (s->tb->flags & FLAG_MASK_PER) {

-- 

Thanks,

David / dhildenb