On Tue, 2023-01-17 at 18:36 +0100, Pierre Morel wrote:
>
> On 1/11/23 09:57, Thomas Huth wrote:
> > On 05/01/2023 15.53, 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.
> >
> > I'm confused ... so does a SCLP value of 2 mean a MNEST level of 4 ?
>
> Sorry, I forgot to change this.
> MNEST = 0 means no MNEST support and only socket is supported so it is
> like MNEST = 2.
> MNEST != 0 set the maximum nested level and correct values may be 2,3 or 4.
> But this setting to 4 should already have been done in previous patch
> where we introduced the books and drawers.
I think setting it to 4 here is fine/preferable, since 2 is the default unless
you tell the guest that more are available, which you do in this patch.
It's only the commit description that is confusing.
>
> I change the commit message with:
> ---
> s390x/sclp: reporting the maximum nested topology entries
>
> The maximum nested topology entries is used by the guest to know
> how many nested topology are available on the machine.
>
> Let's return this information to the guest.
> ---
>
> >
> > > Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> > > ---
> > > 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
> >
> > ... since you update it to 4 here.
>
> Yes, in fact this should be set in the previous patch already to 4.
> So I will do that.
>
> >
> > > 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 */
> > > 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;
> >
> > This seems to be in contradiction to what you've said in the commit
> > description - you set it to 4 and not to 2.
>
> Yes, I change the commit message.
>
> Thanks.
>
> Regards,
> Pierre
>
