Re: [PATCH v1 06/14] s390x: introduce s390_get_memory_limit()

[David Hildenbrand <david@redhat.com>]

On 16.09.24 15:20, Nina Schoetterl-Glausch wrote:
> On Tue, 2024-09-10 at 19:58 +0200, David Hildenbrand wrote:
>> Let's add s390_get_memory_limit(), to query what has been successfully
>> set via s390_set_memory_limit(). Allow setting the limit only once.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> 
> Comment below.
>> ---
>>   target/s390x/cpu-sysemu.c | 19 +++++++++++++++++--
>>   target/s390x/cpu.h        |  1 +
>>   2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
>> index 1cd30c1d84..1915567b3a 100644
>> --- a/target/s390x/cpu-sysemu.c
>> +++ b/target/s390x/cpu-sysemu.c
>> @@ -255,12 +255,27 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
>>       return s390_count_running_cpus();
>>   }
>>   
>> +static uint64_t memory_limit;
>> +
>>   int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
>>   {
>> +    int ret = 0;
>> +
>> +    if (memory_limit) {
>> +        return -EBUSY;
>> +    }
>>       if (kvm_enabled()) {
>> -        return kvm_s390_set_mem_limit(new_limit, hw_limit);
>> +        ret = kvm_s390_set_mem_limit(new_limit, hw_limit);
>> +    }
>> +    if (!ret) {
>> +        memory_limit = new_limit;
>>       }
>> -    return 0;
>> +    return ret;
>> +}
>> +
>> +uint64_t s390_get_memory_limit(void)
>> +{
> 
> Might be nice to guard/warn against s390_set_memory_limit not having been called before.
> 

What about the following on top:

diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
index 1915567b3a..07cb85103a 100644
--- a/target/s390x/cpu-sysemu.c
+++ b/target/s390x/cpu-sysemu.c
@@ -263,6 +263,8 @@ int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
  
      if (memory_limit) {
          return -EBUSY;
+    } else if (!new_limit) {
+        return -EINVAL;
      }
      if (kvm_enabled()) {
          ret = kvm_s390_set_mem_limit(new_limit, hw_limit);
@@ -275,6 +277,8 @@ int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
  
  uint64_t s390_get_memory_limit(void)
  {
+    /* We expect to be called after s390_set_memory_limit(). */
+    assert(memory_limit);
      return memory_limit;
  }


-- 
Cheers,

David / dhildenb