Move the calculation of the Real Mode Area (RMA) size into a helper
function. While we're there clean it up and correct it in a few ways:
* Add comments making it clearer where the various constraints come from
* Remove a pointless check that the RMA fits within Node 0 (we've just
clamped it so that it does)
* The 16GiB limit we apply is only correct for POWER8, but there is also
a 1TiB limit that applies on POWER9.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr.c | 57 +++++++++++++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 52c39daa99..7efd4f2b85 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2664,6 +2664,40 @@ static PCIHostState *spapr_create_default_phb(void)
return PCI_HOST_BRIDGE(dev);
}
+static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
+{
+ MachineState *machine = MACHINE(spapr);
+ hwaddr rma_size = machine->ram_size;
+ hwaddr node0_size = spapr_node0_size(machine);
+
+ /* RMA has to fit in the first NUMA node */
+ rma_size = MIN(rma_size, node0_size);
+
+ /*
+ * VRMA access is via a special 1TiB SLB mapping, so the RMA can
+ * never exceed that
+ */
+ rma_size = MIN(rma_size, TiB);
+
+ /*
+ * RMA size is controlled in hardware by LPCR[RMLS]. On POWER8
+ * the largest RMA that can be specified there is 16GiB
+ */
+ if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
+ 0, spapr->max_compat_pvr)) {
+ rma_size = MIN(rma_size, 16 * GiB);
+ }
+
+ if (rma_size < (MIN_RMA_SLOF * MiB)) {
+ error_setg(errp,
+"pSeries SLOF firmware requires >= %ldMiB guest RMA (Real Mode Area)",
+ MIN_RMA_SLOF);
+ return -1;
+ }
+
+ return rma_size;
+}
+
/* pSeries LPAR / sPAPR hardware init */
static void spapr_machine_init(MachineState *machine)
{
@@ -2675,7 +2709,6 @@ static void spapr_machine_init(MachineState *machine)
int i;
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
- hwaddr node0_size = spapr_node0_size(machine);
long load_limit, fw_size;
char *filename;
Error *resize_hpt_err = NULL;
@@ -2715,20 +2748,7 @@ static void spapr_machine_init(MachineState *machine)
exit(1);
}
- spapr->rma_size = node0_size;
-
- /* Actually we don't support unbounded RMA anymore since we added
- * proper emulation of HV mode. The max we can get is 16G which
- * also happens to be what we configure for PAPR mode so make sure
- * we don't do anything bigger than that
- */
- spapr->rma_size = MIN(spapr->rma_size, 0x400000000ull);
-
- if (spapr->rma_size > node0_size) {
- error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
- spapr->rma_size);
- exit(1);
- }
+ spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
/* Setup a load limit for the ramdisk leaving room for SLOF and FDT */
load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
@@ -2956,13 +2976,6 @@ static void spapr_machine_init(MachineState *machine)
}
}
- if (spapr->rma_size < (MIN_RMA_SLOF * MiB)) {
- error_report(
- "pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area memory)",
- MIN_RMA_SLOF);
- exit(1);
- }
-
if (kernel_filename) {
uint64_t lowaddr = 0;
--
2.23.0
On 29/11/2019 12:35, David Gibson wrote:
> Move the calculation of the Real Mode Area (RMA) size into a helper
> function. While we're there clean it up and correct it in a few ways:
> * Add comments making it clearer where the various constraints come from
> * Remove a pointless check that the RMA fits within Node 0 (we've just
> clamped it so that it does)
> * The 16GiB limit we apply is only correct for POWER8, but there is also
> a 1TiB limit that applies on POWER9.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 57 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 52c39daa99..7efd4f2b85 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2664,6 +2664,40 @@ static PCIHostState *spapr_create_default_phb(void)
> return PCI_HOST_BRIDGE(dev);
> }
>
> +static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
> +{
> + MachineState *machine = MACHINE(spapr);
> + hwaddr rma_size = machine->ram_size;
> + hwaddr node0_size = spapr_node0_size(machine);
> +
> + /* RMA has to fit in the first NUMA node */
> + rma_size = MIN(rma_size, node0_size);
> +
> + /*
> + * VRMA access is via a special 1TiB SLB mapping, so the RMA can
> + * never exceed that
> + */
> + rma_size = MIN(rma_size, TiB);
> +
> + /*
> + * RMA size is controlled in hardware by LPCR[RMLS]. On POWER8
RMA is controlled by LPCR on P8 but the RMLS bits on P9 are reserved
(also reserved in PowerISA 3.0).
> + * the largest RMA that can be specified there is 16GiB
The P8 user manual says:
===
The following RMO sizes are available for the POWER8 processor.
The RMLS[34:37] field in the LPCR defines the RMO sizes, as described below.
1000 - 32 MB
0011 - 64 MB
0111 - 128 MB
0100 - 256 MB
0010 - 1 GB
0001 - 16 GB
0000 - 256 GB
===
The maximum seems to be 256GiB.
> + */
> + if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
> + 0, spapr->max_compat_pvr)) {
> + rma_size = MIN(rma_size, 16 * GiB);
> + }
> +
> + if (rma_size < (MIN_RMA_SLOF * MiB)) {
nit: it is time to redefine MIN_RMA_SLOF to use MiBs imho :)
> + error_setg(errp,
> +"pSeries SLOF firmware requires >= %ldMiB guest RMA (Real Mode Area)",
> + MIN_RMA_SLOF);
Something went wrong with formatting here.
Otherwise looks good. Thanks,
> + return -1;
> + }
> +
> + return rma_size;
> +}
> +
> /* pSeries LPAR / sPAPR hardware init */
> static void spapr_machine_init(MachineState *machine)
> {
> @@ -2675,7 +2709,6 @@ static void spapr_machine_init(MachineState *machine)
> int i;
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> - hwaddr node0_size = spapr_node0_size(machine);
> long load_limit, fw_size;
> char *filename;
> Error *resize_hpt_err = NULL;
> @@ -2715,20 +2748,7 @@ static void spapr_machine_init(MachineState *machine)
> exit(1);
> }
>
> - spapr->rma_size = node0_size;
> -
> - /* Actually we don't support unbounded RMA anymore since we added
> - * proper emulation of HV mode. The max we can get is 16G which
> - * also happens to be what we configure for PAPR mode so make sure
> - * we don't do anything bigger than that
> - */
> - spapr->rma_size = MIN(spapr->rma_size, 0x400000000ull);
> -
> - if (spapr->rma_size > node0_size) {
> - error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> - spapr->rma_size);
> - exit(1);
> - }
> + spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
>
> /* Setup a load limit for the ramdisk leaving room for SLOF and FDT */
> load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
> @@ -2956,13 +2976,6 @@ static void spapr_machine_init(MachineState *machine)
> }
> }
>
> - if (spapr->rma_size < (MIN_RMA_SLOF * MiB)) {
> - error_report(
> - "pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area memory)",
> - MIN_RMA_SLOF);
> - exit(1);
> - }
> -
> if (kernel_filename) {
> uint64_t lowaddr = 0;
>
>
--
Alexey
On 03/12/2019 14:44, Alexey Kardashevskiy wrote:
>
>
> On 29/11/2019 12:35, David Gibson wrote:
>> Move the calculation of the Real Mode Area (RMA) size into a helper
>> function. While we're there clean it up and correct it in a few ways:
>> * Add comments making it clearer where the various constraints come from
>> * Remove a pointless check that the RMA fits within Node 0 (we've just
>> clamped it so that it does)
>> * The 16GiB limit we apply is only correct for POWER8, but there is also
>> a 1TiB limit that applies on POWER9.
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>> hw/ppc/spapr.c | 57 +++++++++++++++++++++++++++++++-------------------
>> 1 file changed, 35 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 52c39daa99..7efd4f2b85 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2664,6 +2664,40 @@ static PCIHostState *spapr_create_default_phb(void)
>> return PCI_HOST_BRIDGE(dev);
>> }
>>
>> +static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
>> +{
>> + MachineState *machine = MACHINE(spapr);
>> + hwaddr rma_size = machine->ram_size;
>> + hwaddr node0_size = spapr_node0_size(machine);
>> +
>> + /* RMA has to fit in the first NUMA node */
>> + rma_size = MIN(rma_size, node0_size);
>> +
>> + /*
>> + * VRMA access is via a special 1TiB SLB mapping, so the RMA can
>> + * never exceed that
>> + */
>> + rma_size = MIN(rma_size, TiB);
>> +
>> + /*
>> + * RMA size is controlled in hardware by LPCR[RMLS]. On POWER8
>
>
> RMA is controlled by LPCR on P8 but the RMLS bits on P9 are reserved
> (also reserved in PowerISA 3.0).
>
>
>> + * the largest RMA that can be specified there is 16GiB
>
>
> The P8 user manual says:
> ===
> The following RMO sizes are available for the POWER8 processor.
> The RMLS[34:37] field in the LPCR defines the RMO sizes, as described below.
> 1000 - 32 MB
> 0011 - 64 MB
> 0111 - 128 MB
> 0100 - 256 MB
> 0010 - 1 GB
> 0001 - 16 GB
> 0000 - 256 GB
> ===
>
> The maximum seems to be 256GiB.
Ah, update from Paul - we do not actually use what LPCR[RMLS] controls -
Real Mode Offset Register (RMOR).
>
>
>> + */
>> + if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
>> + 0, spapr->max_compat_pvr)) {
>> + rma_size = MIN(rma_size, 16 * GiB);
>> + }
>> +
>> + if (rma_size < (MIN_RMA_SLOF * MiB)) {
>
>
> nit: it is time to redefine MIN_RMA_SLOF to use MiBs imho :)
>
>
>> + error_setg(errp,
>> +"pSeries SLOF firmware requires >= %ldMiB guest RMA (Real Mode Area)",
>> + MIN_RMA_SLOF);
>
> Something went wrong with formatting here.
>
> Otherwise looks good. Thanks,
>
>
>
>> + return -1;
>> + }
>> +
>> + return rma_size;
>> +}
>> +
>> /* pSeries LPAR / sPAPR hardware init */
>> static void spapr_machine_init(MachineState *machine)
>> {
>> @@ -2675,7 +2709,6 @@ static void spapr_machine_init(MachineState *machine)
>> int i;
>> MemoryRegion *sysmem = get_system_memory();
>> MemoryRegion *ram = g_new(MemoryRegion, 1);
>> - hwaddr node0_size = spapr_node0_size(machine);
>> long load_limit, fw_size;
>> char *filename;
>> Error *resize_hpt_err = NULL;
>> @@ -2715,20 +2748,7 @@ static void spapr_machine_init(MachineState *machine)
>> exit(1);
>> }
>>
>> - spapr->rma_size = node0_size;
>> -
>> - /* Actually we don't support unbounded RMA anymore since we added
>> - * proper emulation of HV mode. The max we can get is 16G which
>> - * also happens to be what we configure for PAPR mode so make sure
>> - * we don't do anything bigger than that
>> - */
>> - spapr->rma_size = MIN(spapr->rma_size, 0x400000000ull);
>> -
>> - if (spapr->rma_size > node0_size) {
>> - error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
>> - spapr->rma_size);
>> - exit(1);
>> - }
>> + spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
>>
>> /* Setup a load limit for the ramdisk leaving room for SLOF and FDT */
>> load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
>> @@ -2956,13 +2976,6 @@ static void spapr_machine_init(MachineState *machine)
>> }
>> }
>>
>> - if (spapr->rma_size < (MIN_RMA_SLOF * MiB)) {
>> - error_report(
>> - "pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area memory)",
>> - MIN_RMA_SLOF);
>> - exit(1);
>> - }
>> -
>> if (kernel_filename) {
>> uint64_t lowaddr = 0;
>>
>>
>
--
Alexey
On Tue, Dec 03, 2019 at 04:06:46PM +1100, Alexey Kardashevskiy wrote:
>
>
> On 03/12/2019 14:44, Alexey Kardashevskiy wrote:
> >
> >
> > On 29/11/2019 12:35, David Gibson wrote:
> >> Move the calculation of the Real Mode Area (RMA) size into a helper
> >> function. While we're there clean it up and correct it in a few ways:
> >> * Add comments making it clearer where the various constraints come from
> >> * Remove a pointless check that the RMA fits within Node 0 (we've just
> >> clamped it so that it does)
> >> * The 16GiB limit we apply is only correct for POWER8, but there is also
> >> a 1TiB limit that applies on POWER9.
> >>
> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >> ---
> >> hw/ppc/spapr.c | 57 +++++++++++++++++++++++++++++++-------------------
> >> 1 file changed, 35 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 52c39daa99..7efd4f2b85 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -2664,6 +2664,40 @@ static PCIHostState *spapr_create_default_phb(void)
> >> return PCI_HOST_BRIDGE(dev);
> >> }
> >>
> >> +static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
> >> +{
> >> + MachineState *machine = MACHINE(spapr);
> >> + hwaddr rma_size = machine->ram_size;
> >> + hwaddr node0_size = spapr_node0_size(machine);
> >> +
> >> + /* RMA has to fit in the first NUMA node */
> >> + rma_size = MIN(rma_size, node0_size);
> >> +
> >> + /*
> >> + * VRMA access is via a special 1TiB SLB mapping, so the RMA can
> >> + * never exceed that
> >> + */
> >> + rma_size = MIN(rma_size, TiB);
> >> +
> >> + /*
> >> + * RMA size is controlled in hardware by LPCR[RMLS]. On POWER8
> >
> >
> > RMA is controlled by LPCR on P8 but the RMLS bits on P9 are reserved
> > (also reserved in PowerISA 3.0).
> >
> >
> >> + * the largest RMA that can be specified there is 16GiB
> >
> >
> > The P8 user manual says:
> > ===
> > The following RMO sizes are available for the POWER8 processor.
> > The RMLS[34:37] field in the LPCR defines the RMO sizes, as described below.
> > 1000 - 32 MB
> > 0011 - 64 MB
> > 0111 - 128 MB
> > 0100 - 256 MB
> > 0010 - 1 GB
> > 0001 - 16 GB
> > 0000 - 256 GB
> > ===
> >
> > The maximum seems to be 256GiB.
>
>
> Ah, update from Paul - we do not actually use what LPCR[RMLS] controls -
> Real Mode Offset Register (RMOR).
Ah... I realized where the 16GiB limit was coming from.
We don't use RMLS with KVM, but we *do* use it under TCG. The softmmu
code isn't aware of PAPR specific stuff at this point and just
consults the LPCR to handle real mode accesses. And the TCG
implementation only supports up to 16GiB, even though POWER8 supports
more.
And, AFAICT that limit will apply for a POWER9 guest in hash mode as
well. Not for initial boot, because we run in radix mode until we
determine that the guest wants hash, but if we drop back to real mode
after boot, this might matter.
I'm going to have to think about how to sort that mess out.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Tue, Dec 03, 2019 at 02:44:06PM +1100, Alexey Kardashevskiy wrote:
>
>
> On 29/11/2019 12:35, David Gibson wrote:
> > Move the calculation of the Real Mode Area (RMA) size into a helper
> > function. While we're there clean it up and correct it in a few ways:
> > * Add comments making it clearer where the various constraints come from
> > * Remove a pointless check that the RMA fits within Node 0 (we've just
> > clamped it so that it does)
> > * The 16GiB limit we apply is only correct for POWER8, but there is also
> > a 1TiB limit that applies on POWER9.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 57 +++++++++++++++++++++++++++++++-------------------
> > 1 file changed, 35 insertions(+), 22 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 52c39daa99..7efd4f2b85 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2664,6 +2664,40 @@ static PCIHostState *spapr_create_default_phb(void)
> > return PCI_HOST_BRIDGE(dev);
> > }
> >
> > +static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp)
> > +{
> > + MachineState *machine = MACHINE(spapr);
> > + hwaddr rma_size = machine->ram_size;
> > + hwaddr node0_size = spapr_node0_size(machine);
> > +
> > + /* RMA has to fit in the first NUMA node */
> > + rma_size = MIN(rma_size, node0_size);
> > +
> > + /*
> > + * VRMA access is via a special 1TiB SLB mapping, so the RMA can
> > + * never exceed that
> > + */
> > + rma_size = MIN(rma_size, TiB);
> > +
> > + /*
> > + * RMA size is controlled in hardware by LPCR[RMLS]. On POWER8
>
>
> RMA is controlled by LPCR on P8 but the RMLS bits on P9 are reserved
> (also reserved in PowerISA 3.0).
>
>
> > + * the largest RMA that can be specified there is 16GiB
>
>
> The P8 user manual says:
> ===
> The following RMO sizes are available for the POWER8 processor.
> The RMLS[34:37] field in the LPCR defines the RMO sizes, as described below.
> 1000 - 32 MB
> 0011 - 64 MB
> 0111 - 128 MB
> 0100 - 256 MB
> 0010 - 1 GB
> 0001 - 16 GB
> 0000 - 256 GB
> ===
>
> The maximum seems to be 256GiB.
Huh. Ok, looks like I was wrong about where that 16GiB clamp came
from originally. I wonder where it *did* come from, or if it's simply
wrong.
> > + */
> > + if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
> > + 0, spapr->max_compat_pvr)) {
> > + rma_size = MIN(rma_size, 16 * GiB);
> > + }
> > +
> > + if (rma_size < (MIN_RMA_SLOF * MiB)) {
>
>
> nit: it is time to redefine MIN_RMA_SLOF to use MiBs imho :)
Yeah, I'd thought about that too. I've added a patch to clean that up.
>
>
> > + error_setg(errp,
> > +"pSeries SLOF firmware requires >= %ldMiB guest RMA (Real Mode Area)",
> > + MIN_RMA_SLOF);
>
> Something went wrong with formatting here.
Actually, that's intentional. The idea is to keep the string on one
line for greppability, without going too far to the right. I believe
that's a generally accepted exception to the usual formatting rules in
qemu.
>
> Otherwise looks good. Thanks,
>
>
>
> > + return -1;
> > + }
> > +
> > + return rma_size;
> > +}
> > +
> > /* pSeries LPAR / sPAPR hardware init */
> > static void spapr_machine_init(MachineState *machine)
> > {
> > @@ -2675,7 +2709,6 @@ static void spapr_machine_init(MachineState *machine)
> > int i;
> > MemoryRegion *sysmem = get_system_memory();
> > MemoryRegion *ram = g_new(MemoryRegion, 1);
> > - hwaddr node0_size = spapr_node0_size(machine);
> > long load_limit, fw_size;
> > char *filename;
> > Error *resize_hpt_err = NULL;
> > @@ -2715,20 +2748,7 @@ static void spapr_machine_init(MachineState *machine)
> > exit(1);
> > }
> >
> > - spapr->rma_size = node0_size;
> > -
> > - /* Actually we don't support unbounded RMA anymore since we added
> > - * proper emulation of HV mode. The max we can get is 16G which
> > - * also happens to be what we configure for PAPR mode so make sure
> > - * we don't do anything bigger than that
> > - */
> > - spapr->rma_size = MIN(spapr->rma_size, 0x400000000ull);
> > -
> > - if (spapr->rma_size > node0_size) {
> > - error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> > - spapr->rma_size);
> > - exit(1);
> > - }
> > + spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
> >
> > /* Setup a load limit for the ramdisk leaving room for SLOF and FDT */
> > load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
> > @@ -2956,13 +2976,6 @@ static void spapr_machine_init(MachineState *machine)
> > }
> > }
> >
> > - if (spapr->rma_size < (MIN_RMA_SLOF * MiB)) {
> > - error_report(
> > - "pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area memory)",
> > - MIN_RMA_SLOF);
> > - exit(1);
> > - }
> > -
> > if (kernel_filename) {
> > uint64_t lowaddr = 0;
> >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
© 2016 - 2026 Red Hat, Inc.