| Message ID | 20200321003921.434620-1-leonardo@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] spapr/rtas: Add MinMem to ibm, get-system-parameter RTAS call | expand |
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. > 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? > > Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com> > --- > hw/ppc/spapr_rtas.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 9fb8c8632a..0f3fbca7af 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -276,10 +276,12 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu, > > switch (parameter) { > case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: { > - char *param_val = g_strdup_printf("MaxEntCap=%d," > + char *param_val = g_strdup_printf("MinMem=%" PRIu64 "," > + "MaxEntCap=%d," > "DesMem=%" PRIu64 "," > "DesProcs=%d," > "MaxPlatProcs=%d", > + ms->ram_size / MiB, > ms->smp.max_cpus, > ms->ram_size / MiB, > ms->smp.cpus,
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/
On Mon, Mar 23, 2020 at 07:07:21PM -0300, Leonardo Bras wrote: > 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. Ah, thanks. Hm, yes. In the way we manage VMs with KVM and qemu, I don't think we cal really draw any meaningful distinction between MinMem and DesMem, so it's reasonble for them to have the same value. > > > 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. Hm, ok.
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 9fb8c8632a..0f3fbca7af 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -276,10 +276,12 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu, switch (parameter) { case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: { - char *param_val = g_strdup_printf("MaxEntCap=%d," + char *param_val = g_strdup_printf("MinMem=%" PRIu64 "," + "MaxEntCap=%d," "DesMem=%" PRIu64 "," "DesProcs=%d," "MaxPlatProcs=%d", + ms->ram_size / MiB, ms->smp.max_cpus, ms->ram_size / MiB, ms->smp.cpus,
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. 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. Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com> --- hw/ppc/spapr_rtas.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)