Re: [PATCH v14 04/11] s390x/sclp: reporting the maximum nested topology entries

[Pierre Morel]

On 1/17/23 20:58, Nina Schoetterl-Glausch wrote:
> 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.

Yes.
Only the commit message is confusing, it is to be set on 4 in this patch.

I change the commit message to:

---
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 change the MNEST value from 2 to 4 in the SCLP READ INFO
structure now that we support books and drawers.
---

is it OK ?

Regards,
Pierre

> > 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@linux.ibm.com>
> > > > ---
> > > >    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

--
Pierre Morel
IBM Lab Boeblingen