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@xxxxxxxxxx/
