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

[Frederic Barrat]

On 21/01/2022 18:35, Cédric Le Goater wrote:
> On 1/21/22 16: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.
>
>
> Being curious. How do you generate such errors ? could we have the
> output ?


By acking the code building the tce table to remove permissions on the 
page. See pnv_tce_build().
Also, on bare metal, device drivers using 64-bit dma will hit a 
translation entry (TVE#1) configured in bypass mode. So to avoid that 
and force translation, you need to add "iommu=nobypass" to the kernel 
boot args.

Then you will see:
phb4[0:1]: TCE access fault at 0x2a510080
phb4[0:1]:  xlate 100000:R TVE=32c702505
phb4[0:1]:  tta=32c70 lev=-2 tts=5 tps=5
Invalid read at addr 0x100000, size 4, region '(null)', reason: rejected

P8 is more complicated...

  Fred



> Thanks,
>
> C.
>
>
> > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.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;
> >       }
> >   }