Re: [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 1/21/22 12:23, Frederic Barrat wrote:
> If an iommu page has wrong permissions, an error message is displayed,
> but the access is allowed, which is odd. This patch fixes it.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/pci-host/pnv_phb3.c | 11 ++++++-----
>   hw/pci-host/pnv_phb4.c | 11 ++++++-----
>   2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index 7fb35dc031..a757f1a58e 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -816,18 +816,19 @@ static void pnv_phb3_translate_tve(PnvPhb3DMASpace *ds, hwaddr addr,
>           }
>   
>           /* We exit the loop with TCE being the final TCE */
> -        tce_mask = ~((1ull << tce_shift) - 1);
> -        tlb->iova = addr & tce_mask;
> -        tlb->translated_addr = tce & tce_mask;
> -        tlb->addr_mask = ~tce_mask;
> -        tlb->perm = tce & 3;
>           if ((is_write & !(tce & 2)) || ((!is_write) && !(tce & 1))) {
>               phb3_error(phb, "TCE access fault at 0x%"PRIx64, taddr);
>               phb3_error(phb, " xlate %"PRIx64":%c TVE=%"PRIx64, addr,
>                          is_write ? 'W' : 'R', tve);
>               phb3_error(phb, " tta=%"PRIx64" lev=%d tts=%d tps=%d",
>                          tta, lev, tts, tps);
> +            return;
>           }
> +        tce_mask = ~((1ull << tce_shift) - 1);
> +        tlb->iova = addr & tce_mask;
> +        tlb->translated_addr = tce & tce_mask;
> +        tlb->addr_mask = ~tce_mask;
> +        tlb->perm = tce & 3;
>       }
>   }
>   
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index a78add75b0..ee56377c02 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1291,18 +1291,19 @@ static void pnv_phb4_translate_tve(PnvPhb4DMASpace *ds, hwaddr addr,
>           }
>   
>           /* We exit the loop with TCE being the final TCE */
> -        tce_mask = ~((1ull << tce_shift) - 1);
> -        tlb->iova = addr & tce_mask;
> -        tlb->translated_addr = tce & tce_mask;
> -        tlb->addr_mask = ~tce_mask;
> -        tlb->perm = tce & 3;
>           if ((is_write & !(tce & 2)) || ((!is_write) && !(tce & 1))) {
>               phb_error(ds->phb, "TCE access fault at 0x%"PRIx64, taddr);
>               phb_error(ds->phb, " xlate %"PRIx64":%c TVE=%"PRIx64, addr,
>                          is_write ? 'W' : 'R', tve);
>               phb_error(ds->phb, " tta=%"PRIx64" lev=%d tts=%d tps=%d",
>                          tta, lev, tts, tps);
> +            return;
>           }
> +        tce_mask = ~((1ull << tce_shift) - 1);
> +        tlb->iova = addr & tce_mask;
> +        tlb->translated_addr = tce & tce_mask;
> +        tlb->addr_mask = ~tce_mask;
> +        tlb->perm = tce & 3;
>       }
>   }
>