[kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[Claudio Imbrenda]

Create a region 3 table with fc=1 (i.e. a 2G large page) mapping across the
end of memory.

Check that the part of the large page before the end of memory is accessible,
and the part that is after the end of memory is not.

Also fix a typo in the existing edat2 test.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 s390x/edat.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/s390x/edat.c b/s390x/edat.c
index 16138397..1f582efc 100644
--- a/s390x/edat.c
+++ b/s390x/edat.c
@@ -196,6 +196,8 @@ static void test_edat1(void)
 
 static void test_edat2(void)
 {
+	uint64_t mem_end, i;
+
 	report_prefix_push("edat2");
 	p[0] = 42;
 
@@ -206,7 +208,21 @@ static void test_edat2(void)
 	/* Prefixing should not work with huge pages, just like large pages */
 	report(!memcmp(0, VIRT(prefix_buf), LC_SIZE) &&
 		!memcmp(prefix_buf, VIRT(0), LC_SIZE),
-		"pmd, large, prefixing");
+		"pud, large, prefixing");
+
+	mem_end = get_ram_size();
+	if (mem_end >= BIT_ULL(REGION3_SHIFT)) {
+		report_skip("pud spanning end of memory");
+	} else {
+		for (i = 0; i < mem_end; i += PAGE_SIZE)
+			READ_ONCE(*(uint64_t *)VIRT(i));
+		for (i = mem_end; i < BIT_ULL(REGION3_SHIFT); i += PAGE_SIZE) {
+			expect_pgm_int();
+			READ_ONCE(*(uint64_t *)VIRT(i));
+			assert(clear_pgm_int() == PGM_INT_CODE_ADDRESSING);
+		}
+		report_pass("pud spanning end of memory");
+	}
 
 	report_prefix_pop();
 }
-- 
2.46.2

Re: [kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[Nico Boehr]

Quoting Claudio Imbrenda (2024-10-01 13:36:40)
[...]
> diff --git a/s390x/edat.c b/s390x/edat.c
> index 16138397..1f582efc 100644
> --- a/s390x/edat.c
> +++ b/s390x/edat.c
> @@ -196,6 +196,8 @@ static void test_edat1(void)
>  
>  static void test_edat2(void)
>  {
[...]
> @@ -206,7 +208,21 @@ static void test_edat2(void)
>         /* Prefixing should not work with huge pages, just like large pages */
>         report(!memcmp(0, VIRT(prefix_buf), LC_SIZE) &&
>                 !memcmp(prefix_buf, VIRT(0), LC_SIZE),
> -               "pmd, large, prefixing");
> +               "pud, large, prefixing");
> +
> +       mem_end = get_ram_size();
> +       if (mem_end >= BIT_ULL(REGION3_SHIFT)) {
> +               report_skip("pud spanning end of memory");

Does it make sense to explicitly add a mem parameter in unittests.cfg so
this will never be the case?

Re: [kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[Claudio Imbrenda]

On Wed, 02 Oct 2024 16:25:39 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> Quoting Claudio Imbrenda (2024-10-01 13:36:40)
> [...]
> > diff --git a/s390x/edat.c b/s390x/edat.c
> > index 16138397..1f582efc 100644
> > --- a/s390x/edat.c
> > +++ b/s390x/edat.c
> > @@ -196,6 +196,8 @@ static void test_edat1(void)
> >  
> >  static void test_edat2(void)
> >  {  
> [...]
> > @@ -206,7 +208,21 @@ static void test_edat2(void)
> >         /* Prefixing should not work with huge pages, just like large pages */
> >         report(!memcmp(0, VIRT(prefix_buf), LC_SIZE) &&
> >                 !memcmp(prefix_buf, VIRT(0), LC_SIZE),
> > -               "pmd, large, prefixing");
> > +               "pud, large, prefixing");
> > +
> > +       mem_end = get_ram_size();
> > +       if (mem_end >= BIT_ULL(REGION3_SHIFT)) {
> > +               report_skip("pud spanning end of memory");  
> 
> Does it make sense to explicitly add a mem parameter in unittests.cfg so
> this will never be the case?

hmmm, I did not consider this case; I kinda assumed we would never
increase the default guest size

I do not have any strong opinions

Re: [kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[Nico Boehr]

Quoting Claudio Imbrenda (2024-10-02 16:31:33)
> On Wed, 02 Oct 2024 16:25:39 +0200
> Nico Boehr <nrb@linux.ibm.com> wrote:
> 
> > Quoting Claudio Imbrenda (2024-10-01 13:36:40)
> > [...]
> > > diff --git a/s390x/edat.c b/s390x/edat.c
> > > index 16138397..1f582efc 100644
> > > --- a/s390x/edat.c
> > > +++ b/s390x/edat.c
> > > @@ -196,6 +196,8 @@ static void test_edat1(void)
> > >  
> > >  static void test_edat2(void)
> > >  {  
> > [...]
> > > @@ -206,7 +208,21 @@ static void test_edat2(void)
> > >         /* Prefixing should not work with huge pages, just like large pages */
> > >         report(!memcmp(0, VIRT(prefix_buf), LC_SIZE) &&
> > >                 !memcmp(prefix_buf, VIRT(0), LC_SIZE),
> > > -               "pmd, large, prefixing");
> > > +               "pud, large, prefixing");
> > > +
> > > +       mem_end = get_ram_size();
> > > +       if (mem_end >= BIT_ULL(REGION3_SHIFT)) {
> > > +               report_skip("pud spanning end of memory");  
> > 
> > Does it make sense to explicitly add a mem parameter in unittests.cfg so
> > this will never be the case?
> 
> hmmm, I did not consider this case; I kinda assumed we would never
> increase the default guest size
> 
> I do not have any strong opinions

As long as the default mem size is OK, I think it's fine to leave as-is.

Re: [kvm-unit-tests PATCH v1 1/1] s390x: edat: test 2G large page spanning end of memory

[Nico Boehr]

Quoting Claudio Imbrenda (2024-10-01 13:36:40)
[...]
> diff --git a/s390x/edat.c b/s390x/edat.c
> index 16138397..1f582efc 100644
> --- a/s390x/edat.c
> +++ b/s390x/edat.c
[...]
> @@ -206,7 +208,21 @@ static void test_edat2(void)
>         /* Prefixing should not work with huge pages, just like large pages */
>         report(!memcmp(0, VIRT(prefix_buf), LC_SIZE) &&
>                 !memcmp(prefix_buf, VIRT(0), LC_SIZE),
> -               "pmd, large, prefixing");
> +               "pud, large, prefixing");
> +
> +       mem_end = get_ram_size();
> +       if (mem_end >= BIT_ULL(REGION3_SHIFT)) {

Do you mind introducting REGION3_SIZE like the kernel has?


> +               report_skip("pud spanning end of memory");
> +       } else {
> +               for (i = 0; i < mem_end; i += PAGE_SIZE)
> +                       READ_ONCE(*(uint64_t *)VIRT(i));
> +               for (i = mem_end; i < BIT_ULL(REGION3_SHIFT); i += PAGE_SIZE) {
> +                       expect_pgm_int();
> +                       READ_ONCE(*(uint64_t *)VIRT(i));

Would a write behave any different here?

With or without the suggestions above:
Reviewed-by: Nico Boehr <nrb@linux.ibm.com>