qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH/s390-next 2/3] s390x/cpumodel: add zpci, aen and


From: Christian Borntraeger
Subject: Re: [Qemu-devel] [PATCH/s390-next 2/3] s390x/cpumodel: add zpci, aen and ais facilities
Date: Thu, 13 Jul 2017 15:11:15 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0

On 07/13/2017 03:06 PM, Cornelia Huck wrote:
> On Thu, 13 Jul 2017 14:29:55 +0200
> Christian Borntraeger <address@hidden> wrote:
> 
>> On 07/13/2017 02:11 PM, Cornelia Huck wrote:
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>> index 78ebe83..1901153 100644
>>>> --- a/target/s390x/kvm.c
>>>> +++ b/target/s390x/kvm.c
>>>> @@ -302,6 +302,9 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>>>          }
>>>>      }
>>>>  
>>>> +    /* Try to enable AIS facility */
>>>> +    kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);  
>>>
>>> What happens if you fail to enable it? You probably don't want to allow
>>> the feature bit, then?  
>>
>> Then this bit is off. This call will enable it in the kernel, if that fails
>> the kernel will return this bit as disabled.
> 
> Looked at the kernel code again. I thought there was more to it, but I
> misremembered. No further complaints here.
> 
>>>   
>>>> +
>>>>      qemu_mutex_init(&qemu_sigp_mutex);
>>>>  
>>>>      return 0;  
>>>
>>> Let's summarize to make sure that I'm not confused:
>>>
>>> - Starting with zEC12 GA1, we provide zpci, aen, ais in the full model  
>> yes
>>
>>> - Starting with zEC12 GA1, we provide zpci and aen in the default model  
>> yes. ais has to be enabled manually for z12 and z13.
>> The alternative is to have ais as part of the default model. This has the big
>> disadvantage that -cpu zEC12 and -cpu z13 will stop working for all available
>> distro kernels. I believe that a working -cpu z13 is more important than the
>> need to manually enable ais.
> 
> Agreed.
> 
>> For the future
>>  - We can make ais part of the default model for a future system when that 
>> happens since
>>    the features for a new system will require a new host kernel anyway
> 
> Sounds fine.
> 
>>  - We can also make ais part of the default model for a future machine type 
>> (e.g. 2.13)
>>    when we believe that the world has moved on to a newer kernel
> 
> I'd rather avoid relying on that.
> 
>>
>>
>>> - In the host model, we add zpci and aen; they might be switched off
>>>   after applying the found model  
>>
>> We also get ais from the kernel, so the host model will have zpci,ais and
>> aen
>>
>>> - Compat for 2.9 and earlier switches off zpci, aen, ais  
>>
>> yes 
>>
>>> - We unconditionally enable the kvm part of ais  
>>
>> We tell the KVM code in the kernel to enable the facility bit (before the cpu
>> model might take it way) and if QEMU really uses ais, that the kernel does
>> the right thing then
> 
> Yeah, that's fine, see above.
> 
>>>
>>> I'm still not sure what's supposed to happen with new qemu + old kernel
>>> (no ais) + full zEC12 GA1 or later model.  
>>
>> We enable aen and zpci, but disable ais for that guest. In theory a guest
>> can drive PCI devices without AIS. This is a valid configuration since zpci
>> does not require ais. 
> 
> Yes, also fine. Looking at the kernel code cleared things up :)
> 
>> The fact that Linux requires ais to use PCI is unfortunate but that could 
>> be "fixed" Linux if necessary.
> 
> I'm not sure there's a need for this. Once a change like this has
> landed in distro kernels, the same distros will also have the ais
> changes in their hypervisor kernels.
> 
> Thanks for spelling things out again, this stuff always gives me a
> headache.

Assuming that I will keep the return because I like Halils explanation of
"I'm in favor of 3 (keeping) as the resulting code is cleaner:
it does not make any sense to 'continue realizing', even if 
'continue realizing' and set a correct ais_supported just
to fail later does not hurt.
"

Is that an Acked-by or Reviewed-by ?




reply via email to

[Prev in Thread] Current Thread [Next in Thread]