Re: [PATCH v1 2/3] KVM: s390: Add capability to support 2G hugepages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Low] The new capability KVM_CAP_S390_HPAGE_2G is completely undocumented in the KVM API documentation.
- [Low] The new KVM capability is missing from the tools/ UAPI header.

Pre-existing issues:
- [High] Userspace can bypass the mutual exclusivity between CMMA and hugepages by enabling CMMA after hugepages have been enabled.
- [High] Userspace can bypass the architectural restriction that Protected VMs (PV) must use 4K pages by enabling hugepages after the VM is already in protected mode.
--

commit 43482c954e0933d74bf56361955e125830f3a3fe
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Add capability to support 2G hugepages
    
This patch adds the KVM_CAP_S390_HPAGE_2G capability to allow 2G hugepages
to back the guest. It enables the GMAP_FLAG_ALLOW_HPAGE_2G flag for the 
guest gmap and propagates it appropriately.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 801a622691b6a..616d1db7c0d9e 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
> @@ -901,6 +906,27 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>  		VM_EVENT(kvm, 3, "ENABLE: CAP_S390_HPAGE %s",
>  			 r ? "(not available)" : "(success)");
>  		break;
> +	case KVM_CAP_S390_HPAGE_2G:
> +		mutex_lock(&kvm->lock);
> +		if (kvm->created_vcpus) {
> +			r = -EBUSY;
> +		} else if (!hpage_2g || kvm->arch.use_cmma || kvm_is_ucontrol(kvm)) {

[Severity: High]
This is a pre-existing issue, but while this capability enablement checks 
if VCPUs are created, could userspace bypass the Protected VM restriction
by enabling PV first and then enabling hugepages before creating VCPUs?

In kvm_s390_pv_init_vm(), the flags are cleared:

    clear_bit(GMAP_FLAG_ALLOW_HPAGE_1M, &kvm->arch.gmap->flags);
    clear_bit(GMAP_FLAG_ALLOW_HPAGE_2G, &kvm->arch.gmap->flags);

Since !kvm_s390_pv_is_protected(kvm) is not checked during the capability 
enablement, does this allow KVM to map unsupported 1M/2G hugepages into 
a Protected VM's address space?

[Severity: High]
This is also a pre-existing issue, but while the hugepage enablement
checks kvm->arch.use_cmma to enforce mutual exclusivity, does the CMMA
enablement handler lack the reciprocal check?

In kvm_s390_set_mem_control():

    kvm->arch.use_cmma = 1;
    /* Not compatible with cmma. */
    kvm->arch.use_pfmfi = 0;
    ret = 0;

Could userspace enable hugepages first, and then enable CMMA, causing
CMMA dirty tracking operations to silently skip huge pages since they
are not handled by the dat_walk_ops .pte_entry?

> +			r = -EINVAL;
> +		} else {
> +			r = 0;
> +			set_bit(GMAP_FLAG_ALLOW_HPAGE_2G, &kvm->arch.gmap->flags);
[ ... ]
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6c8afa2047bf3..419011097fa8e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
[ ... ]
> @@ -996,6 +996,7 @@ struct kvm_enable_cap {
>  #define KVM_CAP_S390_USER_OPEREXEC 246
>  #define KVM_CAP_S390_KEYOP 247
>  #define KVM_CAP_S390_VSIE_ESAMODE 248
> +#define KVM_CAP_S390_HPAGE_2G 249

[Severity: Low]
Should this new capability be documented in Documentation/virt/kvm/api.rst
to explain its usage, side effects, and mutual exclusivity with CMMA and 
ucontrol VMs?

[Severity: Low]
Does this capability addition also need to be synchronized to 
tools/include/uapi/linux/kvm.h so that userspace tools and selftests
compiled from the kernel tree have the definition available?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603154758.234768-1-imbrenda@linux.ibm.com?part=2