linux-kernel - Re: [PATCH v2 4/7] s390/physmem_info: query diag500(STORAGE LIMIT) to support QEMU/KVM memory devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8131b905c61a7baf4bd09ec4a08e1ace84d36754.camel@linux.ibm.com>
Date: Tue, 15 Oct 2024 11:01:44 -0400
From: Eric Farman <farman@...ux.ibm.com>
To: Heiko Carstens <hca@...ux.ibm.com>, David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-s390@...r.kernel.org, virtualization@...ts.linux.dev,
        linux-doc@...r.kernel.org, kvm@...r.kernel.org,
        Vasily Gorbik
 <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian
 Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle
 <svens@...ux.ibm.com>, Thomas Huth <thuth@...hat.com>,
        Cornelia Huck
 <cohuck@...hat.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Claudio
 Imbrenda <imbrenda@...ux.ibm.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
        Eugenio Pérez <eperezma@...hat.com>,
        Andrew Morton
 <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>,
        Mario
 Casquero <mcasquer@...hat.com>
Subject: Re: [PATCH v2 4/7] s390/physmem_info: query diag500(STORAGE LIMIT)
 to support QEMU/KVM memory devices

On Mon, 2024-10-14 at 20:43 +0200, Heiko Carstens wrote:
> On Mon, Oct 14, 2024 at 04:46:16PM +0200, David Hildenbrand wrote:
> > To support memory devices under QEMU/KVM, such as virtio-mem,
> > we have to prepare our kernel virtual address space accordingly and
> > have to know the highest possible physical memory address we might see
> > later: the storage limit. The good old SCLP interface is not suitable for
> > this use case.
> > 
> > In particular, memory owned by memory devices has no relationship to
> > storage increments, it is always detected using the device driver, and
> > unaware OSes (no driver) must never try making use of that memory.
> > Consequently this memory is located outside of the "maximum storage
> > increment"-indicated memory range.
> > 
> > Let's use our new diag500 STORAGE_LIMIT subcode to query this storage
> > limit that can exceed the "maximum storage increment", and use the
> > existing interfaces (i.e., SCLP) to obtain information about the initial
> > memory that is not owned+managed by memory devices.

...snip...

> The patch below changes your code accordingly, but it is
> untested. Please verify that your code still works.

...snip...

> diff --git a/arch/s390/boot/physmem_info.c b/arch/s390/boot/physmem_info.c
> index fb4e66e80fd8..975fc478e0e3 100644
> --- a/arch/s390/boot/physmem_info.c
> +++ b/arch/s390/boot/physmem_info.c
> @@ -109,10 +109,11 @@ static int diag260(void)
>  	return 0;
>  }
>  
> +#define DIAG500_SC_STOR_LIMIT 4
> +
>  static int diag500_storage_limit(unsigned long *max_physmem_end)
>  {
> -	register unsigned long __nr asm("1") = 0x4;
> -	register unsigned long __storage_limit asm("2") = 0;
> +	unsigned long storage_limit;
>  	unsigned long reg1, reg2;
>  	psw_t old;
>  
> @@ -123,21 +124,24 @@ static int diag500_storage_limit(unsigned long *max_physmem_end)
>  		"	st	%[reg2],4(%[psw_pgm])\n"
>  		"	larl	%[reg1],1f\n"
>  		"	stg	%[reg1],8(%[psw_pgm])\n"
> +		"	lghi	1,%[subcode]\n"
> +		"	lghi	2,0\n"
>  		"	diag	2,4,0x500\n"
>  		"1:	mvc	0(16,%[psw_pgm]),0(%[psw_old])\n"
> +		"	lgr	%[slimit],2\n"
>  		: [reg1] "=&d" (reg1),
>  		  [reg2] "=&a" (reg2),
> -		  "+&d" (__storage_limit),
> +		  [slimit] "=d" (storage_limit),
>  		  "=Q" (get_lowcore()->program_new_psw),
>  		  "=Q" (old)
>  		: [psw_old] "a" (&old),
>  		  [psw_pgm] "a" (&get_lowcore()->program_new_psw),
> -		  "d" (__nr)
> -		: "memory");
> -	if (!__storage_limit)
> -	        return -EINVAL;
> -	/* convert inclusive end to exclusive end. */
> -	*max_physmem_end = __storage_limit + 1;
> +		  [subcode] "i" (DIAG500_SC_STOR_LIMIT)
> +		: "memory", "1", "2");
> +	if (!storage_limit)
> +		return -EINVAL;
> +	/* Convert inclusive end to exclusive end */
> +	*max_physmem_end = storage_limit + 1;
>  	return 0;
>  }
>  
> 

I like the idea of a defined constant here instead of hardcoded, but maybe it should be placed
somewhere in include/uapi so that QEMU can pick it up with update-linux-headers.sh and be in sync
with the kernel, instead of just an equivalent definition in [1] ?

[1] https://lore.kernel.org/qemu-devel/20241008105455.2302628-8-david@redhat.com/