[Crash-utility] Fwd: s390x fixes

[Dave Anderson]

----- Original Message -----
> Hi Dave,
> 
> On Wed, 02 May 2012 09:21:57 -0400 (EDT)
> Dave Anderson <anderson at redhat.com> wrote:
> > 
> > ----- Original Message -----
> > 
> > > > 
> > > > And that's because when a "machdep->uvtop()" operation is done on
> > > > a user page that is not resident, the machine-dependent function
> > > > should return FALSE -- but it should return the PTE value in the
> > > > paddr pointer field so that it can be translated by
> > > > vm_area_page_dump(). The s390x_uvtop() does not return the PTE,
> > > > so the failed output can vary, because it's using an
> > > > uninitialized "paddr" stack variable. But this is another easy
> > > > fix, in this case to s390x_vtop():
> > > > 
> > > > /* lookup virtual address in page tables */
> > > > int s390x_vtop(ulong table, ulong vaddr, physaddr_t *phys_addr,
> > > > int verbose) {
> > > >         ulong entry, paddr;
> > > >         int level, len;
> > > > 
> > > > +       *phys_addr = 0;
> > > 
> > > 
> > > Looks also good. But probably I should implement a better fix that
> > > returns the pte for s390x swap entries.
> > 
> > That's true -- so since you've got the __swp_type() and __swp_offset()
> > macros #define'd for s390x, presumably it should work if you just pass
> > it back as is?  Even though you've got the common s390x_vtop()
> > function used by both s390x_kvtop() and s390x_uvtop(), I think it
> > would be safe to pass the PTE entry back in either case, because
> > kvtop() operations don't care about the paddr contents if you return
> > FALSE.
> 
> The following patch implements swap entry support for s390x and
> fixes your issues (2) and (3).
> 
> Michael
> ---
>  s390x.c |   32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> --- a/s390x.c
> +++ b/s390x.c
> @@ -574,6 +574,19 @@ static ulong _kl_rsg_table_deref_s390x(u
>  	return entry;
>  }
>  
> +/* Check for swap entry */
> +static int swap_entry(ulong entry)
> +{
> +	if (THIS_KERNEL_VERSION < LINUX(2,6,19)) {
> +		if ((entry & 0x601ULL) == 0x600ULL)
> +			return 1;
> +	} else {
> +		if ((entry & 0x403ULL) == 0x403ULL)
> +			return 1;
> +	}
> +	return 0;
> +}
> +
>  /* Page table traversal function */
>  static ulong _kl_pg_table_deref_s390x(ulong vaddr, ulong table)
>  {
> @@ -583,13 +596,11 @@ static ulong _kl_pg_table_deref_s390x(ul
>  	readmem(table + offset, KVADDR, &entry, sizeof(entry), "entry",
>  		FAULT_ON_ERROR);
>  	/*
> -	 * Check if the page table entry could be read and doesn't have
> -	 * any of the reserved bits set.
> +	 * Return zero if the page table entry has any of the reserved bits
> +	 * set (0x900) or the invalid bit (0x400) is set and it is not a
> +	 * swap entry.
>  	 */
> -	if (entry & 0x900ULL)
> -		return 0;
> -	/* Check if the page table entry has the invalid bit set. */
> -	if (entry & 0x400ULL)
> +	if ((entry & 0xd00ULL) && !swap_entry(entry))
>  		return 0;
>  	/* Page table entry is valid and well formed. */
>  	return entry;
> @@ -601,6 +612,7 @@ int s390x_vtop(ulong table, ulong vaddr,
>  	ulong entry, paddr;
>  	int level, len;
>  
> +	*phys_addr = 0;
>  	/*
>  	 * Walk the region and segment tables.
>  	 * We assume that the table length field in the asce is set to the
> @@ -619,7 +631,7 @@ int s390x_vtop(ulong table, ulong vaddr,
>  	while (level >= 0) {
>  		entry = _kl_rsg_table_deref_s390x(vaddr, table, len, level);
>  		if (!entry)
> -			return 0;
> +			return FALSE;
>  		table = entry & ~0xfffULL;
>  		len = entry & 0x3ULL;
>  		level--;
> @@ -637,6 +649,12 @@ int s390x_vtop(ulong table, ulong vaddr,
>  	if (!entry)
>  		return FALSE;
>  
> +	/* For swap entries we have to return FALSE and phys_addr = PTE */
> +	if (swap_entry(entry)) {
> +		*phys_addr = entry;
> +		return FALSE;
> +	}
> +
>  	/* Isolate the page origin from the page table entry. */
>  	paddr = entry & ~0xfffULL;

Nice -- queued for crash-6.0.7.

Thanks,
  Dave