Re: [PATCH v1 10/14] s390x/pv: check initial, not maximum RAM size

[David Hildenbrand]

On 24.09.24 22:17, David Hildenbrand wrote:
> On 24.09.24 18:22, Nina Schoetterl-Glausch wrote:
> > On Tue, 2024-09-10 at 19:58 +0200, David Hildenbrand wrote:
> > > We actually want to check the available RAM, not the maximum RAM size.
> > >
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> >
> > Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > Nit below.
> > > ---
> > >    target/s390x/kvm/pv.c | 2 +-
> > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
> > > index dde836d21a..424cce75ca 100644
> > > --- a/target/s390x/kvm/pv.c
> > > +++ b/target/s390x/kvm/pv.c
> > > @@ -133,7 +133,7 @@ bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms)
> > >         * If the feature is not present or if the VM is not larger than 2 GiB,
> > >         * KVM_PV_ASYNC_CLEANUP_PREPARE fill fail; no point in attempting it.
> > >         */
> > > -    if ((MACHINE(ms)->maxram_size <= 2 * GiB) ||
> > > +    if ((MACHINE(ms)->ram_size <= 2 * GiB) ||
> > >            !kvm_check_extension(kvm_state, 
> > > KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) {
> > >            return false;
> > >        }
> >
> > If I understood the kernel code right, the decision is made wrt
> > the size of the gmap address space, which is the same as the
> > limit set for the VM. So using s390_get_memory_limit would be
> > semantically cleaner.
>
> I wonder if we should just drop the RAM size check. Not convinced the
> slightly faster reboot for such small VMs is really relevant? Makes the
> code more complicated than really necessary.

Thinking about it,

diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 424cce75ca..bbb2108546 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -133,7 +133,7 @@ bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms)
      * If the feature is not present or if the VM is not larger than 2 GiB,
      * KVM_PV_ASYNC_CLEANUP_PREPARE fill fail; no point in attempting it.
      */
-    if ((MACHINE(ms)->ram_size <= 2 * GiB) ||
+    if (s390_get_memory_limit(ms) < 2 * GiB ||
         !kvm_check_extension(kvm_state, KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) 
{
         return false;
     }

Is probably the easiest change, thanks!


--
Cheers,

David / dhildenb