From 50c8e11ac0672b726a2b3e2217cb32dc8416299f Mon Sep 17 00:00:00 2001 From: Frederic Barrat Date: Fri, 28 Jan 2022 13:15:02 +0100 Subject: ppc/pnv: Fail DMA access if page permissions are not correct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel Henrique Barboza Message-Id: <20220121152350.381685-1-fbarrat@linux.ibm.com> Signed-off-by: Cédric Le Goater --- hw/pci-host/pnv_phb3.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'hw/pci-host/pnv_phb3.c') 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; } } -- cgit 1.4.1 From 83d2bea68a778b98ecbf9472be6f1ed8031719ac Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 28 Jan 2022 13:15:02 +0100 Subject: ppc/pnv: use a do-while() loop in pnv_phb3_translate_tve() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'taddr' variable is left unintialized, being set only inside the "while ((lev--) >= 0)" loop where we get the TCE address. The 'lev' var is an int32_t that is being initiliazed by the GETFIELD() macro, which returns an uint64_t. For a human reader this means that 'lev' will always be positive or zero. But some compilers may beg to differ. 'lev' being an int32_t can in theory be set as negative, and the "while ((lev--) >= 0)" loop might never be reached, and 'taddr' will be left unitialized. This can cause phb3_error() to use 'taddr' uninitialized down below: if ((is_write & !(tce & 2)) || ((!is_write) && !(tce & 1))) { phb3_error(phb, "TCE access fault at 0x%"PRIx64, taddr); A quick way of fixing it is to use a do/while() loop. This will keep the same semanting as the existing while() loop does and the compiler will understand that 'taddr' will be initialized at least once. Suggested-by: Matheus K. Ferst Resolves: https://gitlab.com/qemu-project/qemu/-/issues/573 Signed-off-by: Daniel Henrique Barboza Message-Id: <20220127122234.842145-2-danielhb413@gmail.com> Signed-off-by: Cédric Le Goater --- hw/pci-host/pnv_phb3.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'hw/pci-host/pnv_phb3.c') diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index a757f1a58e..aafd46b635 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -792,7 +792,9 @@ static void pnv_phb3_translate_tve(PnvPhb3DMASpace *ds, hwaddr addr, sh = tbl_shift * lev + tce_shift; /* TODO: Multi-level untested */ - while ((lev--) >= 0) { + do { + lev--; + /* Grab the TCE address */ taddr = base | (((addr >> sh) & ((1ul << tbl_shift) - 1)) << 3); if (dma_memory_read(&address_space_memory, taddr, &tce, @@ -813,7 +815,7 @@ static void pnv_phb3_translate_tve(PnvPhb3DMASpace *ds, hwaddr addr, } sh -= tbl_shift; base = tce & ~0xfffull; - } + } while (lev >= 0); /* We exit the loop with TCE being the final TCE */ if ((is_write & !(tce & 2)) || ((!is_write) && !(tce & 1))) { -- cgit 1.4.1