Re: [PATCH 2/5] s390x/pv: Implement CGS check handler

[Thomas Huth]

On 04/01/2023 12.51, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> When a protected VM is started with the maximum number of CPUs (248),
> the service call providing information on the CPUs requires more
> buffer space than allocated and QEMU disgracefully aborts :
>
>      LOADPARM=[........]
>      Using virtio-blk.
>      Using SCSI scheme.
>      
> ...................................................................................
>      qemu-system-s390x: KVM_S390_MEM_OP failed: Argument list too long
>
> Implement a test for this limitation in the ConfidentialGuestSupportClass
> check handler and provide some valid information to the user before the
> machine starts.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/s390x/pv.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
> index 8dfe92d8df..3a7ec70634 100644
> --- a/hw/s390x/pv.c
> +++ b/hw/s390x/pv.c
> @@ -266,6 +266,26 @@ int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error 
> **errp)
>       return 0;
>   }
>   
> +static bool s390_pv_check_cpus(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    uint32_t pv_max_cpus = mc->max_cpus - 1;

Not sure whether "mc->max_cpus - 1" is the right approach here. I think it 
would be better to calculate the amount of CPUs that we can support.

So AFAIK the problem is that SCLP information that is gathered during 
read_SCP_info() in hw/s390x/sclp.c. If protected virtualization is enabled, 
everything has to fit in one page (i.e. 4096 bytes) there.

So we have space for

 (TARGET_PAGE_SIZE - offset_cpu) / sizeof(CPUEntry)

CPUs.

With S390_FEAT_EXTENDED_LENGTH_SCCB enabled, offset_cpu is 144 (see struct 
ReadInfo in sclp.h), otherwise it is 128.

That means, with S390_FEAT_EXTENDED_LENGTH_SCCB we can have a maximum of:

 (4096 - 144) / 16 = 247 CPUs

which is what you were trying to check with the mc->max_cpus - 1 here.

But with "-cpu els=off", it sounds like we could fit all 248 also with 
protected VMs? Could you please give it a try?

Anyway, instead of using "pv_max_cpus = mc->max_cpus - 1" I'd suggest to use 
something like this instead:

 int offset_cpu = s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
                     offsetof(ReadInfo, entries) :
                     SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
 pv_max_cpus = (TARGET_PAGE_SIZE - offset_cpu) /sizeof(CPUEntry);

  Thomas