qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 1/1] spapr/rtas: Add MinMem to ibm,get-system-parameter RTAS


From: Leonardo Bras
Subject: Re: [PATCH 1/1] spapr/rtas: Add MinMem to ibm,get-system-parameter RTAS call
Date: Mon, 23 Mar 2020 19:07:21 -0300
User-agent: Evolution 3.34.4 (3.34.4-1.fc31)

On Mon, 2020-03-23 at 14:24 +1100, David Gibson wrote:
> On Fri, Mar 20, 2020 at 09:39:22PM -0300, Leonardo Bras wrote:
> > Add support for MinMem SPLPAR Characteristic on emulated
> > RTAS call ibm,get-system-parameter.
> > 
> > MinMem represents Minimum Memory, that is described in LOPAPR as:
> > The minimum amount of main store that is needed to power on the
> > partition. Minimum memory is expressed in MB of storage.
> 
> Where exactly does LoPAPR say that?  The version I'm looking at
> doesn't describe MinMem all that clearly, other than to say it must be
> <= DesMem, which currently has the same value here.

Please look for "Minimum Memory". It's on Table 237. SPLPAR Terms. 

> > This  provides a way for the OS to discern hotplugged LMBs and
> > LMBs that have started with the VM, allowing it to better provide
> > a way for memory hot-removal.
> 
> This seems a bit dubious.  Surely we should have this information from
> the dynamic-reconfiguration-memory stuff already?  Trying to discern
> this from purely a number seems very fragile - wouldn't that mean
> making assumptions about how qemu / the host is laying out it's fixed
> and dynamic memory which might not be justified?

I agree. 
I previously sent a RFC proposing the usage of a new flag for this same
reason [1], which I thank you for positive feedback.

Even though I think using a flag is a better solution, I am also
working in this other option (MinMem), that would use parameter already
available on the platform, in case the new flag don't get approved.

So far, using MinMem looks like a very complex solution on kernel side,
and I think it's a poor solution.

I decided to send this patch because it's a simple change to the
platform support, that should cause no harm and could even be useful to
other OSes.


Best regards,
Leonardo

[1] http://patchwork.ozlabs.org/patch/1249931/

Attachment: signature.asc
Description: This is a digitally signed message part


reply via email to

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