linux-kernel - Re: [PATCH] powerpc/pseries/eeh: Fix get PE state translation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87ttc8d0vf.fsf@gmail.com>
Date: Fri, 15 Nov 2024 20:01:48 +0530
From: "Ritesh Harjani (IBM)" <ritesh.list@...il.com> 
To: Narayana Murty N <nnmlinux@...ux.ibm.com>, linuxppc-dev@...ts.ozlabs.org, mpe@...erman.id.au, linux-kernel@...r.kernel.org
Cc: mahesh@...ux.ibm.com, oohall@...il.com, npiggin@...il.com, christophe.leroy@...roup.eu, maddy@...ux.ibm.com, naveen@...nel.org, vaibhav@...ux.ibm.com, ganeshgr@...ux.ibm.com, sbhat@...ux.ibm.com
Subject: Re: [PATCH] powerpc/pseries/eeh: Fix get PE state translation

Narayana Murty N <nnmlinux@...ux.ibm.com> writes:

> The PE Reset State "0" obtained from RTAS calls
> ibm_read_slot_reset_[state|state2] indicates that
> the Reset is deactivated and the PE is not in the MMIO
> Stopped or DMA Stopped state.
>
> With PE Reset State "0", the MMIO and DMA is allowed for
> the PE.

Looking at the PAPR spec - I do agree that it states the same. i.e.
The "0" Initial PE state means the "Not Reset", "Load/Store allowed" &
"DMA allowed" (Normal Operations). 

> The function pseries_eeh_get_state() is currently
> not indicating that to the caller because of  which the
> drivers are unable to resume the MMIO and DMA activity.

It's new to me, but could you help explain the user visible effect
of what gets broken. Since this looks like pseries_eeh_get_state() has
always been like this when it got first implemented.
Is there also a unit test somewhere which you are testing?

IIUC eeh_pe_get_state() was implemented[1] for supporting EEH for VFIO PCI
devices. i.e. the VFIO_EEH_PE_GET_STATE operation of VFIO EEH PE ioctl op
uses pseries_eeh_get_state() helper to query PE state on pseries LPAR.
So are you suggesting that EEH functionality for VFIO PCI device was
never enabled/tested before on pseries?  

[1]: https://lore.kernel.org/all/1402364517-28561-3-git-send-email-gwshan@linux.vnet.ibm.com/

Checking the powernv side of implementation I do see that it does
enables the EEH_STATE_[MMIO|DMA]_ENABLED flags in the result mask for
the callers. So doing the same for pseries eeh get state implementation
does look like the right thing to do here IMO.

> The patch fixes that by reflecting what is actually allowed.

You say this is "fixes" so I am also assuming you are also looking for
stable backports of this? If yes - could you please also add the "Fixes"
tag and cc stable? 

-ritesh

>
> Signed-off-by: Narayana Murty N <nnmlinux@...ux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 1893f66371fa..b12ef382fec7 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -580,8 +580,10 @@ static int pseries_eeh_get_state(struct eeh_pe *pe, int *delay)
>  
>  	switch(rets[0]) {
>  	case 0:
> -		result = EEH_STATE_MMIO_ACTIVE |
> -			 EEH_STATE_DMA_ACTIVE;
> +		result = EEH_STATE_MMIO_ACTIVE	|
> +			 EEH_STATE_DMA_ACTIVE	|
> +			 EEH_STATE_MMIO_ENABLED	|
> +			 EEH_STATE_DMA_ENABLED;
>  		break;
>  	case 1:
>  		result = EEH_STATE_RESET_ACTIVE |
> -- 
> 2.45.2