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

[David Hildenbrand]

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