* [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct
@ 2022-01-21 15:23 Frederic Barrat
2022-01-21 16:20 ` Daniel Henrique Barboza
2022-01-21 17:35 ` Cédric Le Goater
0 siblings, 2 replies; 4+ messages in thread
From: Frederic Barrat @ 2022-01-21 15:23 UTC (permalink / raw)
To: clg, danielhb413, qemu-ppc, qemu-devel; +Cc: aik
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>
---
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;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct
2022-01-21 15:23 [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct Frederic Barrat
@ 2022-01-21 16:20 ` Daniel Henrique Barboza
2022-01-21 17:35 ` Cédric Le Goater
1 sibling, 0 replies; 4+ messages in thread
From: Daniel Henrique Barboza @ 2022-01-21 16:20 UTC (permalink / raw)
To: Frederic Barrat, clg, qemu-ppc, qemu-devel; +Cc: aik
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;
> }
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct
2022-01-21 15:23 [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct Frederic Barrat
2022-01-21 16:20 ` Daniel Henrique Barboza
@ 2022-01-21 17:35 ` Cédric Le Goater
2022-01-24 8:22 ` Frederic Barrat
1 sibling, 1 reply; 4+ messages in thread
From: Cédric Le Goater @ 2022-01-21 17:35 UTC (permalink / raw)
To: Frederic Barrat, danielhb413, qemu-ppc, qemu-devel; +Cc: aik
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 ?
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;
> }
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct
2022-01-21 17:35 ` Cédric Le Goater
@ 2022-01-24 8:22 ` Frederic Barrat
0 siblings, 0 replies; 4+ messages in thread
From: Frederic Barrat @ 2022-01-24 8:22 UTC (permalink / raw)
To: Cédric Le Goater, danielhb413, qemu-ppc, qemu-devel; +Cc: aik
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;
>> }
>> }
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-24 8:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-21 15:23 [PATCH] ppc/pnv: Fail DMA access if page permissions are not correct Frederic Barrat
2022-01-21 16:20 ` Daniel Henrique Barboza
2022-01-21 17:35 ` Cédric Le Goater
2022-01-24 8:22 ` Frederic Barrat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).