On Thu, 2023-01-05 at 15:53 +0100, Pierre Morel wrote:
> The maximum nested topology entries is used by the guest to know
> how many nested topology are available on the machine.
>
> Currently, SCLP READ SCP INFO reports MNEST = 0, which is the
> equivalent of reporting the default value of 2.
> Let's use the default SCLP value of 2 and increase this value in the
> future patches implementing higher levels.
>
> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
Reviewed-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
if you address the issues Thomas found with the commit description
and the nits below.
> ---
> include/hw/s390x/sclp.h | 5 +++--
> hw/s390x/sclp.c | 4 ++++
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index 712fd68123..4ce852473c 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -112,12 +112,13 @@ typedef struct CPUEntry {
> } QEMU_PACKED CPUEntry;
>
> #define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET 128
> -#define SCLP_READ_SCP_INFO_MNEST 2
> +#define SCLP_READ_SCP_INFO_MNEST 4
> typedef struct ReadInfo {
> SCCBHeader h;
> uint16_t rnmax;
> uint8_t rnsize;
> - uint8_t _reserved1[16 - 11]; /* 11-15 */
> + uint8_t _reserved1[15 - 11]; /* 11-14 */
> + uint8_t stsi_parm; /* 15-16 */
The numbering here is the same as the one in the arch doc, instead
of the maybe more usual one where the right number is exclusive.
So 15-16 looks like a two byte field, so just do 15 or just drop it.
> uint16_t entries_cpu; /* 16-17 */
> uint16_t offset_cpu; /* 18-19 */
> uint8_t _reserved2[24 - 20]; /* 20-23 */
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index eff74479f4..07e3cb4cac 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -20,6 +20,7 @@
> #include "hw/s390x/event-facility.h"
> #include "hw/s390x/s390-pci-bus.h"
> #include "hw/s390x/ipl.h"
> +#include "hw/s390x/cpu-topology.h"
>
> static inline SCLPDevice *get_sclp_device(void)
> {
> @@ -125,6 +126,9 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>
> /* CPU information */
> prepare_cpu_entries(machine, entries_start, &cpu_count);
> + if (s390_has_topology()) {
> + read_info->stsi_parm = SCLP_READ_SCP_INFO_MNEST;
> + }
Maybe move that up a bit, not sure if it belongs under the CPU information section.
I'd leave prepare_cpu_entries and read_info->entries_cpu = adjacent in any case.
> read_info->entries_cpu = cpu_to_be16(cpu_count);
> read_info->offset_cpu = cpu_to_be16(offset_cpu);
> read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
