Message ID | 20200616104231.27805-1-aik@ozlabs.ru (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [kernel] powerpc/powernv/ioda: Return correct error if TCE level allocation failed | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (062ce06f9dcd140b6cd97102fec593a57c5fb397) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > The iommu_table_ops::xchg_no_kill() callback updates TCE. It is quite > possible that not entire table is allocated if it is huge and multilevel > so xchg may also allocate subtables. If failed, it returns H_HARDWARE > for failed allocation and H_TOO_HARD if it needs it but cannot do because > the alloc parameter is "false" (set when called with MMU=off to force > retry with MMU=on). > > The problem is that having separate errors only matters in real mode > (MMU=off) but the only caller with alloc="false" does not check the exact > error code and simply returns H_TOO_HARD; and for every other mode > alloc is "true". Also, the function is also called from the ioctl() > handler of the VFIO SPAPR TCE IOMMU subdriver which does not expect > hypervisor error codes (H_xxx) and will expose them to the userspace. > > This converts wrong error codes to a simple -1. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > I could make it "return alloc ? -ENOMEM : -EBUSY" but > is EBUSY a good match for H_TOO_HARD? I think -EAGAIN would be the best match. But it would be simpler if it just returned -ENOMEM always. In both cases the problem is that the function needs to allocate memory but couldn't. If a caller passes alloc=false, it knows that, so if it sees ENOMEM it can retry with alloc=true. cheers
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c index f923359d8afc..59d73fdadeb9 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c +++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c @@ -166,7 +166,7 @@ int pnv_tce_xchg(struct iommu_table *tbl, long index, if (!ptce) { ptce = pnv_tce(tbl, false, idx, alloc); if (!ptce) - return alloc ? H_HARDWARE : H_TOO_HARD; + return -1; } if (newtce & TCE_PCI_WRITE)
The iommu_table_ops::xchg_no_kill() callback updates TCE. It is quite possible that not entire table is allocated if it is huge and multilevel so xchg may also allocate subtables. If failed, it returns H_HARDWARE for failed allocation and H_TOO_HARD if it needs it but cannot do because the alloc parameter is "false" (set when called with MMU=off to force retry with MMU=on). The problem is that having separate errors only matters in real mode (MMU=off) but the only caller with alloc="false" does not check the exact error code and simply returns H_TOO_HARD; and for every other mode alloc is "true". Also, the function is also called from the ioctl() handler of the VFIO SPAPR TCE IOMMU subdriver which does not expect hypervisor error codes (H_xxx) and will expose them to the userspace. This converts wrong error codes to a simple -1. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- I could make it "return alloc ? -ENOMEM : -EBUSY" but is EBUSY a good match for H_TOO_HARD? --- arch/powerpc/platforms/powernv/pci-ioda-tce.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)