Re: [PATCH 4/4] KVM: s390: vsie: Implement ASTFLEIE facility 2

[Janosch Frank <frankja@linux.ibm.com>]

On 2/12/26 10:24, Christoph Schlameuss wrote:
> From: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> 
> Implement shadowing of format-2 facility list when running in VSIE.
> 
> ASTFLEIE2 is available since IBM z16.
> To function G1 has to run this KVM code and G1 and G2 have to run QEMU
> with ASTFLEIE2 support.
> 
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Co-developed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> ---

[...]

> +static int handle_stfle_2(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
> +			  u32 fac_list_origin)
> +{
> +	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
> +	u8 *shadow_fac = &vsie_page->fac[0];
> +	u64 len;
> +
> +	if (read_guest_real(vcpu, fac_list_origin, &len, sizeof(len)))
> +		return set_validity_icpt(scb_s, 0x1090U);
> +	fac_list_origin += sizeof(len);
> +	len = (len & 0xff);
> +	memcpy(shadow_fac, &len, sizeof(len)); /* discard reserved bits */
> +	shadow_fac += sizeof(len);

You can choose between adding a struct for the new format and what you 
did here. Is this really the better option?


Add the struct and add a constant for the format 2 format control used 
in the second to last line of this function while you're at it.

> +	len += 1;
> +	/* assert no overflow with maximum len */
> +	BUILD_BUG_ON(sizeof(vsie_page->fac) < 257 * sizeof(u64));> +	if (read_guest_real(vcpu, fac_list_origin, shadow_fac, len * 
sizeof(u64)))
> +		return set_validity_icpt(scb_s, 0x1090U);

Sprinkle in some \n between the build bugs and code or just move them to 
the top as one block.
I'd much rather have build bugs at the top of the function than inside 
the code.

> +	BUILD_BUG_ON(!IS_ALIGNED(offsetof(struct vsie_page, fac), 8));

That was already added last patch, no?

> +	scb_s->fac = (u32)virt_to_phys(&vsie_page->fac) | 2;
> +	return 0;
> +}
> +
>   /*
>    * Try to shadow + enable the guest 2 provided facility list.
>    * Retry instruction execution if enabled for and provided by guest 2.
> @@ -1057,9 +1080,11 @@ static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>   		case 0:
>   			return handle_stfle_0(vcpu, vsie_page, fac_list_origin);
>   		case 1:
> +			return set_validity_icpt(&vsie_page->scb_s, 0x1330U);
>   		case 2:
> +			return handle_stfle_2(vcpu, vsie_page, fac_list_origin);
>   		case 3:
> -			unreachable();
> +			return set_validity_icpt(&vsie_page->scb_s, 0x1330U);
>   		}
>   	}
>   	return 0;
>